All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jon Hunter <jonathanh@nvidia.com>
To: Bartosz Golaszewski <brgl@bgdev.pl>
Cc: Jonathan Corbet <corbet@lwn.net>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	"Rafael J . Wysocki" <rafael@kernel.org>,
	Guenter Roeck <linux@roeck-us.net>,
	Jean Delvare <jdelvare@suse.com>,
	linux-doc <linux-doc@vger.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	<linux-hwmon@vger.kernel.org>,
	Bartosz Golaszewski <bgolaszewski@baylibre.com>
Subject: Re: [PATCH v2 4/6] devres: handle zero size in devm_kmalloc()
Date: Fri, 10 Jul 2020 17:30:54 +0100	[thread overview]
Message-ID: <c780f571-9aef-d02a-a9c0-677f6c1f7a0b@nvidia.com> (raw)
In-Reply-To: <CAMRc=Md+iHTeaYi1F-ykb3HaDTBoiGuNr7s224ay9Jgfhy1TcA@mail.gmail.com>


On 10/07/2020 17:24, Bartosz Golaszewski wrote:
> On Fri, Jul 10, 2020 at 6:11 PM Jon Hunter <jonathanh@nvidia.com> wrote:
>>
>>
>> On 10/07/2020 17:03, Bartosz Golaszewski wrote:
>>> On Fri, Jul 10, 2020 at 3:46 PM Jon Hunter <jonathanh@nvidia.com> wrote:
>>>>
>>>> Hi Bartosz,
>>>>
>>>> On 29/06/2020 07:50, Bartosz Golaszewski wrote:
>>>>> From: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>>>>
>>>>> Make devm_kmalloc() behave similarly to non-managed kmalloc(): return
>>>>> ZERO_SIZE_PTR when requested size is 0. Update devm_kfree() to handle
>>>>> this case.
>>>>>
>>>>> Signed-off-by: Bartosz Golaszewski <bgolaszewski@baylibre.com>
>>>>> ---
>>>>>  drivers/base/devres.c | 9 ++++++---
>>>>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>>>>
>>>>> diff --git a/drivers/base/devres.c b/drivers/base/devres.c
>>>>> index 1df1fb10b2d9..ed615d3b9cf1 100644
>>>>> --- a/drivers/base/devres.c
>>>>> +++ b/drivers/base/devres.c
>>>>> @@ -819,6 +819,9 @@ void *devm_kmalloc(struct device *dev, size_t size, gfp_t gfp)
>>>>>  {
>>>>>       struct devres *dr;
>>>>>
>>>>> +     if (unlikely(!size))
>>>>> +             return ZERO_SIZE_PTR;
>>>>> +
>>>>>       /* use raw alloc_dr for kmalloc caller tracing */
>>>>>       dr = alloc_dr(devm_kmalloc_release, size, gfp, dev_to_node(dev));
>>>>>       if (unlikely(!dr))
>>>>> @@ -950,10 +953,10 @@ void devm_kfree(struct device *dev, const void *p)
>>>>>       int rc;
>>>>>
>>>>>       /*
>>>>> -      * Special case: pointer to a string in .rodata returned by
>>>>> -      * devm_kstrdup_const().
>>>>> +      * Special cases: pointer to a string in .rodata returned by
>>>>> +      * devm_kstrdup_const() or NULL/ZERO ptr.
>>>>>        */
>>>>> -     if (unlikely(is_kernel_rodata((unsigned long)p)))
>>>>> +     if (unlikely(is_kernel_rodata((unsigned long)p) || ZERO_OR_NULL_PTR(p)))
>>>>>               return;
>>>>>
>>>>>       rc = devres_destroy(dev, devm_kmalloc_release,
>>>>
>>>>
>>>> This change caught a bug in one of our Tegra drivers, which I am in the
>>>> process of fixing. Once I bisected to this commit it was easy to track
>>>> down, but I am wondering if there is any reason why we don't add a
>>>> WARN_ON() if size is 0 in devm_kmalloc? It was essentially what I ended
>>>> up doing to find the bug.
>>>>
>>>> Jon
>>>>
>>>> --
>>>> nvpublic
>>>
>>> Hi Jon,
>>>
>>> this is in line with what the regular kmalloc() does. If size is zero,
>>> it returns ZERO_SIZE_PTR. It's not an error condition. Actually in
>>> user-space malloc() does a similar thing: for size == 0 it allocates
>>> one-byte and returns a pointer to it (at least in glibc).
>>
>>
>> Yes that's fine, I was just wondering if there is any reason not to WARN
>> as well?
>>
>> Cheers
>> Jon
>>
> 
> Why? Nothing bad happens. Regular kmalloc() doesn't warn, why should
> devm_kmalloc() do?


Simply because it is easier to track down a bug. In my case the NULL
pointer crash did not occur until entering suspend when the memory, that
was allocated at probe time, was first actually accessed. So it was not
immediately obvious which call to devm_kmalloc caused the problem.
Anyway, if kmalloc does not warn either, then fine, it was purely a
question.

Jon

-- 
nvpublic

  reply	other threads:[~2020-07-10 16:31 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-06-29  6:50 [PATCH v2 0/6] devres: provide and use devm_krealloc() Bartosz Golaszewski
2020-06-29  6:50 ` [PATCH v2 1/6] devres: remove stray space from devm_kmalloc() definition Bartosz Golaszewski
2020-06-29  6:50 ` [PATCH v2 2/6] devres: move the size check from alloc_dr() into a separate function Bartosz Golaszewski
2020-06-29  6:50 ` [PATCH v2 3/6] device: remove 'extern' attribute from function prototypes in device.h Bartosz Golaszewski
2020-06-29  6:50 ` [PATCH v2 4/6] devres: handle zero size in devm_kmalloc() Bartosz Golaszewski
2020-07-10 13:46   ` Jon Hunter
2020-07-10 16:03     ` Bartosz Golaszewski
2020-07-10 16:11       ` Jon Hunter
2020-07-10 16:24         ` Bartosz Golaszewski
2020-07-10 16:30           ` Jon Hunter [this message]
2021-04-11  3:21   ` Dmitry Torokhov
2021-04-12 19:23     ` Bartosz Golaszewski
2020-06-29  6:50 ` [PATCH v2 5/6] devres: provide devm_krealloc() Bartosz Golaszewski
2020-07-02 12:42   ` Greg Kroah-Hartman
2020-07-02 13:11     ` Bartosz Golaszewski
2020-07-06 16:38       ` Bartosz Golaszewski
2020-07-06 16:41         ` Greg Kroah-Hartman
2020-07-10 13:32         ` Greg Kroah-Hartman
2020-06-29  6:50 ` [PATCH v2 6/6] hwmon: pmbus: use more devres helpers Bartosz Golaszewski
2020-06-29 16:32   ` Guenter Roeck
2020-07-02 12:44   ` Greg Kroah-Hartman
2020-07-02 13:06     ` Bartosz Golaszewski
2020-07-02 14:29     ` Guenter Roeck
2020-07-02 12:44 ` [PATCH v2 0/6] devres: provide and use devm_krealloc() Greg Kroah-Hartman

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=c780f571-9aef-d02a-a9c0-677f6c1f7a0b@nvidia.com \
    --to=jonathanh@nvidia.com \
    --cc=bgolaszewski@baylibre.com \
    --cc=brgl@bgdev.pl \
    --cc=corbet@lwn.net \
    --cc=gregkh@linuxfoundation.org \
    --cc=jdelvare@suse.com \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-hwmon@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux@roeck-us.net \
    --cc=rafael@kernel.org \
    /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.