From: Alan Stern <stern@rowland.harvard.edu>
To: Aleksandr Nogikh <nogikh@google.com>
Cc: syzbot <syzbot+28748250ab47a8f04100@syzkaller.appspotmail.com>,
bvanassche@acm.org, emilne@redhat.com,
gregkh@linuxfoundation.org, linux-kernel@vger.kernel.org,
linux-usb@vger.kernel.org, martin.petersen@oracle.com,
syzkaller-bugs@googlegroups.com, tasos@tasossah.com,
usb-storage@lists.one-eyed-alien.net
Subject: Re: [syzbot] [usb-storage?] divide error in isd200_ata_command
Date: Wed, 28 Feb 2024 14:18:00 -0500 [thread overview]
Message-ID: <380909e4-6e0a-402f-b3ac-de07e520c910@rowland.harvard.edu> (raw)
In-Reply-To: <CANp29Y4DUvL5zsnqQmhPGkbc=EN6UjFrWF9EZGE5U_=0C9+1Nw@mail.gmail.com>
On Wed, Feb 28, 2024 at 05:52:50PM +0100, Aleksandr Nogikh wrote:
> Hi Alan,
>
> Please try it once more with the full commit hash.
Thanks for the advice. Are you a good person to complain to about the
difference between what syzbot provides and what it will accept? This
bug report states
HEAD commit: f2e367d6ad3b Merge tag 'for-6.8/dm-fix-3' of git://git.ker..
git tree: upstream
But if I specify "upstream" as the git tree on a syz test request, it
doesn't accept it. Now you're suggesting that if I put f2e367d6ad3b as
the commit ID, it won't accept it.
There's probably already a bugfix request for this, but I'd like to push
on it some more. Syzbot's output should be acceptable as its input!
Okay, here goes with the full commit ID...
Alan Stern
On Mon, Feb 26, 2024 at 01:42:26AM -0800, syzbot wrote:
> Hello,
>
> syzbot found the following issue on:
>
> HEAD commit: f2e367d6ad3b Merge tag 'for-6.8/dm-fix-3' of git://git.ker..
> git tree: upstream
> console+strace: https://syzkaller.appspot.com/x/log.txt?x=114e10e4180000
> kernel config: https://syzkaller.appspot.com/x/.config?x=eff9f3183d0a20dd
> dashboard link: https://syzkaller.appspot.com/bug?extid=28748250ab47a8f04100
> compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1064b372180000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=10aca6ac180000
>
> Downloadable assets:
> disk image: https://storage.googleapis.com/syzbot-assets/c55ca1fdc5ad/disk-f2e367d6.raw.xz
> vmlinux: https://storage.googleapis.com/syzbot-assets/4556a82fb4ed/vmlinux-f2e367d6.xz
> kernel image: https://storage.googleapis.com/syzbot-assets/95338ed9dad1/bzImage-f2e367d6.xz
>
> The issue was bisected to:
>
> commit 321da3dc1f3c92a12e3c5da934090d2992a8814c
> Author: Martin K. Petersen <martin.petersen@oracle.com>
> Date: Tue Feb 13 14:33:06 2024 +0000
>
> scsi: sd: usb_storage: uas: Access media prior to querying device properties
>
> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=15a3934a180000
> final oops: https://syzkaller.appspot.com/x/report.txt?x=17a3934a180000
> console output: https://syzkaller.appspot.com/x/log.txt?x=13a3934a180000
>
> IMPORTANT: if you fix the issue, please add the following tag to the commit:
> Reported-by: syzbot+28748250ab47a8f04100@syzkaller.appspotmail.com
> Fixes: 321da3dc1f3c ("scsi: sd: usb_storage: uas: Access media prior to querying device properties")
>
> divide error: 0000 [#1] PREEMPT SMP KASAN PTI
> CPU: 0 PID: 5070 Comm: usb-storage Not tainted 6.8.0-rc5-syzkaller-00297-gf2e367d6ad3b #0
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/25/2024
> RIP: 0010:isd200_scsi_to_ata drivers/usb/storage/isd200.c:1318 [inline]
> RIP: 0010:isd200_ata_command+0x776/0x2380 drivers/usb/storage/isd200.c:1529
#syz test: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/ f2e367d6ad3bdc527c2b14e759c2f010d6b2b7a1
Index: usb-devel/drivers/usb/storage/isd200.c
===================================================================
--- usb-devel.orig/drivers/usb/storage/isd200.c
+++ usb-devel/drivers/usb/storage/isd200.c
@@ -1105,7 +1105,7 @@ static void isd200_dump_driveid(struct u
static int isd200_get_inquiry_data( struct us_data *us )
{
struct isd200_info *info = (struct isd200_info *)us->extra;
- int retStatus = ISD200_GOOD;
+ int retStatus;
u16 *id = info->id;
usb_stor_dbg(us, "Entering isd200_get_inquiry_data\n");
@@ -1137,6 +1137,13 @@ static int isd200_get_inquiry_data( stru
isd200_fix_driveid(id);
isd200_dump_driveid(us, id);
+ /* Prevent division by 0 in isd200_scsi_to_ata() */
+ if (id[ATA_ID_HEADS] == 0 || id[ATA_ID_SECTORS] == 0) {
+ usb_stor_dbg(us, " Invalid ATA Identify data\n");
+ retStatus = ISD200_ERROR;
+ goto Done;
+ }
+
memset(&info->InquiryData, 0, sizeof(info->InquiryData));
/* Standard IDE interface only supports disks */
@@ -1202,6 +1209,7 @@ static int isd200_get_inquiry_data( stru
}
}
+ Done:
usb_stor_dbg(us, "Leaving isd200_get_inquiry_data %08X\n", retStatus);
return(retStatus);
@@ -1481,22 +1489,27 @@ static int isd200_init_info(struct us_da
static int isd200_Initialization(struct us_data *us)
{
+ int rc = 0;
+
usb_stor_dbg(us, "ISD200 Initialization...\n");
/* Initialize ISD200 info struct */
- if (isd200_init_info(us) == ISD200_ERROR) {
+ if (isd200_init_info(us) < 0) {
usb_stor_dbg(us, "ERROR Initializing ISD200 Info struct\n");
+ rc = -ENOMEM;
} else {
/* Get device specific data */
- if (isd200_get_inquiry_data(us) != ISD200_GOOD)
+ if (isd200_get_inquiry_data(us) != ISD200_GOOD) {
usb_stor_dbg(us, "ISD200 Initialization Failure\n");
- else
+ rc = -EINVAL;
+ } else {
usb_stor_dbg(us, "ISD200 Initialization complete\n");
+ }
}
- return 0;
+ return rc;
}
next prev parent reply other threads:[~2024-02-28 19:18 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-26 9:42 [syzbot] [usb-storage?] divide error in isd200_ata_command syzbot
2024-02-26 10:59 ` Oliver Neukum
2024-02-26 18:13 ` Alan Stern
2024-02-27 2:46 ` Martin K. Petersen
2024-02-27 3:05 ` Alan Stern
2024-02-28 16:20 ` Oliver Neukum
2024-02-28 19:23 ` Alan Stern
2024-02-27 19:36 ` Alan Stern
2024-02-28 5:20 ` syzbot
2024-02-28 16:12 ` Alan Stern
2024-02-28 16:13 ` syzbot
2024-02-28 16:52 ` Aleksandr Nogikh
2024-02-28 19:18 ` Alan Stern [this message]
2024-02-29 0:33 ` syzbot
2024-02-29 19:30 ` [PATCH] USB: usb-storage: Prevent divide-by-0 " Alan Stern
2024-03-01 1:11 ` Martin K. Petersen
2024-03-01 11:36 ` PrasannaKumar Muralidharan
2024-02-29 15:40 ` [syzbot] [usb-storage?] divide " Aleksandr Nogikh
2024-02-29 16:12 ` Alan Stern
2024-02-29 17:57 ` Aleksandr Nogikh
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=380909e4-6e0a-402f-b3ac-de07e520c910@rowland.harvard.edu \
--to=stern@rowland.harvard.edu \
--cc=bvanassche@acm.org \
--cc=emilne@redhat.com \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=martin.petersen@oracle.com \
--cc=nogikh@google.com \
--cc=syzbot+28748250ab47a8f04100@syzkaller.appspotmail.com \
--cc=syzkaller-bugs@googlegroups.com \
--cc=tasos@tasossah.com \
--cc=usb-storage@lists.one-eyed-alien.net \
/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;
as well as URLs for NNTP newsgroup(s).