Thread (24 messages) 24 messages, 5 authors, 2021-04-12

Re: [PATCH v2 4/6] devres: handle zero size in devm_kmalloc()

From: Jon Hunter <jonathanh@nvidia.com>
Date: 2020-07-10 16:31:01
Also in: linux-hwmon, lkml

On 10/07/2020 17:24, Bartosz Golaszewski wrote:
On Fri, Jul 10, 2020 at 6:11 PM Jon Hunter [off-list ref] wrote:
quoted

On 10/07/2020 17:03, Bartosz Golaszewski wrote:
quoted
On Fri, Jul 10, 2020 at 3:46 PM Jon Hunter [off-list ref] wrote:
quoted
Hi Bartosz,

On 29/06/2020 07:50, Bartosz Golaszewski wrote:
quoted
From: Bartosz Golaszewski <redacted>

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 <redacted>
---
 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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help