Messages in this thread | | | Subject | Re: [PATCH] usb: storage: sddr55: Fix a possible null-pointer dereference in sddr55_transport() | From | Jia-Ju Bai <> | Date | Mon, 29 Jul 2019 19:44:08 +0800 |
| |
On 2019/7/29 19:15, Oliver Neukum wrote: > Am Montag, den 29.07.2019, 18:05 +0800 schrieb Jia-Ju Bai: > > Hi, > >> In sddr55_transport(), there is an if statement on line 836 to check >> whether info->lba_to_pba is NULL: >> if (info->lba_to_pba == NULL || ...) >> >> When info->lba_to_pba is NULL, it is used on line 948: >> pba = info->lba_to_pba[lba]; >> >> Thus, a possible null-pointer dereference may occur. > Yes, in practice READ_CAPACITY will always be called and set > up the correct translation table, but you can probably exploit > this. > >> To fix this bug, info->lba_to_pba is checked before being used. >> >> This bug is found by a static analysis tool STCheck written by us. >> >> Signed-off-by: Jia-Ju Bai <baijiaju1990@gmail.com> >> --- >> drivers/usb/storage/sddr55.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/usb/storage/sddr55.c b/drivers/usb/storage/sddr55.c >> index b8527c55335b..50afc39aa21d 100644 >> --- a/drivers/usb/storage/sddr55.c >> +++ b/drivers/usb/storage/sddr55.c >> @@ -945,7 +945,8 @@ static int sddr55_transport(struct scsi_cmnd *srb, struct us_data *us) >> return USB_STOR_TRANSPORT_FAILED; >> } >> >> - pba = info->lba_to_pba[lba]; >> + if (info->lba_to_pba) >> + pba = info->lba_to_pba[lba]; > If you use that fix, pba will be uninitialized when used. It should be > something like: > > pba = info->lba_to_pba ? info->lba_to_pba[lba] : 0;
Thanks for the advice. I will send a v2 patch.
Best wishes, Jia-Ju Bai
| |