Messages in this thread | | | Subject | Re: [PATCH] knav_qmss_queue: fix a missing-check bug in knav_pool_create() | From | santosh.shilimkar@oracle ... | Date | Tue, 11 Jun 2019 14:10:49 -0700 |
| |
On 6/11/19 3:08 AM, Gen Zhang wrote: > On Tue, Jun 11, 2019 at 10:54:15AM +0100, Marc Zyngier wrote: >> Hi Gen, >> >> No idea why I'm being cc'd on this but hey... ;-) > I copied email address ftom thid commit:-) > https://github.com/torvalds/linux/commit/832ad0e3da4510fd17f98804abe512ea9a747035#diff-f2a24befc247191f4b81af93e9336785 >> >> On 11/06/2019 10:37, Gen Zhang wrote: >>> On Thu, May 30, 2019 at 11:39:49AM +0800, Gen Zhang wrote: >>>> In knav_pool_create(), 'pool->name' is allocated by kstrndup(). It >>>> returns NULL when fails. So 'pool->name' should be checked. And free >>>> 'pool' when error. >>>> >>>> Signed-off-by: Gen Zhang <blackgod016574@gmail.com> >>>> --- >>>> diff --git a/drivers/soc/ti/knav_qmss_queue.c b/drivers/soc/ti/knav_qmss_queue.c >>>> index 8b41837..0f8cb28 100644 >>>> --- a/drivers/soc/ti/knav_qmss_queue.c >>>> +++ b/drivers/soc/ti/knav_qmss_queue.c >>>> @@ -814,6 +814,12 @@ void *knav_pool_create(const char *name, >>>> } >>>> >>>> pool->name = kstrndup(name, KNAV_NAME_SIZE - 1, GFP_KERNEL); >>>> + if (!pool->name) { >>>> + dev_err(kdev->dev, "failed to duplicate for pool(%s)\n", >>>> + name); >> >> There is no need to output anything, the kernel will be loud enough if >> you run out of memory. > Thanks for your comments. >> >>>> + ret = -ENOMEM; >>>> + goto err_name; >>>> + } >>>> pool->kdev = kdev; >>>> pool->dev = kdev->dev; >>>> >>>> @@ -864,6 +870,7 @@ void *knav_pool_create(const char *name, >>>> mutex_unlock(&knav_dev_lock); >>>> err: >>>> kfree(pool->name); >>>> +err_name: >> >> kfree(NULL) is perfectly valid, there is no need to create a second >> label. Just branch to the existing error label. > Sure, better not to add redundant codes. >> >>>> devm_kfree(kdev->dev, pool); >>>> return ERR_PTR(ret); >>>> } >>> Can anyone look into this patch? >>> >>> Thanks >>> Gen >>> >> >> The real question is whether this is actually an error at all. >> pool->name doesn't seem to be used for anything but debug information, >> and the printing code can perfectly accommodate a NULL pointer. > That sounds reasonable. This patch just fixes a *theoretical* bug. > Not even theoretical bug.
| |