• Eric Biggers's avatar
    libata: fix length validation of ATAPI-relayed SCSI commands · 058f58e2
    Eric Biggers authored
    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 607126c2 ("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: 607126c2 ("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: default avatarEric Biggers <ebiggers@google.com>
    Signed-off-by: default avatarTejun Heo <tj@kernel.org>
    058f58e2
libata-scsi.c 127 KB