ata: WARNING in ata_sff_qc_issue

[Dmitry Vyukov]

Hello,

The following program triggers WARNING in ata_sff_qc_issue:

// autogenerated by syzkaller (http://github.com/google/syzkaller)
#include <sys/syscall.h>
#include <stdint.h>

int main()
{
syscall(__NR_mmap, 0x20000000ul, 0xf9b000ul, 0x3ul, 0x32ul, -1, 0);
int fd = syscall(__NR_open, "/dev/sg0", 0x800142ul, 0, 0, 0, 0, 0, 0);
(*(uint32_t*)0x20007000 = (uint32_t)0x50);
(*(uint32_t*)0x20007004 = (uint32_t)0xd7b);
(*(uint64_t*)0x20007008 = (uint64_t)0x1000000000);
(*(uint32_t*)0x20007010 = (uint32_t)0x6);
(*(uint32_t*)0x20007014 = (uint32_t)0x5);
(*(uint32_t*)0x20007018 = (uint32_t)0x10001);
(*(uint32_t*)0x2000701c = (uint32_t)0x0);
(*(uint16_t*)0x20007020 = (uint16_t)0x3);
(*(uint16_t*)0x20007022 = (uint16_t)0xfffffffffffffffd);
(*(uint32_t*)0x20007024 = (uint32_t)0x8000000001885);
(*(uint32_t*)0x20007028 = (uint32_t)0x1);
(*(uint32_t*)0x2000702c = (uint32_t)0x0);
(*(uint32_t*)0x20007030 = (uint32_t)0x0);
(*(uint32_t*)0x20007034 = (uint32_t)0x0);
(*(uint32_t*)0x20007038 = (uint32_t)0x0);
(*(uint32_t*)0x2000703c = (uint32_t)0x0);
(*(uint32_t*)0x20007040 = (uint32_t)0x0);
(*(uint32_t*)0x20007044 = (uint32_t)0x0);
(*(uint32_t*)0x20007048 = (uint32_t)0x0);
(*(uint32_t*)0x2000704c = (uint32_t)0x0);
syscall(__NR_write, fd, 0x20007000ul, 0x50ul);
return 0;
}


sg_write: data in/out 3415/28 bytes for SCSI command 0x85-- guessing data in;
program a.out not setting count and/or reply_len properly
------------[ cut here ]------------
WARNING: CPU: 3 PID: 2936 at drivers/ata/libata-sff.c:1485
ata_sff_qc_issue+0x6a2/0x820 drivers/ata/libata-sff.c:1485
Kernel panic - not syncing: panic_on_warn set ...

CPU: 3 PID: 2936 Comm: a.out Not tainted 4.10.0+ #230
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Bochs 01/01/2011
Call Trace:
__dump_stack lib/dump_stack.c:15 [inline]
dump_stack+0x2ee/0x3ef lib/dump_stack.c:51
panic+0x1fb/0x412 kernel/panic.c:179
__warn+0x1c4/0x1e0 kernel/panic.c:540
warn_slowpath_null+0x2c/0x40 kernel/panic.c:583
ata_sff_qc_issue+0x6a2/0x820 drivers/ata/libata-sff.c:1485
ata_bmdma_qc_issue+0x288/0x5b0 drivers/ata/libata-sff.c:2799
ata_qc_issue+0x6f5/0x1030 drivers/ata/libata-core.c:5335
ata_scsi_translate+0x39b/0x5f0 drivers/ata/libata-scsi.c:2025
__ata_scsi_queuecmd drivers/ata/libata-scsi.c:4253 [inline]
ata_scsi_queuecmd+0x37b/0x7d0 drivers/ata/libata-scsi.c:4302
scsi_dispatch_cmd+0x43a/0xb80 drivers/scsi/scsi_lib.c:1691
scsi_request_fn+0x1071/0x1d80 drivers/scsi/scsi_lib.c:1826
__blk_run_queue_uncond block/blk-core.c:305 [inline]
__blk_run_queue+0xc5/0x130 block/blk-core.c:323
blk_execute_rq_nowait+0x31b/0x470 block/blk-exec.c:79
sg_common_write.isra.21+0x11b7/0x1c10 drivers/scsi/sg.c:803
sg_write+0x7fa/0xe90 drivers/scsi/sg.c:679
__vfs_write+0x5b1/0x740 fs/read_write.c:510
vfs_write+0x187/0x530 fs/read_write.c:560
SYSC_write fs/read_write.c:607 [inline]
SyS_write+0xfb/0x230 fs/read_write.c:599
entry_SYSCALL_64_fastpath+0x1f/0xc2
RIP: 0033:0x434b09
RSP: 002b:00007ffd3ba38278 EFLAGS: 00000203 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 0000000000401860 RCX: 0000000000434b09
RDX: 0000000000000050 RSI: 0000000020007000 RDI: 0000000000000003
RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000203 R12: 0000001000000000
R13: 0000000000401860 R14: 00000000004018f0 R15: 0000000000000000
Kernel Offset: disabled
Rebooting in 86400 seconds..


If the WARNING is merely to inform user about invalid protocol, please
issue a single line pr_err without the stack trace (invalid protocol
value may be more interesting).

On commit e5d56efc97f8240d0b5d66c03949382b6d7e5570

[Tejun Heo]

Hello,

On Tue, Feb 28, 2017 at 03:33:48PM +0100, Dmitry Vyukov wrote:
> If the WARNING is merely to inform user about invalid protocol, please
> issue a single line pr_err without the stack trace (invalid protocol
> value may be more interesting).

Yeah, the warning is harmless. The code was written before
passthrough support and just assumed that nobody higher up in the
stack should be issuing unsupported protocols and warned on such cases
as a precaution. I'm just gonna drop the WARN. The error code to sg
command should be enough to indicate the failure.

Thanks.

--
tejun

[Tejun Heo]

Hello,

Applied the following to libata/for-4.11-fixes.

Thanks.

------ 8< ------
From 0580b762a4d6b70817476b90042813f8573283fa Mon Sep 17 00:00:00 2001
From: Tejun Heo <t...@kernel.org>
Date: Mon, 6 Mar 2017 15:26:54 -0500
Subject: [PATCH] libata: drop WARN from protocol error in ata_sff_qc_issue()

ata_sff_qc_issue() expects upper layers to never issue commands on a
command protocol that it doesn't implement. While the assumption
holds fine with the usual IO path, nothing filters based on the
command protocol in the passthrough path (which was added later),
allowing the warning to be tripped with a passthrough command with the
right (well, wrong) protocol.

Failing with AC_ERR_SYSTEM is the right thing to do anyway. Remove
the unnecessary WARN.

Reported-by: Dmitry Vyukov <dvy...@google.com>
Link: http://lkml.kernel.org/r/CACT4Y+bXkvevNZU8uP6X0QVq...@mail.gmail.com
Signed-off-by: Tejun Heo <t...@kernel.org>
---
drivers/ata/libata-sff.c | 1 -
1 file changed, 1 deletion(-)

diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
index 2bd92dc..274d6d7 100644
--- a/drivers/ata/libata-sff.c
+++ b/drivers/ata/libata-sff.c
@@ -1482,7 +1482,6 @@ unsigned int ata_sff_qc_issue(struct ata_queued_cmd *qc)
break;

default:
- WARN_ON_ONCE(1);
return AC_ERR_SYSTEM;
}

--
2.9.3