[linux-next][PATCH 1/1] gpio: Delete excess allocated label memory

[linux-next][PATCH 1/1] gpio: Delete excess allocated label memory

[Xiaolei Wang]

The changes in commit 1f2bcb8c8ccd ("gpio: protect the
descriptor label with SRCU"), desc_set_label has already
allocated memory space for the label, so there is no need
to allocate it again. otherwise memory leaks will be
introduced.

unreferenced object 0xffff0000c3e4d0c0 (size 32):
  comm "kworker/u16:4", pid 60, jiffies 4294894555
  hex dump (first 32 bytes):
    72 65 67 75 6c 61 74 6f 72 2d 63 61 6e 32 2d 73  regulator-can2-s
    74 62 79 00 00 00 ff ff ff ff ff ff eb db ff ff  tby.............
  backtrace (crc 2c3a0350):
    [<00000000e93c5cf4>] kmemleak_alloc+0x34/0x40
    [<0000000097a2657f>] __kmalloc_node_track_caller+0x2c4/0x524
    [<000000000dd1c057>] kstrdup+0x4c/0x98
    [<00000000b513a96a>] kstrdup_const+0x34/0x40
    [<000000008a7f0feb>] gpiod_request_commit+0xdc/0x358
    [<00000000fc71ad64>] gpiod_request+0xd8/0x204
    [<00000000fa24b091>] gpiod_find_and_request+0x170/0x780
    [<0000000086ecf92d>] gpiod_get_index+0x70/0xe0
    [<000000004aef97f9>] gpiod_get_optional+0x18/0x30
    [<00000000312f1b25>] reg_fixed_voltage_probe+0x58c/0xad8
    [<00000000e6f47635>] platform_probe+0xc4/0x198
    [<00000000cf78fbdb>] really_probe+0x204/0x5a8
    [<00000000e28d05ec>] __driver_probe_device+0x158/0x2c4
    [<00000000e4fe452b>] driver_probe_device+0x60/0x18c
    [<00000000479fcf5d>] __device_attach_driver+0x168/0x208
    [<000000007d389f38>] bus_for_each_drv+0x104/0x190

Fixes: 1f2bcb8c8ccd ("gpio: protect the descriptor label with SRCU")
Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
---
 drivers/gpio/gpiolib.c | 6 ------
 1 file changed, 6 deletions(-)

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 02be0ba1a402..32191547dece 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -2250,12 +2250,6 @@ static int gpiod_request_commit(struct gpio_desc *desc, const char *label)
 	if (test_and_set_bit(FLAG_REQUESTED, &desc->flags))
 		return -EBUSY;
 
-	if (label) {
-		label = kstrdup_const(label, GFP_KERNEL);
-		if (!label)
-			return -ENOMEM;
-	}
-
 	/* NOTE:  gpio_request() can be called in early boot,
 	 * before IRQs are enabled, for non-sleeping (SOC) GPIOs.
 	 */
-- 
2.25.1

Re: [linux-next][PATCH 1/1] gpio: Delete excess allocated label memory

[Bartosz Golaszewski]

On Sat, Feb 17, 2024 at 2:53 PM Xiaolei Wang <xiaolei.wang@windriver.com> wrote:
>
> The changes in commit 1f2bcb8c8ccd ("gpio: protect the
> descriptor label with SRCU"), desc_set_label has already
> allocated memory space for the label, so there is no need
> to allocate it again. otherwise memory leaks will be
> introduced.
>

No, we *want* to copy it if it's not in .rodata for the same reason we
introduced SRCU. This may be a valid report but the fix is wrong.

> unreferenced object 0xffff0000c3e4d0c0 (size 32):
>   comm "kworker/u16:4", pid 60, jiffies 4294894555
>   hex dump (first 32 bytes):
>     72 65 67 75 6c 61 74 6f 72 2d 63 61 6e 32 2d 73  regulator-can2-s
>     74 62 79 00 00 00 ff ff ff ff ff ff eb db ff ff  tby.............
>   backtrace (crc 2c3a0350):
>     [<00000000e93c5cf4>] kmemleak_alloc+0x34/0x40
>     [<0000000097a2657f>] __kmalloc_node_track_caller+0x2c4/0x524
>     [<000000000dd1c057>] kstrdup+0x4c/0x98
>     [<00000000b513a96a>] kstrdup_const+0x34/0x40
>     [<000000008a7f0feb>] gpiod_request_commit+0xdc/0x358
>     [<00000000fc71ad64>] gpiod_request+0xd8/0x204
>     [<00000000fa24b091>] gpiod_find_and_request+0x170/0x780
>     [<0000000086ecf92d>] gpiod_get_index+0x70/0xe0
>     [<000000004aef97f9>] gpiod_get_optional+0x18/0x30
>     [<00000000312f1b25>] reg_fixed_voltage_probe+0x58c/0xad8
>     [<00000000e6f47635>] platform_probe+0xc4/0x198
>     [<00000000cf78fbdb>] really_probe+0x204/0x5a8
>     [<00000000e28d05ec>] __driver_probe_device+0x158/0x2c4
>     [<00000000e4fe452b>] driver_probe_device+0x60/0x18c
>     [<00000000479fcf5d>] __device_attach_driver+0x168/0x208
>     [<000000007d389f38>] bus_for_each_drv+0x104/0x190
>

Can you post the full kmemleak report for this, please?

Bart

> Fixes: 1f2bcb8c8ccd ("gpio: protect the descriptor label with SRCU")
> Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
> ---
>  drivers/gpio/gpiolib.c | 6 ------
>  1 file changed, 6 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 02be0ba1a402..32191547dece 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -2250,12 +2250,6 @@ static int gpiod_request_commit(struct gpio_desc *desc, const char *label)
>         if (test_and_set_bit(FLAG_REQUESTED, &desc->flags))
>                 return -EBUSY;
>
> -       if (label) {
> -               label = kstrdup_const(label, GFP_KERNEL);
> -               if (!label)
> -                       return -ENOMEM;
> -       }
> -
>         /* NOTE:  gpio_request() can be called in early boot,
>          * before IRQs are enabled, for non-sleeping (SOC) GPIOs.
>          */
> --
> 2.25.1
>

Re: [linux-next][PATCH 1/1] gpio: Delete excess allocated label memory

[Bartosz Golaszewski]

On Sat, 17 Feb 2024 19:35:43 +0100, Bartosz Golaszewski <brgl@bgdev.pl> said:
> On Sat, Feb 17, 2024 at 2:53 PM Xiaolei Wang <xiaolei.wang@windriver.com> wrote:
>>
>> The changes in commit 1f2bcb8c8ccd ("gpio: protect the
>> descriptor label with SRCU"), desc_set_label has already
>> allocated memory space for the label, so there is no need
>> to allocate it again. otherwise memory leaks will be
>> introduced.
>>
>
> No, we *want* to copy it if it's not in .rodata for the same reason we
> introduced SRCU. This may be a valid report but the fix is wrong.
>
>> unreferenced object 0xffff0000c3e4d0c0 (size 32):
>>   comm "kworker/u16:4", pid 60, jiffies 4294894555
>>   hex dump (first 32 bytes):
>>     72 65 67 75 6c 61 74 6f 72 2d 63 61 6e 32 2d 73  regulator-can2-s
>>     74 62 79 00 00 00 ff ff ff ff ff ff eb db ff ff  tby.............
>>   backtrace (crc 2c3a0350):
>>     [<00000000e93c5cf4>] kmemleak_alloc+0x34/0x40
>>     [<0000000097a2657f>] __kmalloc_node_track_caller+0x2c4/0x524
>>     [<000000000dd1c057>] kstrdup+0x4c/0x98
>>     [<00000000b513a96a>] kstrdup_const+0x34/0x40
>>     [<000000008a7f0feb>] gpiod_request_commit+0xdc/0x358
>>     [<00000000fc71ad64>] gpiod_request+0xd8/0x204
>>     [<00000000fa24b091>] gpiod_find_and_request+0x170/0x780
>>     [<0000000086ecf92d>] gpiod_get_index+0x70/0xe0
>>     [<000000004aef97f9>] gpiod_get_optional+0x18/0x30
>>     [<00000000312f1b25>] reg_fixed_voltage_probe+0x58c/0xad8
>>     [<00000000e6f47635>] platform_probe+0xc4/0x198
>>     [<00000000cf78fbdb>] really_probe+0x204/0x5a8
>>     [<00000000e28d05ec>] __driver_probe_device+0x158/0x2c4
>>     [<00000000e4fe452b>] driver_probe_device+0x60/0x18c
>>     [<00000000479fcf5d>] __device_attach_driver+0x168/0x208
>>     [<000000007d389f38>] bus_for_each_drv+0x104/0x190
>>
>
> Can you post the full kmemleak report for this, please?
>
> Bart
>

Ah, I think I see the problem. Can you test the following diff:

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 02be0ba1a402..0fdd4ad242bd 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -695,10 +695,15 @@ EXPORT_SYMBOL_GPL(gpiochip_line_is_valid);
 static void gpiodev_release(struct device *dev)
 {
 	struct gpio_device *gdev = to_gpio_device(dev);
+	struct gpio_desc *desc;
 	unsigned int i;

-	for (i = 0; i < gdev->ngpio; i++)
-		cleanup_srcu_struct(&gdev->descs[i].srcu);
+	for (i = 0; i < gdev->ngpio; i++) {
+		desc = &gdev->descs[i];
+
+		kfree_const(desc->label);
+		cleanup_srcu_struct(&desc->srcu);
+	}

 	ida_free(&gpio_ida, gdev->id);
 	kfree_const(gdev->label);

and let me know if it fixes the issue?

Bart

>> Fixes: 1f2bcb8c8ccd ("gpio: protect the descriptor label with SRCU")
>> Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
>> ---
>>  drivers/gpio/gpiolib.c | 6 ------
>>  1 file changed, 6 deletions(-)
>>
>> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
>> index 02be0ba1a402..32191547dece 100644
>> --- a/drivers/gpio/gpiolib.c
>> +++ b/drivers/gpio/gpiolib.c
>> @@ -2250,12 +2250,6 @@ static int gpiod_request_commit(struct gpio_desc *desc, const char *label)
>>         if (test_and_set_bit(FLAG_REQUESTED, &desc->flags))
>>                 return -EBUSY;
>>
>> -       if (label) {
>> -               label = kstrdup_const(label, GFP_KERNEL);
>> -               if (!label)
>> -                       return -ENOMEM;
>> -       }
>> -
>>         /* NOTE:  gpio_request() can be called in early boot,
>>          * before IRQs are enabled, for non-sleeping (SOC) GPIOs.
>>          */
>> --
>> 2.25.1
>>
>

Re: [linux-next][PATCH 1/1] gpio: Delete excess allocated label memory

[xiaolei wang]

On 2/18/24 02:52, Bartosz Golaszewski wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> On Sat, 17 Feb 2024 19:35:43 +0100, Bartosz Golaszewski <brgl@bgdev.pl> said:
>> On Sat, Feb 17, 2024 at 2:53 PM Xiaolei Wang <xiaolei.wang@windriver.com> wrote:
>>> The changes in commit 1f2bcb8c8ccd ("gpio: protect the
>>> descriptor label with SRCU"), desc_set_label has already
>>> allocated memory space for the label, so there is no need
>>> to allocate it again. otherwise memory leaks will be
>>> introduced.
>>>
>> No, we *want* to copy it if it's not in .rodata for the same reason we
>> introduced SRCU. This may be a valid report but the fix is wrong.
>>
>>> unreferenced object 0xffff0000c3e4d0c0 (size 32):
>>>    comm "kworker/u16:4", pid 60, jiffies 4294894555
>>>    hex dump (first 32 bytes):
>>>      72 65 67 75 6c 61 74 6f 72 2d 63 61 6e 32 2d 73  regulator-can2-s
>>>      74 62 79 00 00 00 ff ff ff ff ff ff eb db ff ff  tby.............
>>>    backtrace (crc 2c3a0350):
>>>      [<00000000e93c5cf4>] kmemleak_alloc+0x34/0x40
>>>      [<0000000097a2657f>] __kmalloc_node_track_caller+0x2c4/0x524
>>>      [<000000000dd1c057>] kstrdup+0x4c/0x98
>>>      [<00000000b513a96a>] kstrdup_const+0x34/0x40
>>>      [<000000008a7f0feb>] gpiod_request_commit+0xdc/0x358
>>>      [<00000000fc71ad64>] gpiod_request+0xd8/0x204
>>>      [<00000000fa24b091>] gpiod_find_and_request+0x170/0x780
>>>      [<0000000086ecf92d>] gpiod_get_index+0x70/0xe0
>>>      [<000000004aef97f9>] gpiod_get_optional+0x18/0x30
>>>      [<00000000312f1b25>] reg_fixed_voltage_probe+0x58c/0xad8
>>>      [<00000000e6f47635>] platform_probe+0xc4/0x198
>>>      [<00000000cf78fbdb>] really_probe+0x204/0x5a8
>>>      [<00000000e28d05ec>] __driver_probe_device+0x158/0x2c4
>>>      [<00000000e4fe452b>] driver_probe_device+0x60/0x18c
>>>      [<00000000479fcf5d>] __device_attach_driver+0x168/0x208
>>>      [<000000007d389f38>] bus_for_each_drv+0x104/0x190
>>>
>> Can you post the full kmemleak report for this, please?
>>
>> Bart
>>
> Ah, I think I see the problem. Can you test the following diff:
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 02be0ba1a402..0fdd4ad242bd 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -695,10 +695,15 @@ EXPORT_SYMBOL_GPL(gpiochip_line_is_valid);
>   static void gpiodev_release(struct device *dev)
>   {
>          struct gpio_device *gdev = to_gpio_device(dev);
> +       struct gpio_desc *desc;
>          unsigned int i;
>
> -       for (i = 0; i < gdev->ngpio; i++)
> -               cleanup_srcu_struct(&gdev->descs[i].srcu);
> +       for (i = 0; i < gdev->ngpio; i++) {
> +               desc = &gdev->descs[i];
> +
> +               kfree_const(desc->label);
> +               cleanup_srcu_struct(&desc->srcu);
> +       }
>
>          ida_free(&gpio_ida, gdev->id);
>          kfree_const(gdev->label);
>
> and let me know if it fixes the issue?
I tried the following modifications, which did not fix the problem,

diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
index 02be0ba1a402..5940ef88399c 100644
--- a/drivers/gpio/gpiolib.c
+++ b/drivers/gpio/gpiolib.c
@@ -695,10 +695,14 @@ EXPORT_SYMBOL_GPL(gpiochip_line_is_valid);
   static void gpiodev_release(struct device *dev)
   {
          struct gpio_device *gdev = to_gpio_device(dev);
+ struct gpio_desc *desc;
          unsigned int i;

- for (i = 0; i < gdev->ngpio; i++)
- cleanup_srcu_struct(&gdev->descs[i].srcu);
+ for (i = 0; i < gdev->ngpio; i++) {
+ desc = &gdev->descs[i];
+ kfree_const(desc->label);
+ cleanup_srcu_struct(&desc->srcu);
+ }

          ida_free(&gpio_ida, gdev->id);

          kfree_const(gdev->label);


unreferenced object 0xffff0000c0e83740 (size 32):
   comm "kworker/u16:0", pid 10, jiffies 4294894561
   hex dump (first 32 bytes):
     72 65 67 75 6c 61 74 6f 72 2d 75 73 64 68 63 32 regulator-usdhc2
     00 37 e8 c0 00 00 ff ff 00 00 00 00 00 00 00 00 .7..............
   backtrace (crc 222fadd4):
     [<00000000cd106dac>] kmemleak_alloc+0x34/0x40
     [<00000000e084cf5f>] __kmalloc_node_track_caller+0x2c4/0x524
     [<00000000746173b0>] kstrdup+0x4c/0x98
     [<00000000f79b3bf4>] kstrdup_const+0x34/0x40
     [<00000000f47eb728>] gpiod_request_commit+0xdc/0x358
     [<000000003a6899af>] gpiod_request+0xd8/0x204
     [<00000000c93118be>] gpiod_find_and_request+0x170/0x780
     [<00000000660eebdd>] gpiod_get_index+0x70/0xe0
     [<00000000eb599be3>] gpiod_get_optional+0x18/0x30
     [<00000000e8b3cfcb>] reg_fixed_voltage_probe+0x58c/0xad8
     [<000000009737579a>] platform_probe+0xc4/0x198
     [<00000000405158e3>] really_probe+0x204/0x5a8
     [<00000000772dd378>] __driver_probe_device+0x158/0x2c4
     [<00000000ddc25aba>] driver_probe_device+0x60/0x18c
     [<00000000cc4413ff>] __device_attach_driver+0x168/0x208
     [<00000000e04f3755>] bus_for_each_drv+0x104/0x190

I'm a little confused. In the gpiod_request_commit() function, if label 
is true, we allocate space kstrdup_const() for it, but desc_set_label() 
will still be called. If label is true, space will be allocated to it 
again, desc-> label will be pointed to the place where new is allocated,

thanks

xiaolei

>
> Bart
>
>>> Fixes: 1f2bcb8c8ccd ("gpio: protect the descriptor label with SRCU")
>>> Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
>>> ---
>>>   drivers/gpio/gpiolib.c | 6 ------
>>>   1 file changed, 6 deletions(-)
>>>
>>> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
>>> index 02be0ba1a402..32191547dece 100644
>>> --- a/drivers/gpio/gpiolib.c
>>> +++ b/drivers/gpio/gpiolib.c
>>> @@ -2250,12 +2250,6 @@ static int gpiod_request_commit(struct gpio_desc *desc, const char *label)
>>>          if (test_and_set_bit(FLAG_REQUESTED, &desc->flags))
>>>                  return -EBUSY;
>>>
>>> -       if (label) {
>>> -               label = kstrdup_const(label, GFP_KERNEL);
>>> -               if (!label)
>>> -                       return -ENOMEM;
>>> -       }
>>> -
>>>          /* NOTE:  gpio_request() can be called in early boot,
>>>           * before IRQs are enabled, for non-sleeping (SOC) GPIOs.
>>>           */
>>> --
>>> 2.25.1
>>>

Re: [linux-next][PATCH 1/1] gpio: Delete excess allocated label memory

[Bartosz Golaszewski]

On Sun, Feb 18, 2024 at 1:55 AM xiaolei wang <xiaolei.wang@windriver.com> wrote:
>
>
> On 2/18/24 02:52, Bartosz Golaszewski wrote:
> > CAUTION: This email comes from a non Wind River email account!
> > Do not click links or open attachments unless you recognize the sender and know the content is safe.
> >
> > On Sat, 17 Feb 2024 19:35:43 +0100, Bartosz Golaszewski <brgl@bgdev.pl> said:
> >> On Sat, Feb 17, 2024 at 2:53 PM Xiaolei Wang <xiaolei.wang@windriver.com> wrote:
> >>> The changes in commit 1f2bcb8c8ccd ("gpio: protect the
> >>> descriptor label with SRCU"), desc_set_label has already
> >>> allocated memory space for the label, so there is no need
> >>> to allocate it again. otherwise memory leaks will be
> >>> introduced.
> >>>
> >> No, we *want* to copy it if it's not in .rodata for the same reason we
> >> introduced SRCU. This may be a valid report but the fix is wrong.
> >>
> >>> unreferenced object 0xffff0000c3e4d0c0 (size 32):
> >>>    comm "kworker/u16:4", pid 60, jiffies 4294894555
> >>>    hex dump (first 32 bytes):
> >>>      72 65 67 75 6c 61 74 6f 72 2d 63 61 6e 32 2d 73  regulator-can2-s
> >>>      74 62 79 00 00 00 ff ff ff ff ff ff eb db ff ff  tby.............
> >>>    backtrace (crc 2c3a0350):
> >>>      [<00000000e93c5cf4>] kmemleak_alloc+0x34/0x40
> >>>      [<0000000097a2657f>] __kmalloc_node_track_caller+0x2c4/0x524
> >>>      [<000000000dd1c057>] kstrdup+0x4c/0x98
> >>>      [<00000000b513a96a>] kstrdup_const+0x34/0x40
> >>>      [<000000008a7f0feb>] gpiod_request_commit+0xdc/0x358
> >>>      [<00000000fc71ad64>] gpiod_request+0xd8/0x204
> >>>      [<00000000fa24b091>] gpiod_find_and_request+0x170/0x780
> >>>      [<0000000086ecf92d>] gpiod_get_index+0x70/0xe0
> >>>      [<000000004aef97f9>] gpiod_get_optional+0x18/0x30
> >>>      [<00000000312f1b25>] reg_fixed_voltage_probe+0x58c/0xad8
> >>>      [<00000000e6f47635>] platform_probe+0xc4/0x198
> >>>      [<00000000cf78fbdb>] really_probe+0x204/0x5a8
> >>>      [<00000000e28d05ec>] __driver_probe_device+0x158/0x2c4
> >>>      [<00000000e4fe452b>] driver_probe_device+0x60/0x18c
> >>>      [<00000000479fcf5d>] __device_attach_driver+0x168/0x208
> >>>      [<000000007d389f38>] bus_for_each_drv+0x104/0x190
> >>>
> >> Can you post the full kmemleak report for this, please?
> >>
> >> Bart
> >>
> > Ah, I think I see the problem. Can you test the following diff:
> >
> > diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> > index 02be0ba1a402..0fdd4ad242bd 100644
> > --- a/drivers/gpio/gpiolib.c
> > +++ b/drivers/gpio/gpiolib.c
> > @@ -695,10 +695,15 @@ EXPORT_SYMBOL_GPL(gpiochip_line_is_valid);
> >   static void gpiodev_release(struct device *dev)
> >   {
> >          struct gpio_device *gdev = to_gpio_device(dev);
> > +       struct gpio_desc *desc;
> >          unsigned int i;
> >
> > -       for (i = 0; i < gdev->ngpio; i++)
> > -               cleanup_srcu_struct(&gdev->descs[i].srcu);
> > +       for (i = 0; i < gdev->ngpio; i++) {
> > +               desc = &gdev->descs[i];
> > +
> > +               kfree_const(desc->label);
> > +               cleanup_srcu_struct(&desc->srcu);
> > +       }
> >
> >          ida_free(&gpio_ida, gdev->id);
> >          kfree_const(gdev->label);
> >
> > and let me know if it fixes the issue?
> I tried the following modifications, which did not fix the problem,
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 02be0ba1a402..5940ef88399c 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -695,10 +695,14 @@ EXPORT_SYMBOL_GPL(gpiochip_line_is_valid);
>    static void gpiodev_release(struct device *dev)
>    {
>           struct gpio_device *gdev = to_gpio_device(dev);
> + struct gpio_desc *desc;
>           unsigned int i;
>
> - for (i = 0; i < gdev->ngpio; i++)
> - cleanup_srcu_struct(&gdev->descs[i].srcu);
> + for (i = 0; i < gdev->ngpio; i++) {
> + desc = &gdev->descs[i];
> + kfree_const(desc->label);
> + cleanup_srcu_struct(&desc->srcu);
> + }
>
>           ida_free(&gpio_ida, gdev->id);
>
>           kfree_const(gdev->label);
>
>
> unreferenced object 0xffff0000c0e83740 (size 32):
>    comm "kworker/u16:0", pid 10, jiffies 4294894561
>    hex dump (first 32 bytes):
>      72 65 67 75 6c 61 74 6f 72 2d 75 73 64 68 63 32 regulator-usdhc2
>      00 37 e8 c0 00 00 ff ff 00 00 00 00 00 00 00 00 .7..............
>    backtrace (crc 222fadd4):
>      [<00000000cd106dac>] kmemleak_alloc+0x34/0x40
>      [<00000000e084cf5f>] __kmalloc_node_track_caller+0x2c4/0x524
>      [<00000000746173b0>] kstrdup+0x4c/0x98
>      [<00000000f79b3bf4>] kstrdup_const+0x34/0x40
>      [<00000000f47eb728>] gpiod_request_commit+0xdc/0x358
>      [<000000003a6899af>] gpiod_request+0xd8/0x204
>      [<00000000c93118be>] gpiod_find_and_request+0x170/0x780
>      [<00000000660eebdd>] gpiod_get_index+0x70/0xe0
>      [<00000000eb599be3>] gpiod_get_optional+0x18/0x30
>      [<00000000e8b3cfcb>] reg_fixed_voltage_probe+0x58c/0xad8
>      [<000000009737579a>] platform_probe+0xc4/0x198
>      [<00000000405158e3>] really_probe+0x204/0x5a8
>      [<00000000772dd378>] __driver_probe_device+0x158/0x2c4
>      [<00000000ddc25aba>] driver_probe_device+0x60/0x18c
>      [<00000000cc4413ff>] __device_attach_driver+0x168/0x208
>      [<00000000e04f3755>] bus_for_each_drv+0x104/0x190
>
> I'm a little confused. In the gpiod_request_commit() function, if label
> is true, we allocate space kstrdup_const() for it, but desc_set_label()
> will still be called. If label is true, space will be allocated to it
> again, desc-> label will be pointed to the place where new is allocated,
>

Ah, right I see it now. I sent a fix, please leave your Tested-by if
it works. Thanks for the report.

Bart

BTW: This is still not a complete kmemleak report, please next time
paste the entire thing.

> thanks
>
> xiaolei
>
> >
> > Bart
> >
> >>> Fixes: 1f2bcb8c8ccd ("gpio: protect the descriptor label with SRCU")
> >>> Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
> >>> ---
> >>>   drivers/gpio/gpiolib.c | 6 ------
> >>>   1 file changed, 6 deletions(-)
> >>>
> >>> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> >>> index 02be0ba1a402..32191547dece 100644
> >>> --- a/drivers/gpio/gpiolib.c
> >>> +++ b/drivers/gpio/gpiolib.c
> >>> @@ -2250,12 +2250,6 @@ static int gpiod_request_commit(struct gpio_desc *desc, const char *label)
> >>>          if (test_and_set_bit(FLAG_REQUESTED, &desc->flags))
> >>>                  return -EBUSY;
> >>>
> >>> -       if (label) {
> >>> -               label = kstrdup_const(label, GFP_KERNEL);
> >>> -               if (!label)
> >>> -                       return -ENOMEM;
> >>> -       }
> >>> -
> >>>          /* NOTE:  gpio_request() can be called in early boot,
> >>>           * before IRQs are enabled, for non-sleeping (SOC) GPIOs.
> >>>           */
> >>> --
> >>> 2.25.1
> >>>

Re: [linux-next][PATCH 1/1] gpio: Delete excess allocated label memory

[Bartosz Golaszewski]

On Sat, Feb 17, 2024 at 2:53 PM Xiaolei Wang <xiaolei.wang@windriver.com> wrote:
>
> The changes in commit 1f2bcb8c8ccd ("gpio: protect the
> descriptor label with SRCU"), desc_set_label has already
> allocated memory space for the label, so there is no need
> to allocate it again. otherwise memory leaks will be
> introduced.
>
> unreferenced object 0xffff0000c3e4d0c0 (size 32):
>   comm "kworker/u16:4", pid 60, jiffies 4294894555
>   hex dump (first 32 bytes):
>     72 65 67 75 6c 61 74 6f 72 2d 63 61 6e 32 2d 73  regulator-can2-s
>     74 62 79 00 00 00 ff ff ff ff ff ff eb db ff ff  tby.............
>   backtrace (crc 2c3a0350):
>     [<00000000e93c5cf4>] kmemleak_alloc+0x34/0x40
>     [<0000000097a2657f>] __kmalloc_node_track_caller+0x2c4/0x524
>     [<000000000dd1c057>] kstrdup+0x4c/0x98
>     [<00000000b513a96a>] kstrdup_const+0x34/0x40
>     [<000000008a7f0feb>] gpiod_request_commit+0xdc/0x358
>     [<00000000fc71ad64>] gpiod_request+0xd8/0x204
>     [<00000000fa24b091>] gpiod_find_and_request+0x170/0x780
>     [<0000000086ecf92d>] gpiod_get_index+0x70/0xe0
>     [<000000004aef97f9>] gpiod_get_optional+0x18/0x30
>     [<00000000312f1b25>] reg_fixed_voltage_probe+0x58c/0xad8
>     [<00000000e6f47635>] platform_probe+0xc4/0x198
>     [<00000000cf78fbdb>] really_probe+0x204/0x5a8
>     [<00000000e28d05ec>] __driver_probe_device+0x158/0x2c4
>     [<00000000e4fe452b>] driver_probe_device+0x60/0x18c
>     [<00000000479fcf5d>] __device_attach_driver+0x168/0x208
>     [<000000007d389f38>] bus_for_each_drv+0x104/0x190
>
> Fixes: 1f2bcb8c8ccd ("gpio: protect the descriptor label with SRCU")
> Signed-off-by: Xiaolei Wang <xiaolei.wang@windriver.com>
> ---
>  drivers/gpio/gpiolib.c | 6 ------
>  1 file changed, 6 deletions(-)
>
> diff --git a/drivers/gpio/gpiolib.c b/drivers/gpio/gpiolib.c
> index 02be0ba1a402..32191547dece 100644
> --- a/drivers/gpio/gpiolib.c
> +++ b/drivers/gpio/gpiolib.c
> @@ -2250,12 +2250,6 @@ static int gpiod_request_commit(struct gpio_desc *desc, const char *label)
>         if (test_and_set_bit(FLAG_REQUESTED, &desc->flags))
>                 return -EBUSY;
>
> -       if (label) {
> -               label = kstrdup_const(label, GFP_KERNEL);
> -               if (!label)
> -                       return -ENOMEM;
> -       }
> -
>         /* NOTE:  gpio_request() can be called in early boot,
>          * before IRQs are enabled, for non-sleeping (SOC) GPIOs.
>          */
> --
> 2.25.1
>

A queued this, sorry for the confusion, this is of course a correct fix.

Bart