From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Zhouyang Jia <jiazhouyang09@gmail.com>,
linux-usb@vger.kernel.org, usb-storage@lists.one-eyed-alien.net,
linux-kernel@vger.kernel.org
Subject: [v2] usb: storage: add error handling for kcalloc
Date: Thu, 28 Jun 2018 19:52:41 +0900 [thread overview]
Message-ID: <20180628105241.GC5191@kroah.com> (raw)
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
---
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
WARNING: multiple messages have this Message-ID (diff)
From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To: Alan Stern <stern@rowland.harvard.edu>
Cc: Zhouyang Jia <jiazhouyang09@gmail.com>,
linux-usb@vger.kernel.org, usb-storage@lists.one-eyed-alien.net,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2] usb: storage: add error handling for kcalloc
Date: Thu, 28 Jun 2018 19:52:41 +0900 [thread overview]
Message-ID: <20180628105241.GC5191@kroah.com> (raw)
In-Reply-To: <Pine.LNX.4.44L0.1806251118110.1779-100000@iolanthe.rowland.org>
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
next reply other threads:[~2018-06-28 10:52 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-06-28 10:52 Greg Kroah-Hartman [this message]
2018-06-28 10:52 ` [PATCH v2] usb: storage: add error handling for kcalloc Greg Kroah-Hartman
-- strict thread matches above, loose matches on Subject: below --
2018-06-25 15:22 [v2] " Alan Stern
2018-06-25 15:22 ` [PATCH v2] " Alan Stern
2018-06-25 12:33 [v2] " Greg Kroah-Hartman
2018-06-25 12:33 ` [PATCH v2] " Greg Kroah-Hartman
2018-06-14 14:58 [v2] " Alan Stern
2018-06-14 14:58 ` [PATCH v2] " Alan Stern
2018-06-14 13:29 [v2] " Zhouyang Jia
2018-06-14 13:29 ` [PATCH v2] " Zhouyang Jia
2018-06-12 14:31 Alan Stern
2018-06-12 14:31 ` [PATCH] " Alan Stern
2018-06-11 8:52 Zhouyang Jia
2018-06-11 8:52 ` [PATCH] " Zhouyang Jia
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=20180628105241.GC5191@kroah.com \
--to=gregkh@linuxfoundation.org \
--cc=jiazhouyang09@gmail.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-usb@vger.kernel.org \
--cc=stern@rowland.harvard.edu \
--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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.