From: Eric Biggers <ebiggers3@gmail.com>
To: linux-ide@vger.kernel.org, Tejun Heo <tj@kernel.org>
Cc: linux-scsi@vger.kernel.org, syzkaller-bugs@googlegroups.com,
Vinayak Kale <vinayak.kale@seagate.com>,
Eric Biggers <ebiggers@google.com>
Subject: [PATCH] libata: don't try to pass through NCQ commands to non-NCQ devices
Date: Sat, 3 Feb 2018 20:33:51 -0800 [thread overview]
Message-ID: <20180204043351.16812-1-ebiggers3@gmail.com> (raw)
In-Reply-To: <94eb2c1fae0400ce57055cd4e9bb@google.com>
From: Eric Biggers <ebiggers@google.com>
syzkaller hit a WARN() in ata_bmdma_qc_issue() when writing to /dev/sg0.
This happened because it issued an ATA pass-through command (ATA_16)
where the protocol field indicated that NCQ should be used -- but the
device did not support NCQ.
We could just remove the WARN() from libata-sff.c, but the real problem
seems to be that the SCSI -> ATA translation code passes through NCQ
commands without verifying that the device actually supports NCQ.
Fix this by adding the appropriate check to ata_scsi_pass_thru().
Here's reproducer that works in QEMU when /dev/sg0 refers to a disk of
the default type ("82371SB PIIX3 IDE"):
#include <fcntl.h>
#include <unistd.h>
int main()
{
char buf[53] = { 0 };
buf[36] = 0x85; /* ATA_16 */
buf[37] = (12 << 1); /* FPDMA */
buf[38] = 0x1; /* Has data */
buf[51] = 0xC8; /* ATA_CMD_READ */
write(open("/dev/sg0", O_RDWR), buf, sizeof(buf));
}
Fixes: ee7fb331c3ac ("libata: add support for NCQ commands for SG interface")
Reported-by: syzbot+2f69ca28df61bdfc77cd36af2e789850355a221e@syzkaller.appspotmail.com
Cc: <stable@vger.kernel.org> # v4.4+
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
drivers/ata/libata-scsi.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index 47d421666451c..197e110f8ac75 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3316,6 +3316,12 @@ static unsigned int ata_scsi_pass_thru(struct ata_queued_cmd *qc)
goto invalid_fld;
}
+ /* We may not issue NCQ commands to devices not supporting NCQ */
+ if (ata_is_ncq(tf->protocol) && !ata_ncq_enabled(dev)) {
+ fp = 1;
+ goto invalid_fld;
+ }
+
/* sanity check for pio multi commands */
if ((cdb[1] & 0xe0) && !is_multi_taskfile(tf)) {
fp = 1;
--
2.16.1
next prev parent reply other threads:[~2018-02-04 4:34 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-31 10:06 WARNING in ata_bmdma_qc_issue syzbot
2018-02-04 4:33 ` Eric Biggers [this message]
2018-02-12 17:21 ` [PATCH] libata: don't try to pass through NCQ commands to non-NCQ devices Tejun Heo
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20180204043351.16812-1-ebiggers3@gmail.com \
--to=ebiggers3@gmail.com \
--cc=ebiggers@google.com \
--cc=linux-ide@vger.kernel.org \
--cc=linux-scsi@vger.kernel.org \
--cc=syzkaller-bugs@googlegroups.com \
--cc=tj@kernel.org \
--cc=vinayak.kale@seagate.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.