Source-Changes-HG archive
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]
[src/trunk]: src/sys/dev fix use-after-free for ata xfer on bio submission fo...
details: https://anonhg.NetBSD.org/src/rev/65dbf3b670cb
branches: trunk
changeset: 1009121:65dbf3b670cb
user: jdolecek <jdolecek%NetBSD.org@localhost>
date: Mon Apr 13 10:49:34 2020 +0000
description:
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
diffstat:
sys/dev/ata/ata.c | 20 +++-------
sys/dev/ata/ata_recovery.c | 12 ++---
sys/dev/ata/ata_wdc.c | 12 ++---
sys/dev/ata/atavar.h | 6 +-
sys/dev/ata/satapmp_subr.c | 26 +++++--------
sys/dev/ata/wd.c | 92 +++++++++++++++------------------------------
sys/dev/ic/ahcisata_core.c | 34 +++-------------
sys/dev/ic/mvsata.c | 32 +++------------
sys/dev/ic/siisata.c | 38 ++++---------------
sys/dev/ic/wdc.c | 25 +-----------
sys/dev/ic/wdcvar.h | 4 +-
sys/dev/scsipi/atapi_wdc.c | 14 +++----
12 files changed, 93 insertions(+), 222 deletions(-)
diffs (truncated from 804 to 300 lines):
diff -r 3f756fb56bb8 -r 65dbf3b670cb sys/dev/ata/ata.c
--- a/sys/dev/ata/ata.c Mon Apr 13 09:34:02 2020 +0000
+++ b/sys/dev/ata/ata.c Mon Apr 13 10:49:34 2020 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ata.c,v 1.154 2020/04/04 21:36:15 jdolecek Exp $ */
+/* $NetBSD: ata.c,v 1.155 2020/04/13 10:49:34 jdolecek Exp $ */
/*
* Copyright (c) 1998, 2001 Manuel Bouyer. All rights reserved.
@@ -25,7 +25,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ata.c,v 1.154 2020/04/04 21:36:15 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata.c,v 1.155 2020/04/13 10:49:34 jdolecek Exp $");
#include "opt_ata.h"
@@ -847,13 +847,8 @@
xfer->c_ata_c.flags = AT_READ | flags;
xfer->c_ata_c.data = tb;
xfer->c_ata_c.bcount = ATA_BSIZE;
- if ((*atac->atac_bustype_ata->ata_exec_command)(drvp,
- xfer) != ATACMD_COMPLETE) {
- ATADEBUG_PRINT(("ata_get_parms: wdc_exec_command failed\n"),
- DEBUG_FUNCS|DEBUG_PROBE);
- rv = CMD_AGAIN;
- goto out;
- }
+ (*atac->atac_bustype_ata->ata_exec_command)(drvp, xfer);
+ ata_wait_cmd(chp, xfer);
if (xfer->c_ata_c.flags & (AT_ERROR | AT_TIMEOU | AT_DF)) {
ATADEBUG_PRINT(("ata_get_parms: ata_c.flags=0x%x\n",
xfer->c_ata_c.flags), DEBUG_FUNCS|DEBUG_PROBE);
@@ -937,11 +932,8 @@
xfer->c_ata_c.r_count = mode;
xfer->c_ata_c.flags = flags;
xfer->c_ata_c.timeout = 1000; /* 1s */
- if ((*atac->atac_bustype_ata->ata_exec_command)(drvp,
- xfer) != ATACMD_COMPLETE) {
- rv = CMD_AGAIN;
- goto out;
- }
+ (*atac->atac_bustype_ata->ata_exec_command)(drvp, xfer);
+ ata_wait_cmd(chp, xfer);
if (xfer->c_ata_c.flags & (AT_ERROR | AT_TIMEOU | AT_DF)) {
rv = CMD_ERR;
goto out;
diff -r 3f756fb56bb8 -r 65dbf3b670cb sys/dev/ata/ata_recovery.c
--- a/sys/dev/ata/ata_recovery.c Mon Apr 13 09:34:02 2020 +0000
+++ b/sys/dev/ata/ata_recovery.c Mon Apr 13 10:49:34 2020 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ata_recovery.c,v 1.3 2020/04/04 22:30:02 jdolecek Exp $ */
+/* $NetBSD: ata_recovery.c,v 1.4 2020/04/13 10:49:34 jdolecek Exp $ */
/*-
* Copyright (c) 2018 The NetBSD Foundation, Inc.
@@ -27,7 +27,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ata_recovery.c,v 1.3 2020/04/04 22:30:02 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata_recovery.c,v 1.4 2020/04/13 10:49:34 jdolecek Exp $");
#include "opt_ata.h"
@@ -103,11 +103,9 @@
xfer->c_ata_c.data = tb;
xfer->c_ata_c.bcount = sizeof(chp->recovery_blk);
- if ((*atac->atac_bustype_ata->ata_exec_command)(drvp,
- xfer) != ATACMD_COMPLETE) {
- rv = EAGAIN;
- goto out;
- }
+ (*atac->atac_bustype_ata->ata_exec_command)(drvp, xfer);
+ ata_wait_cmd(chp, xfer);
+
if (xfer->c_ata_c.flags & (AT_ERROR | AT_TIMEOU | AT_DF)) {
rv = EINVAL;
goto out;
diff -r 3f756fb56bb8 -r 65dbf3b670cb sys/dev/ata/ata_wdc.c
--- a/sys/dev/ata/ata_wdc.c Mon Apr 13 09:34:02 2020 +0000
+++ b/sys/dev/ata/ata_wdc.c Mon Apr 13 10:49:34 2020 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: ata_wdc.c,v 1.114 2020/04/04 21:36:15 jdolecek Exp $ */
+/* $NetBSD: ata_wdc.c,v 1.115 2020/04/13 10:49:34 jdolecek Exp $ */
/*
* Copyright (c) 1998, 2001, 2003 Manuel Bouyer.
@@ -54,7 +54,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: ata_wdc.c,v 1.114 2020/04/04 21:36:15 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: ata_wdc.c,v 1.115 2020/04/13 10:49:34 jdolecek Exp $");
#include "opt_ata.h"
#include "opt_wdc.h"
@@ -102,7 +102,7 @@
#define ATA_DELAY 10000 /* 10s for a drive I/O */
-static int wdc_ata_bio(struct ata_drive_datas*, struct ata_xfer *);
+static void wdc_ata_bio(struct ata_drive_datas*, struct ata_xfer *);
static int wdc_ata_bio_start(struct ata_channel *,struct ata_xfer *);
static int _wdc_ata_bio_start(struct ata_channel *,struct ata_xfer *);
static void wdc_ata_bio_poll(struct ata_channel *,struct ata_xfer *);
@@ -140,10 +140,9 @@
};
/*
- * Handle block I/O operation. Return ATACMD_COMPLETE, ATACMD_QUEUED, or
- * ATACMD_TRY_AGAIN. Must be called at splbio().
+ * Handle block I/O operation.
*/
-static int
+static void
wdc_ata_bio(struct ata_drive_datas *drvp, struct ata_xfer *xfer)
{
struct ata_channel *chp = drvp->chnl_softc;
@@ -171,7 +170,6 @@
xfer->c_bcount = ata_bio->bcount;
xfer->ops = &wdc_bio_xfer_ops;
ata_exec_xfer(chp, xfer);
- return (ata_bio->flags & ATA_ITSDONE) ? ATACMD_COMPLETE : ATACMD_QUEUED;
}
static int
diff -r 3f756fb56bb8 -r 65dbf3b670cb sys/dev/ata/atavar.h
--- a/sys/dev/ata/atavar.h Mon Apr 13 09:34:02 2020 +0000
+++ b/sys/dev/ata/atavar.h Mon Apr 13 10:49:34 2020 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: atavar.h,v 1.104 2020/04/04 21:36:15 jdolecek Exp $ */
+/* $NetBSD: atavar.h,v 1.105 2020/04/13 10:49:34 jdolecek Exp $ */
/*
* Copyright (c) 1998, 2001 Manuel Bouyer.
@@ -358,10 +358,10 @@
*/
struct ata_bustype {
int bustype_type; /* symbolic name of type */
- int (*ata_bio)(struct ata_drive_datas *, struct ata_xfer *);
+ void (*ata_bio)(struct ata_drive_datas *, struct ata_xfer *);
void (*ata_reset_drive)(struct ata_drive_datas *, int, uint32_t *);
void (*ata_reset_channel)(struct ata_channel *, int);
- int (*ata_exec_command)(struct ata_drive_datas *,
+ void (*ata_exec_command)(struct ata_drive_datas *,
struct ata_xfer *);
#define ATACMD_COMPLETE 0x01
diff -r 3f756fb56bb8 -r 65dbf3b670cb sys/dev/ata/satapmp_subr.c
--- a/sys/dev/ata/satapmp_subr.c Mon Apr 13 09:34:02 2020 +0000
+++ b/sys/dev/ata/satapmp_subr.c Mon Apr 13 10:49:34 2020 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: satapmp_subr.c,v 1.15 2018/10/22 20:13:47 jdolecek Exp $ */
+/* $NetBSD: satapmp_subr.c,v 1.16 2020/04/13 10:49:34 jdolecek Exp $ */
/*
* Copyright (c) 2012 Manuel Bouyer. All rights reserved.
@@ -25,7 +25,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: satapmp_subr.c,v 1.15 2018/10/22 20:13:47 jdolecek Exp $");
+__KERNEL_RCSID(0, "$NetBSD: satapmp_subr.c,v 1.16 2020/04/13 10:49:34 jdolecek Exp $");
#include <sys/param.h>
#include <sys/systm.h>
@@ -72,13 +72,10 @@
xfer->c_ata_c.flags = AT_LBA48 | AT_READREG | AT_WAIT;
ata_channel_unlock(chp);
- if ((*atac->atac_bustype_ata->ata_exec_command)(drvp,
- xfer) != ATACMD_COMPLETE) {
- aprint_error_dev(chp->atabus,
- "PMP port %d register %d read failed\n", port, reg);
- error = EIO;
- goto out;
- }
+
+ (*atac->atac_bustype_ata->ata_exec_command)(drvp, xfer);
+ ata_wait_cmd(chp, xfer);
+
if (xfer->c_ata_c.flags & (AT_TIMEOU | AT_DF)) {
aprint_error_dev(chp->atabus,
"PMP port %d register %d read failed, flags 0x%x\n",
@@ -148,13 +145,10 @@
xfer->c_ata_c.flags = AT_LBA48 | AT_WAIT;
ata_channel_unlock(chp);
- if ((*atac->atac_bustype_ata->ata_exec_command)(drvp,
- xfer) != ATACMD_COMPLETE) {
- aprint_error_dev(chp->atabus,
- "PMP port %d register %d write failed\n", port, reg);
- error = EIO;
- goto out;
- }
+
+ (*atac->atac_bustype_ata->ata_exec_command)(drvp, xfer);
+ ata_wait_cmd(chp, xfer);
+
if (xfer->c_ata_c.flags & (AT_TIMEOU | AT_DF)) {
aprint_error_dev(chp->atabus,
"PMP port %d register %d write failed, flags 0x%x\n",
diff -r 3f756fb56bb8 -r 65dbf3b670cb sys/dev/ata/wd.c
--- a/sys/dev/ata/wd.c Mon Apr 13 09:34:02 2020 +0000
+++ b/sys/dev/ata/wd.c Mon Apr 13 10:49:34 2020 +0000
@@ -1,4 +1,4 @@
-/* $NetBSD: wd.c,v 1.461 2020/04/13 08:05:02 maxv Exp $ */
+/* $NetBSD: wd.c,v 1.462 2020/04/13 10:49:34 jdolecek Exp $ */
/*
* Copyright (c) 1998, 2001 Manuel Bouyer. All rights reserved.
@@ -54,7 +54,7 @@
*/
#include <sys/cdefs.h>
-__KERNEL_RCSID(0, "$NetBSD: wd.c,v 1.461 2020/04/13 08:05:02 maxv Exp $");
+__KERNEL_RCSID(0, "$NetBSD: wd.c,v 1.462 2020/04/13 10:49:34 jdolecek Exp $");
#include "opt_ata.h"
#include "opt_wd.h"
@@ -201,7 +201,7 @@
static void wdbioretry(void *);
static void wdbiorequeue(void *);
static void wddone(device_t, struct ata_xfer *);
-static int wd_get_params(struct wd_softc *, uint8_t, struct ataparams *);
+static int wd_get_params(struct wd_softc *, struct ataparams *);
static void wd_set_geometry(struct wd_softc *);
static int wd_flushcache(struct wd_softc *, int, bool);
static int wd_trim(struct wd_softc *, daddr_t, long);
@@ -336,7 +336,7 @@
aprint_normal("\n");
/* read our drive info */
- if (wd_get_params(wd, AT_WAIT, &wd->sc_params) != 0) {
+ if (wd_get_params(wd, &wd->sc_params) != 0) {
aprint_error_dev(self, "IDENTIFY failed\n");
goto out;
}
@@ -760,16 +760,8 @@
wd->inflight++;
mutex_exit(&wd->sc_lock);
- switch (wd->atabus->ata_bio(wd->drvp, xfer)) {
- case ATACMD_TRY_AGAIN:
- panic("wdstart1: try again");
- break;
- case ATACMD_QUEUED:
- case ATACMD_COMPLETE:
- break;
- default:
- panic("wdstart1: bad return code from ata_bio()");
- }
+ /* Queue the xfer */
+ wd->atabus->ata_bio(wd->drvp, xfer);
mutex_enter(&wd->sc_lock);
}
@@ -1165,7 +1157,7 @@
int param_error;
/* Load the physical device parameters. */
- param_error = wd_get_params(wd, AT_WAIT, &wd->sc_params);
+ param_error = wd_get_params(wd, &wd->sc_params);
if (param_error != 0) {
aprint_error_dev(dksc->sc_dev, "IDENTIFY failed\n");
error = EIO;
@@ -1609,18 +1601,9 @@
xfer->c_bio.bcount = nblk * dg->dg_secsize;
xfer->c_bio.databuf = va;
#ifndef WD_DUMP_NOT_TRUSTED
- switch (err = wd->atabus->ata_bio(wd->drvp, xfer)) {
- case ATACMD_TRY_AGAIN:
- panic("wddump: try again");
- break;
- case ATACMD_QUEUED:
- panic("wddump: polled command has been queued");
- break;
- case ATACMD_COMPLETE:
- break;
- default:
- panic("wddump: unknown atacmd code %d", err);
- }
+ /* This will poll until the bio is complete */
+ wd->atabus->ata_bio(wd->drvp, xfer);
+
switch(err = xfer->c_bio.error) {
case TIMEOUT:
printf("wddump: device timed out");
@@ -1707,10 +1690,11 @@
}
int
-wd_get_params(struct wd_softc *wd, uint8_t flags, struct ataparams *params)
+wd_get_params(struct wd_softc *wd, struct ataparams *params)
{
int retry = 0;
struct ata_channel *chp = wd->drvp->chnl_softc;
Home |
Main Index |
Thread Index |
Old Index