* [PATCH] usb: storage: add error handling for kcalloc @ 2018-06-11 8:52 Zhouyang Jia 2018-06-12 14:31 ` Alan Stern 2018-06-14 13:29 ` [PATCH v2] " Zhouyang Jia 0 siblings, 2 replies; 7+ messages in thread From: Zhouyang Jia @ 2018-06-11 8:52 UTC (permalink / raw) Cc: Zhouyang Jia, Alan Stern, Greg Kroah-Hartman, linux-usb, usb-storage, linux-kernel 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 <jiazhouyang09@gmail.com> --- drivers/usb/storage/alauda.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/drivers/usb/storage/alauda.c b/drivers/usb/storage/alauda.c index 900591d..c56355c 100644 --- a/drivers/usb/storage/alauda.c +++ b/drivers/usb/storage/alauda.c @@ -437,6 +437,11 @@ 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)) { + pr_warn("alauda_init_media: memory allocation failed\n"); + return USB_STOR_TRANSPORT_ERROR; + } if (alauda_reset_media(us) != USB_STOR_XFER_GOOD) return USB_STOR_TRANSPORT_ERROR; -- 2.7.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] usb: storage: add error handling for kcalloc 2018-06-11 8:52 [PATCH] usb: storage: add error handling for kcalloc Zhouyang Jia @ 2018-06-12 14:31 ` Alan Stern 2018-06-14 13:29 ` [PATCH v2] " Zhouyang Jia 1 sibling, 0 replies; 7+ messages in thread From: Alan Stern @ 2018-06-12 14:31 UTC (permalink / raw) To: Zhouyang Jia; +Cc: Greg Kroah-Hartman, linux-usb, usb-storage, linux-kernel On Mon, 11 Jun 2018, Zhouyang Jia wrote: > 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 <jiazhouyang09@gmail.com> > --- > drivers/usb/storage/alauda.c | 5 +++++ > 1 file changed, 5 insertions(+) > > diff --git a/drivers/usb/storage/alauda.c b/drivers/usb/storage/alauda.c > index 900591d..c56355c 100644 > --- a/drivers/usb/storage/alauda.c > +++ b/drivers/usb/storage/alauda.c > @@ -437,6 +437,11 @@ 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)) { > + pr_warn("alauda_init_media: memory allocation failed\n"); > + return USB_STOR_TRANSPORT_ERROR; > + } pr_info() isn't a good routine to use here, because it doesn't print the device or driver name. In any case, a log message isn't necessary. kcalloc() will already put a message in the log if either of the allocations fails. Alan Stern > > if (alauda_reset_media(us) != USB_STOR_XFER_GOOD) > return USB_STOR_TRANSPORT_ERROR; > ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] usb: storage: add error handling for kcalloc 2018-06-11 8:52 [PATCH] usb: storage: add error handling for kcalloc Zhouyang Jia 2018-06-12 14:31 ` Alan Stern @ 2018-06-14 13:29 ` Zhouyang Jia 2018-06-14 14:58 ` Alan Stern 2018-06-25 12:33 ` Greg Kroah-Hartman 1 sibling, 2 replies; 7+ messages in thread From: Zhouyang Jia @ 2018-06-14 13:29 UTC (permalink / raw) Cc: Zhouyang Jia, Alan Stern, Greg Kroah-Hartman, linux-usb, usb-storage, linux-kernel 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 <jiazhouyang09@gmail.com> --- 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; if (alauda_reset_media(us) != USB_STOR_XFER_GOOD) return USB_STOR_TRANSPORT_ERROR; -- 2.7.4 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] usb: storage: add error handling for kcalloc 2018-06-14 13:29 ` [PATCH v2] " Zhouyang Jia @ 2018-06-14 14:58 ` Alan Stern 2018-06-25 12:33 ` Greg Kroah-Hartman 1 sibling, 0 replies; 7+ messages in thread From: Alan Stern @ 2018-06-14 14:58 UTC (permalink / raw) To: Zhouyang Jia; +Cc: Greg Kroah-Hartman, linux-usb, usb-storage, linux-kernel On Thu, 14 Jun 2018, Zhouyang Jia wrote: > 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 <jiazhouyang09@gmail.com> > --- > 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; > > if (alauda_reset_media(us) != USB_STOR_XFER_GOOD) > return USB_STOR_TRANSPORT_ERROR; > Acked-by: Alan Stern <stern@rowland.harvard.edu> ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] usb: storage: add error handling for kcalloc 2018-06-14 13:29 ` [PATCH v2] " Zhouyang Jia 2018-06-14 14:58 ` Alan Stern @ 2018-06-25 12:33 ` Greg Kroah-Hartman 2018-06-25 15:22 ` Alan Stern 1 sibling, 1 reply; 7+ messages in thread From: Greg Kroah-Hartman @ 2018-06-25 12:33 UTC (permalink / raw) To: Zhouyang Jia; +Cc: Alan Stern, linux-usb, usb-storage, linux-kernel On Thu, Jun 14, 2018 at 09:29:11PM +0800, Zhouyang Jia wrote: > 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 <jiazhouyang09@gmail.com> > 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 :( ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] usb: storage: add error handling for kcalloc 2018-06-25 12:33 ` Greg Kroah-Hartman @ 2018-06-25 15:22 ` Alan Stern 2018-06-28 10:52 ` Greg Kroah-Hartman 0 siblings, 1 reply; 7+ messages in thread From: Alan Stern @ 2018-06-25 15:22 UTC (permalink / raw) To: Greg Kroah-Hartman; +Cc: Zhouyang Jia, linux-usb, usb-storage, linux-kernel On Mon, 25 Jun 2018, Greg Kroah-Hartman wrote: > On Thu, Jun 14, 2018 at 09:29:11PM +0800, Zhouyang Jia wrote: > > 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 <jiazhouyang09@gmail.com> > > 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2] usb: storage: add error handling for kcalloc 2018-06-25 15:22 ` Alan Stern @ 2018-06-28 10:52 ` Greg Kroah-Hartman 0 siblings, 0 replies; 7+ messages in thread From: Greg Kroah-Hartman @ 2018-06-28 10:52 UTC (permalink / raw) To: Alan Stern; +Cc: Zhouyang Jia, linux-usb, usb-storage, linux-kernel On Mon, Jun 25, 2018 at 11:22:34AM -0400, Alan Stern wrote: > On Mon, 25 Jun 2018, Greg Kroah-Hartman wrote: > > > On Thu, Jun 14, 2018 at 09:29:11PM +0800, Zhouyang Jia wrote: > > > 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 <jiazhouyang09@gmail.com> > > > 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. Ok, I'm just going to drop it then. thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2018-06-28 11:01 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2018-06-11 8:52 [PATCH] usb: storage: add error handling for kcalloc Zhouyang Jia 2018-06-12 14:31 ` Alan Stern 2018-06-14 13:29 ` [PATCH v2] " Zhouyang Jia 2018-06-14 14:58 ` Alan Stern 2018-06-25 12:33 ` Greg Kroah-Hartman 2018-06-25 15:22 ` Alan Stern 2018-06-28 10:52 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox