NetBSD Problem Report #55169
From skrll@netbsd.org Mon Apr 13 08:07:25 2020
Return-Path: <skrll@netbsd.org>
Received: from mail.netbsd.org (mail.netbsd.org [199.233.217.200])
(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
(Client CN "mail.NetBSD.org", Issuer "mail.NetBSD.org CA" (not verified))
by mollari.NetBSD.org (Postfix) with ESMTPS id 2B5561A9213
for <gnats-bugs@gnats.NetBSD.org>; Mon, 13 Apr 2020 08:07:25 +0000 (UTC)
Message-Id: <20200413080724.D0EB184D63@mail.netbsd.org>
Date: Mon, 13 Apr 2020 08:07:24 +0000 (UTC)
From: Nick Hudson <skrll@netbsd.org>
Reply-To: Nick Hudson <skrll@netbsd.org>
To: gnats-bugs@NetBSD.org
Subject: Use after free in ata layer
X-Send-Pr-Version: 3.95
>Number: 55169
>Category: kern
>Synopsis: KASAN reported use after free by ahcicore.c
>Confidential: no
>Severity: serious
>Priority: medium
>Responsible: jdolecek
>State: closed
>Class: sw-bug
>Submitter-Id: net
>Arrival-Date: Mon Apr 13 08:10:00 +0000 2020
>Closed-Date: Mon Apr 13 12:04:06 +0000 2020
>Last-Modified: Fri Dec 30 14:40:01 +0000 2022
>Originator: Nick Hudson
>Release: NetBSD 9.99.56
>Organization:
None / Some
>Environment:
System: NetBSD tx1 (GENERIC64_TX1) #0: Sun Apr 12 10:00:26 BST 2020 nick@zoom:/home/nick/netbsd/trunk/obj.evbarm64-el/sys/arch/evbarm/compile/GENERIC64_TX1
Architecture: aarch64
Machine: evbarm
>Description:
ahci_ata_bio+0xe0 is sys/dev/ic/ahcisata_core.c#1394
panic: ASan: Unauthorized Access In 0xffffc00000290014: Addr 0xffffc0000b4d6898 [2 bytes, read, PoolUseAfterFree]
[ 505.7560156] cpu0: Begin traceback...
[ 505.7560156] trace fp ffffc000c9e95d80
[ 505.7647943] fp ffffc000c9e95db0 vpanic() at ffffc000007851d8 netbsd:vpanic+0x210
[ 505.7747959] fp ffffc000c9e95e30 panic() at ffffc0000078530c netbsd:panic+0xa4
[ 505.7847958] fp ffffc000c9e95f10 kasan_report() at ffffc00000749b0c netbsd:kasan_report+0x94
[ 505.7948010] fp ffffc000c9e95f20 __asan_load2() at ffffc000007549e8 netbsd:__asan_load2+0xc8
[ 505.8047986] fp ffffc000c9e95f30 ahci_ata_bio() at ffffc00000290010 netbsd:ahci_ata_bio+0xe0
[ 505.8155968] fp ffffc000c9e95f80 wdstart1() at ffffc000000ec038 netbsd:wdstart1+0x2e0
[ 505.8356017] fp ffffc000c9e95ff0 wd_diskstart() at ffffc000000ed0f4 netbsd:wd_diskstart+0xbc
[ 505.8456013] fp ffffc000c9e96040 dk_start() at ffffc00000839f3c netbsd:dk_start+0x14c
[ 505.8556029] fp ffffc000c9e960b0 bdev_strategy() at ffffc00000765704 netbsd:bdev_strategy+0xec
[ 505.8656035] fp ffffc000c9e960e0 spec_strategy() at ffffc000008343d4 netbsd:spec_strategy+0x15c
[ 505.8757272] fp ffffc000c9e96130 VOP_STRATEGY() at ffffc000008238a0 netbsd:VOP_STRATEGY+0xf0
[ 505.8856061] fp ffffc000c9e961d0 genfs_getpages() at ffffc000008286ac netbsd:genfs_getpages+0x18a4
[ 505.8956079] fp ffffc000c9e96620 VOP_GETPAGES() at ffffc00000824058 netbsd:VOP_GETPAGES+0x128
[ 505.9056072] fp ffffc000c9e96780 ubc_fault() at ffffc0000069a7f4 netbsd:ubc_fault+0x28c
[ 505.9167081] fp ffffc000c9e968e0 uvm_fault_internal() at ffffc0000069e5fc netbsd:uvm_fault_internal+0x864
[ 505.9286916] fp ffffc000c9e96d80 data_abort_handler() at ffffc000000e0e40 netbsd:data_abort_handler+0x228
[ 505.9402960] tf ffffc000c9e96e00 el1_trap() at ffffc000000dc5c0 netbsd:el1_trap
[ 505.9510246] ---- trapframe 0xffffc000c9e96e00 (304 bytes) ----
[ 505.9510246] pc=ffffc00000af5778, spsr=0000000020000005
[ 505.9640311] esr=0000000096000007, far=ffffc000bacdd000
[ 505.9640311] x0=ffffc0000bf84100, x1=ffffc000bacdd000
[ 505.9752974] x2=0000000000000004, x3=ffffc000bacdd000
[ 505.9752974] x4=0000000000000004, x5=0000c0000bf84504
[ 505.9865663] x6=ffffc0000bf84100, x7=ffffc0000bf84100
[ 505.9865663] x8=0000000000000000, x9=0000c0000bf84504
[ 505.9978306] x10=0000000000000404, x11=0000000000000001
[ 505.9978306] x12=00000000f2f2f2f2, x13=00000000f1f1f1f1
[ 506.0090974] x14=00000000000000a5, x15=ffffc0000b579a20
[ 506.0090974] x16=000000020013f1f8, x17=0000fa48bedc85a4
[ 506.0203643] x18=0000000000000000, x19=ffffc000bacdd000
[ 506.0203643] x20=ffffc0000bf84100, x21=ffffc00001a39ca0
[ 506.0316303] x22=ffffc00000cae000, x23=ffffc000bacdd000
[ 506.0316303] x24=ffffc0000bf84100, x25=ffffc000019a9d00
[ 506.0428970] x26=ffffc000c9e97658, x27=ffffc00000cae820
[ 506.0428970] x28=0000000000000000, fp=x29=ffffc000c9e971b0
[ 506.0541686] lr=x30=ffffc000000dbabc, sp=ffffc000c9e97130
[ 506.0541686] ------------------------------------------------
[ 506.0654304] fp ffffc000c9e971b0 memcpy() at ffffc00000af5778 netbsd:memcpy+0x118
[ 506.0741663] fp ffffc000c9e971c0 copyout_vmspace() at ffffc00000762afc netbsd:copyout_vmspace+0x124
[ 506.0841691] fp ffffc000c9e972d0 uiomove() at ffffc00000762cb0 netbsd:uiomove+0x100
[ 506.0941684] fp ffffc000c9e97350 ubc_uiomove() at ffffc0000069b194 netbsd:ubc_uiomove+0x1ec
[ 506.1041705] fp ffffc000c9e97440 ffs_read() at ffffc0000068072c netbsd:ffs_read+0x174
[ 506.1141718] fp ffffc000c9e974b0 VOP_READ() at ffffc000008214c4 netbsd:VOP_READ+0xe4
[ 506.1241725] fp ffffc000c9e97570 vn_rdwr() at ffffc00000814be0 netbsd:vn_rdwr+0x1d8
[ 506.1341724] fp ffffc000c9e976b0 check_exec() at ffffc000006eb1b4 netbsd:check_exec+0x3f4
[ 506.1441746] fp ffffc000c9e978b0 execve_loadvm() at ffffc000006ebc50 netbsd:execve_loadvm+0x430
[ 506.1549577] fp ffffc000c9e97b10 execve1() at ffffc000006f0040 netbsd:execve1+0x60
[ 506.1657748] fp ffffc000c9e97d90 syscall() at ffffc000000de7e4 netbsd:syscall+0x2a4
[ 506.1766755] tf ffffc000c9e97ed0 el0_trap() at ffffc000000dc62c netbsd:el0_trap
[ 506.1876397] ---- trapframe 0xffffc000c9e97ed0 (304 bytes) ----
[ 506.1876397] pc=0000fa48bedc85a8, spsr=0000000080000000
[ 506.2006471] esr=000000005600003b, far=000000020011a4e0
[ 506.2006471] x0=0000000200140910, x1=0000000200140938
[ 506.2119136] x2=0000000200140948, x3=0000000000000000
[ 506.2119136] x4=0000000000000000, x5=0000000200140938
[ 506.2231797] x6=0000000200140000, x7=0000000000000198
[ 506.2231797] x8=0000000200140910, x9=0000000000000011
[ 506.2344467] x10=0000000000000004, x11=0000000000000001
[ 506.2344467] x12=0000fa48bf17b458, x13=0000000000000005
[ 506.2457148] x14=00000000000000a5, x15=0000fa48be800ea8
[ 506.2457148] x16=000000020013f1f8, x17=0000fa48bedc85a4
[ 506.2569798] x18=0000000000000000, x19=0000000000000000
[ 506.2569798] x20=0000000200140938, x21=0000000000000000
[ 506.2682485] x22=0000000200140910, x23=0000000200140948
[ 506.2682485] x24=0000000000000000, x25=0000000200140120
[ 506.2795127] x26=0000000000000000, x27=0000000000000000
[ 506.2795127] x28=0000000000000000, fp=x29=0000ffffffe37b40
[ 506.2907791] lr=x30=0000000200108de0, sp=0000ffffffe37b40
[ 506.2907791] ------------------------------------------------
[ 506.3020457] cpu0: End traceback...
Stopped in pid 611.1 (sh) at netbsd:cpu_Debugger+0x4: ret
db{0}>
>How-To-Repeat:
Don't do an awful lot to provke it
>Fix:
unknown
>Release-Note:
>Audit-Trail:
Responsible-Changed-From-To: gnats-admin->kern-bug-people
Responsible-Changed-By: skrll@NetBSD.org
Responsible-Changed-When: Mon, 13 Apr 2020 08:18:11 +0000
Responsible-Changed-Why:
Fix error
Responsible-Changed-From-To: kern-bug-people->jdolecek
Responsible-Changed-By: jdolecek@NetBSD.org
Responsible-Changed-When: Mon, 13 Apr 2020 08:26:06 +0000
Responsible-Changed-Why:
I'll look at this.
From: "Jaromir Dolecek" <jdolecek@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/55169 CVS commit: src/sys/dev
Date: Mon, 13 Apr 2020 10:49:35 +0000
Module Name: src
Committed By: jdolecek
Date: Mon Apr 13 10:49:35 UTC 2020
Modified Files:
src/sys/dev/ata: ata.c ata_recovery.c ata_wdc.c atavar.h satapmp_subr.c
wd.c
src/sys/dev/ic: ahcisata_core.c mvsata.c siisata.c wdc.c wdcvar.h
src/sys/dev/scsipi: atapi_wdc.c
Log Message:
fix use-after-free for ata xfer on bio submission found by KASAN
driver ata_bio hooks read parts of the xfer after ata_exec_xfer()
call in order to determine return value, change so that the hook
doesn't return any value - callers do not care already,
as all I/O requests are asynchronous
this problem was uncovered by recent change for wd(4) to not hold
wd mutex during ata_bio call, the interrupt for the xfer might
thus actually fire immediately
adjust also ata_exec_command driver hooks similarily - remove all
completion and waiting logic from drivers, upper layer ata code
using AT_WAIT/AT_POLL changed to call ata_wait_cmd() itself
PR kern/55169 by Nick Hudson
To generate a diff of this commit:
cvs rdiff -u -r1.154 -r1.155 src/sys/dev/ata/ata.c
cvs rdiff -u -r1.3 -r1.4 src/sys/dev/ata/ata_recovery.c
cvs rdiff -u -r1.114 -r1.115 src/sys/dev/ata/ata_wdc.c
cvs rdiff -u -r1.104 -r1.105 src/sys/dev/ata/atavar.h
cvs rdiff -u -r1.15 -r1.16 src/sys/dev/ata/satapmp_subr.c
cvs rdiff -u -r1.461 -r1.462 src/sys/dev/ata/wd.c
cvs rdiff -u -r1.82 -r1.83 src/sys/dev/ic/ahcisata_core.c
cvs rdiff -u -r1.55 -r1.56 src/sys/dev/ic/mvsata.c
cvs rdiff -u -r1.41 -r1.42 src/sys/dev/ic/siisata.c
cvs rdiff -u -r1.298 -r1.299 src/sys/dev/ic/wdc.c
cvs rdiff -u -r1.99 -r1.100 src/sys/dev/ic/wdcvar.h
cvs rdiff -u -r1.137 -r1.138 src/sys/dev/scsipi/atapi_wdc.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
State-Changed-From-To: open->feedback
State-Changed-By: jdolecek@NetBSD.org
State-Changed-When: Mon, 13 Apr 2020 10:52:08 +0000
State-Changed-Why:
This should be fixed now.
State-Changed-From-To: feedback->closed
State-Changed-By: skrll@NetBSD.org
State-Changed-When: Mon, 13 Apr 2020 12:04:06 +0000
State-Changed-Why:
Fixed.
Thanks for the quick turnaround.
From: "Martin Husemann" <martin@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc:
Subject: PR/55169 CVS commit: [netbsd-9] src/sys/dev
Date: Fri, 30 Dec 2022 14:39:10 +0000
Module Name: src
Committed By: martin
Date: Fri Dec 30 14:39:10 UTC 2022
Modified Files:
src/sys/dev/ata [netbsd-9]: ata.c ata_recovery.c ata_wdc.c atavar.h
satapmp_subr.c wd.c
src/sys/dev/ic [netbsd-9]: ahcisata_core.c mvsata.c siisata.c wdc.c
wdcvar.h
src/sys/dev/scsipi [netbsd-9]: atapi_wdc.c
src/sys/dev/usb [netbsd-9]: umass_isdata.c
Log Message:
Pull up following revision(s) (requested by tsutsui in ticket #1557):
sys/dev/ic/ahcisata_core.c: revision 1.83
sys/dev/ic/ahcisata_core.c: revision 1.102
sys/dev/ata/ata.c: revision 1.164
sys/dev/ata/ata_wdc.c: revision 1.115
sys/dev/ata/ata_recovery.c: revision 1.4
sys/dev/ic/siisata.c: revision 1.42
sys/dev/ic/wdc.c: revision 1.308
sys/dev/ic/mvsata.c: revision 1.56
sys/dev/scsipi/atapi_wdc.c: revision 1.138
sys/dev/ic/siisata.c: revision 1.49
sys/dev/ata/atavar.h: revision 1.105
sys/dev/ata/wd.c: revision 1.460
sys/dev/ata/ata.c: revision 1.155
sys/dev/ata/wd.c: revision 1.462
sys/dev/ata/atavar.h: revision 1.109
sys/dev/ata/satapmp_subr.c: revision 1.16
sys/dev/ic/wdc.c: revision 1.299
sys/dev/ic/ahcisata_core.c: revision 1.93
sys/dev/ata/ata_wdc.c: revision 1.120
sys/dev/ic/wdcvar.h: revision 1.100
sys/dev/scsipi/atapi_wdc.c: revision 1.141
sys/dev/ic/mvsata.c: revision 1.61
sys/dev/usb/umass_isdata.c (apply patch)
drop wd lock in wdstart1() before calling the ata_bio hook; when called
from ata thread context, that can still need to sleep for wdc attachments
in wdcwait()
fix use-after-free for ata xfer on bio submission found by KASAN
driver ata_bio hooks read parts of the xfer after ata_exec_xfer()
call in order to determine return value, change so that the hook
doesn't return any value - callers do not care already,
as all I/O requests are asynchronous
this problem was uncovered by recent change for wd(4) to not hold
wd mutex during ata_bio call, the interrupt for the xfer might
thus actually fire immediately
adjust also ata_exec_command driver hooks similarily - remove all
completion and waiting logic from drivers, upper layer ata code
using AT_WAIT/AT_POLL changed to call ata_wait_cmd() itself
PR kern/55169 by Nick Hudson
Function declaration formating whitespace consistency. NFCI.
PR kern/56403
Fix kernel freeze for wdc(4) variants with ATAC_CAP_NOIRQ:
(1) Change ata_xfer_ops:c_poll from void to int function. When it returns
ATAPOLL_AGAIN, let ata_xfer_start() iterate itself again.
(2) Let wdc_ata_bio_poll() return ATAPOLL_AGAIN until ATA_ITSDONE is
achieved.
A similar change has been made for mvsata(4) (see mvsata_bio_poll()),
and no functional changes for other devices.
This is how the drivers worked before jdolecek-ncq branch was merged.
Note that this changes are less likely to cause infinite recursion:
(1) wdc_ata_bio_intr() called from wdc_ata_bio_poll() asserts ATA_ITSDONE
in its error handling paths via wdc_ata_bio_done().
(2) Return value from c_start (= wdc_ata_bio_start()) is checked in
ata_xfer_start().
Therefore, errors encountered in ata_xfer_ops:c_poll and c_start routines
terminate the recursion for wdc(4). The situation is similar for mvsata(4).
Still, there is a possibility where ata_xfer_start() takes long time to
finish a normal operation. This can result in a delayed response for lower
priority interrupts. But, I've never observed such a situation, even when
heavy thrashing takes place for swap partition in wd(4).
"Go ahead" by jdolecek@.
To generate a diff of this commit:
cvs rdiff -u -r1.149.2.2 -r1.149.2.3 src/sys/dev/ata/ata.c
cvs rdiff -u -r1.2.8.1 -r1.2.8.2 src/sys/dev/ata/ata_recovery.c
cvs rdiff -u -r1.113 -r1.113.4.1 src/sys/dev/ata/ata_wdc.c
cvs rdiff -u -r1.103 -r1.103.4.1 src/sys/dev/ata/atavar.h
cvs rdiff -u -r1.15 -r1.15.4.1 src/sys/dev/ata/satapmp_subr.c
cvs rdiff -u -r1.452.2.2 -r1.452.2.3 src/sys/dev/ata/wd.c
cvs rdiff -u -r1.75.4.4 -r1.75.4.5 src/sys/dev/ic/ahcisata_core.c
cvs rdiff -u -r1.48 -r1.48.2.1 src/sys/dev/ic/mvsata.c
cvs rdiff -u -r1.39 -r1.39.4.1 src/sys/dev/ic/siisata.c
cvs rdiff -u -r1.291.4.1 -r1.291.4.2 src/sys/dev/ic/wdc.c
cvs rdiff -u -r1.98.10.1 -r1.98.10.2 src/sys/dev/ic/wdcvar.h
cvs rdiff -u -r1.133 -r1.133.4.1 src/sys/dev/scsipi/atapi_wdc.c
cvs rdiff -u -r1.42 -r1.42.4.1 src/sys/dev/usb/umass_isdata.c
Please note that diffs are not public domain; they are subject to the
copyright notices on the relevant files.
>Unformatted:
NetBSD 9.99.56 (GENERIC64_TX1) #0: Sun Apr 12 10:00:26 BST 2020
nick@zoom:/home/nick/netbsd/trunk/obj.evbarm64-el/sys/arch/evbarm/compile/GENERIC64_TX1
(Contact us)
$NetBSD: query-full-pr,v 1.47 2022/09/11 19:34:41 kim Exp $
$NetBSD: gnats_config.sh,v 1.9 2014/08/02 14:16:04 spz Exp $
Copyright © 1994-2022
The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.