[v2] usb: storage: add error handling for kcalloc
From: Alan Stern <stern@rowland.harvard.edu>
Date: 2018-06-25 15:22:40
Also in:
lkml
On Mon, 25 Jun 2018, Greg Kroah-Hartman wrote:
On Thu, Jun 14, 2018 at 09:29:11PM +0800, Zhouyang Jia wrote:quoted
When kcalloc fails, the lack of error-handling code may cause unexpected results. This patch adds error-handling code after calling kcalloc. Signed-off-by: Zhouyang Jia <redacted> Acked-by: Alan Stern <stern@rowland.harvard.edu> --- v1->v2: - Remove pr_warn statement. --- drivers/usb/storage/alauda.c | 3 +++ 1 file changed, 3 insertions(+)diff --git a/drivers/usb/storage/alauda.c b/drivers/usb/storage/alauda.c index 900591d..4e17609 100644 --- a/drivers/usb/storage/alauda.c +++ b/drivers/usb/storage/alauda.c@@ -437,6 +437,9 @@ static int alauda_init_media(struct us_data *us) + MEDIA_INFO(us).blockshift + MEDIA_INFO(us).pageshift); MEDIA_INFO(us).pba_to_lba = kcalloc(num_zones, sizeof(u16*), GFP_NOIO); MEDIA_INFO(us).lba_to_pba = kcalloc(num_zones, sizeof(u16*), GFP_NOIO); + if ((MEDIA_INFO(us).pba_to_lba == NULL) + || (MEDIA_INFO(us).lba_to_pba == NULL)) + return USB_STOR_TRANSPORT_ERROR;You just leaked memory if only one of these succeeded :(
That's not really true. The memory gets deallocated later on in any case, when alauda_info_destructor() calls alauda_free_maps() if not before. More troubling is the fact that this routine (i.e, alauda_init_media) gets called from only one place, in alauda_check_media, and the caller completely ignores the return value! Furthermore, the caller always returns USB_STOR_TRANSPORT_FAILED. So on the whole, I don't think this patch is going to make any difference to the driver's operation. Alan Stern --- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html