From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: linux-kernel@vger.kernel.org
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
stable@vger.kernel.org,
syzbot+1ff6f9fcc3c35f1c72a95e26528c8e7e3276e4da@syzkaller.appspotmail.com,
Eric Biggers <ebiggers@google.com>, Tejun Heo <tj@kernel.org>
Subject: [PATCH 4.4 12/43] libata: fix length validation of ATAPI-relayed SCSI commands
Date: Tue, 27 Mar 2018 18:27:16 +0200 [thread overview]
Message-ID: <20180327162717.069237899@linuxfoundation.org> (raw)
In-Reply-To: <20180327162716.407986916@linuxfoundation.org>
4.4-stable review patch. If anyone has any objections, please let me know.
------------------
From: Eric Biggers <ebiggers@google.com>
commit 058f58e235cbe03e923b30ea7c49995a46a8725f upstream.
syzkaller reported a crash in ata_bmdma_fill_sg() when writing to
/dev/sg1. The immediate cause was that the ATA command's scatterlist
was not DMA-mapped, which causes 'pi - 1' to underflow, resulting in a
write to 'qc->ap->bmdma_prd[0xffffffff]'.
Strangely though, the flag ATA_QCFLAG_DMAMAP was set in qc->flags. The
root cause is that when __ata_scsi_queuecmd() is preparing to relay a
SCSI command to an ATAPI device, it doesn't correctly validate the CDB
length before copying it into the 16-byte buffer 'cdb' in 'struct
ata_queued_cmd'. Namely, it validates the fixed CDB length expected
based on the SCSI opcode but not the actual CDB length, which can be
larger due to the use of the SG_NEXT_CMD_LEN ioctl. Since 'flags' is
the next member in ata_queued_cmd, a buffer overflow corrupts it.
Fix it by requiring that the actual CDB length be <= 16 (ATAPI_CDB_LEN).
[Really it seems the length should be required to be <= dev->cdb_len,
but the current behavior seems to have been intentionally introduced by
commit 607126c2a21c ("libata-scsi: be tolerant of 12-byte ATAPI commands
in 16-byte CDBs") to work around a userspace bug in mplayer. Probably
the workaround is no longer needed (mplayer was fixed in 2007), but
continuing to allow lengths to up 16 appears harmless for now.]
Here's a reproducer that works in QEMU when /dev/sg1 refers to the
CD-ROM drive that qemu-system-x86_64 creates by default:
#include <fcntl.h>
#include <sys/ioctl.h>
#include <unistd.h>
#define SG_NEXT_CMD_LEN 0x2283
int main()
{
char buf[53] = { [36] = 0x7e, [52] = 0x02 };
int fd = open("/dev/sg1", O_RDWR);
ioctl(fd, SG_NEXT_CMD_LEN, &(int){ 17 });
write(fd, buf, sizeof(buf));
}
The crash was:
BUG: unable to handle kernel paging request at ffff8cb97db37ffc
IP: ata_bmdma_fill_sg drivers/ata/libata-sff.c:2623 [inline]
IP: ata_bmdma_qc_prep+0xa4/0xc0 drivers/ata/libata-sff.c:2727
PGD fb6c067 P4D fb6c067 PUD 0
Oops: 0002 [#1] SMP
CPU: 1 PID: 150 Comm: syz_ata_bmdma_q Not tainted 4.15.0-next-20180202 #99
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-20171110_100015-anatol 04/01/2014
[...]
Call Trace:
ata_qc_issue+0x100/0x1d0 drivers/ata/libata-core.c:5421
ata_scsi_translate+0xc9/0x1a0 drivers/ata/libata-scsi.c:2024
__ata_scsi_queuecmd drivers/ata/libata-scsi.c:4326 [inline]
ata_scsi_queuecmd+0x8c/0x210 drivers/ata/libata-scsi.c:4375
scsi_dispatch_cmd+0xa2/0xe0 drivers/scsi/scsi_lib.c:1727
scsi_request_fn+0x24c/0x530 drivers/scsi/scsi_lib.c:1865
__blk_run_queue_uncond block/blk-core.c:412 [inline]
__blk_run_queue+0x3a/0x60 block/blk-core.c:432
blk_execute_rq_nowait+0x93/0xc0 block/blk-exec.c:78
sg_common_write.isra.7+0x272/0x5a0 drivers/scsi/sg.c:806
sg_write+0x1ef/0x340 drivers/scsi/sg.c:677
__vfs_write+0x31/0x160 fs/read_write.c:480
vfs_write+0xa7/0x160 fs/read_write.c:544
SYSC_write fs/read_write.c:589 [inline]
SyS_write+0x4d/0xc0 fs/read_write.c:581
do_syscall_64+0x5e/0x110 arch/x86/entry/common.c:287
entry_SYSCALL_64_after_hwframe+0x21/0x86
Fixes: 607126c2a21c ("libata-scsi: be tolerant of 12-byte ATAPI commands in 16-byte CDBs")
Reported-by: syzbot+1ff6f9fcc3c35f1c72a95e26528c8e7e3276e4da@syzkaller.appspotmail.com
Cc: <stable@vger.kernel.org> # v2.6.24+
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
drivers/ata/libata-scsi.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -3472,7 +3472,9 @@ static inline int __ata_scsi_queuecmd(st
if (likely((scsi_op != ATA_16) || !atapi_passthru16)) {
/* relay SCSI command to ATAPI device */
int len = COMMAND_SIZE(scsi_op);
- if (unlikely(len > scmd->cmd_len || len > dev->cdb_len))
+ if (unlikely(len > scmd->cmd_len ||
+ len > dev->cdb_len ||
+ scmd->cmd_len > ATAPI_CDB_LEN))
goto bad_cdb_len;
xlat_func = atapi_xlat;
next prev parent reply other threads:[~2018-03-27 16:27 UTC|newest]
Thread overview: 64+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-27 16:27 [PATCH 4.4 00/43] 4.4.125-stable review Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 01/43] MIPS: ralink: Remove ralink_halt() Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 02/43] iio: st_pressure: st_accel: pass correct platform data to init Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 03/43] ALSA: usb-audio: Fix parsing descriptor of UAC2 processing unit Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 04/43] ALSA: aloop: Sync stale timer before release Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 05/43] ALSA: aloop: Fix access to not-yet-ready substream via cable Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 06/43] ALSA: hda/realtek - Always immediately update mute LED with pin VREF Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 07/43] mmc: dw_mmc: fix falling from idmac to PIO mode when dw_mci_reset occurs Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 08/43] PCI: Add function 1 DMA alias quirk for Highpoint RocketRAID 644L Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 09/43] ahci: Add PCI-id for the Highpoint Rocketraid 644L card Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 10/43] clk: bcm2835: Protect sections updating shared registers Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 11/43] Bluetooth: btusb: Fix quirk for Atheros 1525/QCA6174 Greg Kroah-Hartman
2018-03-27 16:27 ` Greg Kroah-Hartman [this message]
2018-03-27 16:27 ` [PATCH 4.4 13/43] libata: remove WARN() for DMA or PIO command without data Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 14/43] libata: Apply NOLPM quirk to Crucial MX100 512GB SSDs Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 15/43] libata: disable LPM for Crucial BX100 SSD 500GB drive Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 16/43] libata: Enable queued TRIM for Samsung SSD 860 Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 17/43] libata: Apply NOLPM quirk to Crucial M500 480 and 960GB SSDs Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 18/43] libata: Make Crucial BX100 500GB LPM quirk apply to all firmware versions Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 19/43] libata: Modify quirks for MX100 to limit NCQ_TRIM quirk to MU01 version Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 20/43] mm/vmalloc: add interfaces to free unmapped page table Greg Kroah-Hartman
2018-03-27 20:17 ` Dan Rue
2018-03-27 20:27 ` Kani, Toshi
2018-03-27 20:31 ` Nathan Chancellor
2018-03-27 20:40 ` Kani, Toshi
2018-03-27 20:47 ` Nathan Chancellor
2018-03-28 6:32 ` gregkh
2018-03-28 6:47 ` Nathan Chancellor
2018-03-28 9:58 ` gregkh
2018-03-28 15:06 ` Kani, Toshi
2018-03-28 16:16 ` gregkh
2018-03-27 16:27 ` [PATCH 4.4 21/43] x86/mm: implement free pmd/pte page interfaces Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 22/43] drm/vmwgfx: Fix a destoy-while-held mutex problem Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 23/43] drm/radeon: Dont turn off DP sink when disconnected Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 24/43] drm: udl: Properly check framebuffer mmap offsets Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 25/43] acpi, numa: fix pxm to online numa node associations Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 26/43] brcmfmac: fix P2P_DEVICE ethernet address generation Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 27/43] rtlwifi: rtl8723be: Fix loss of signal Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 28/43] tracing: probeevent: Fix to support minus offset from symbol Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 29/43] mtd: nand: fsl_ifc: Fix nand waitfunc return value Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 30/43] staging: ncpfs: memory corruption in ncp_read_kernel() Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 31/43] can: cc770: Fix stalls on rt-linux, remove redundant IRQ ack Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 32/43] can: cc770: Fix queue stall & dropped RTR reply Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 33/43] can: cc770: Fix use after free in cc770_tx_interrupt() Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 34/43] tty: vt: fix up tabstops properly Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 35/43] kvm/x86: fix icebp instruction handling Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 36/43] x86/build/64: Force the linker to use 2MB page size Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 37/43] x86/boot/64: Verify alignment of the LOAD segment Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 38/43] x86/entry/64: Dont use IST entry for #BP stack Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 39/43] perf/x86/intel: Dont accidentally clear high bits in bdw_limit_period() Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 40/43] staging: lustre: ptlrpc: kfree used instead of kvfree Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 41/43] kbuild: disable clangs default use of -fmerge-all-constants Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 42/43] bpf: skip unnecessary capability check Greg Kroah-Hartman
2018-03-27 16:27 ` [PATCH 4.4 43/43] bpf, x64: increase number of passes Greg Kroah-Hartman
2018-03-27 18:24 ` [PATCH 4.4 00/43] 4.4.125-stable review Nathan Chancellor
2018-03-28 10:04 ` Greg Kroah-Hartman
2018-03-27 20:21 ` Dan Rue
2018-03-28 10:03 ` Greg Kroah-Hartman
2018-03-27 22:59 ` Shuah Khan
2018-03-28 0:21 ` kernelci.org bot
2018-03-30 23:56 ` Kevin Hilman
2018-03-28 9:58 ` Greg Kroah-Hartman
2018-03-28 15:42 ` Dan Rue
2018-03-28 13:38 ` Guenter Roeck
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=20180327162717.069237899@linuxfoundation.org \
--to=gregkh@linuxfoundation.org \
--cc=ebiggers@google.com \
--cc=linux-kernel@vger.kernel.org \
--cc=stable@vger.kernel.org \
--cc=syzbot+1ff6f9fcc3c35f1c72a95e26528c8e7e3276e4da@syzkaller.appspotmail.com \
--cc=tj@kernel.org \
/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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox