[linux-next:master 7918/13392] drivers/soc/microchip/mpfs-sys-controller.c:73:1-6: WARNING: invalid free of devm_ allocated data

[linux-next:master 7918/13392] drivers/soc/microchip/mpfs-sys-controller.c:73:1-6: WARNING: invalid free of devm_ allocated data

[kernel test robot]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
head:   97add482e9033a9386baa8824fbea34d0cbfe663
commit: d0054a470c33902f5ae88835ed8a8ecc3cf8faa4 [7918/13392] soc: add microchip polarfire soc system controller
config: ia64-randconfig-c004-20220317 (https://download.01.org/0day-ci/archive/20220318/202203180259.lgIylRZV-lkp@intel.com/config)
compiler: ia64-linux-gcc (GCC) 11.2.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


cocci warnings: (new ones prefixed by >>)
>> drivers/soc/microchip/mpfs-sys-controller.c:73:1-6: WARNING: invalid free of devm_ allocated data

vim +73 drivers/soc/microchip/mpfs-sys-controller.c

    66	
    67	static void mpfs_sys_controller_delete(struct kref *kref)
    68	{
    69		struct mpfs_sys_controller *sys_controller = container_of(kref, struct mpfs_sys_controller,
    70						       consumers);
    71	
    72		mbox_free_channel(sys_controller->chan);
  > 73		kfree(sys_controller);
    74	}
    75	

---
0-DAY CI Kernel Test Service
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [linux-next:master 7918/13392] drivers/soc/microchip/mpfs-sys-controller.c:73:1-6: WARNING: invalid free of devm_ allocated data

[conor dooley]

> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
> head:   97add482e9033a9386baa8824fbea34d0cbfe663
> commit: d0054a470c33902f5ae88835ed8a8ecc3cf8faa4 [7918/13392] soc: add microchip polarfire soc system controller
> config: ia64-randconfig-c004-20220317 (https://download.01.org/0day-ci/archive/20220318/202203180259.lgIylRZV-lkp@intel.com/config)
> compiler: ia64-linux-gcc (GCC) 11.2.0
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@intel.com>
> 
> 
> cocci warnings: (new ones prefixed by >>)
> >> drivers/soc/microchip/mpfs-sys-controller.c:73:1-6: WARNING: invalid free of devm_ allocated data

Silly mistake, should've changed this when the alloc was converted to
devm_.

@Nicolas, rest of this week is public holiday, but I'll try to
send a fix before the weekend. Should it go to you or to Arnd?

> 
> vim +73 drivers/soc/microchip/mpfs-sys-controller.c
> 
>     66	
>     67	static void mpfs_sys_controller_delete(struct kref *kref)
>     68	{
>     69		struct mpfs_sys_controller *sys_controller = container_of(kref, struct mpfs_sys_controller,
>     70						       consumers);
>     71	
>     72		mbox_free_channel(sys_controller->chan);
>   > 73		kfree(sys_controller);
>     74	}
>     75	
> 
> ---
> 0-DAY CI Kernel Test Service
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [linux-next:master 7918/13392] drivers/soc/microchip/mpfs-sys-controller.c:73:1-6: WARNING: invalid free of devm_ allocated data

[Nicolas Ferre]

Conor, Arnd,

On 17/03/2022 at 22:17, conor dooley wrote:
>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
>> head:   97add482e9033a9386baa8824fbea34d0cbfe663
>> commit: d0054a470c33902f5ae88835ed8a8ecc3cf8faa4 [7918/13392] soc: add microchip polarfire soc system controller
>> config: ia64-randconfig-c004-20220317 (https://download.01.org/0day-ci/archive/20220318/202203180259.lgIylRZV-lkp@intel.com/config)
>> compiler: ia64-linux-gcc (GCC) 11.2.0
>>
>> If you fix the issue, kindly add following tag as appropriate
>> Reported-by: kernel test robot <lkp@intel.com>
>>
>>
>> cocci warnings: (new ones prefixed by >>)
>>>> drivers/soc/microchip/mpfs-sys-controller.c:73:1-6: WARNING: invalid free of devm_ allocated data
> 
> Silly mistake, should've changed this when the alloc was converted to
> devm_.
> 
> @Nicolas, rest of this week is public holiday, but I'll try to
> send a fix before the weekend. Should it go to you or to Arnd?

I cannot queue in my branches because otherwise it will conflict with 
the patch already taken by Arnd as an individual patch [1]:
"soc: microchip: make mpfs_sys_controller_put static"

What I propose is that we deal with this during the stabilization phase 
as a "fix" after 5.18-rc1 is released (in 2 weeks).

Is there a drawback to this approach in the sense that it can trigger 
other test robots?

This way, we won't have to disturb Arnd as I believe that his branches 
are all ready to be sent to Linus after this week-end. Additional 
benefit, you don't have to change your plans and enjoy St Patrick 
week-end ;-).

Best regards,
   Nicolas

[1]: 
https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git/commit/?h=for-next&id=17588e867e694e8a03234628145f7c4d738288e2


>> vim +73 drivers/soc/microchip/mpfs-sys-controller.c
>>
>>      66
>>      67        static void mpfs_sys_controller_delete(struct kref *kref)
>>      68        {
>>      69                struct mpfs_sys_controller *sys_controller = container_of(kref, struct mpfs_sys_controller,
>>      70                                                       consumers);
>>      71
>>      72                mbox_free_channel(sys_controller->chan);
>>    > 73                kfree(sys_controller);
>>      74        }
>>      75
>>
>> ---
>> 0-DAY CI Kernel Test Service
>> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org


-- 
Nicolas Ferre

Re: [linux-next:master 7918/13392] drivers/soc/microchip/mpfs-sys-controller.c:73:1-6: WARNING: invalid free of devm_ allocated data

[Nicolas Ferre]

(+ Arnd)

Conor, Arnd,

On 17/03/2022 at 22:17, conor dooley wrote:
>> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git master
>> head:   97add482e9033a9386baa8824fbea34d0cbfe663
>> commit: d0054a470c33902f5ae88835ed8a8ecc3cf8faa4 [7918/13392] soc: add microchip polarfire soc system controller
>> config: ia64-randconfig-c004-20220317 (https://download.01.org/0day-ci/archive/20220318/202203180259.lgIylRZV-lkp@intel.com/config)
>> compiler: ia64-linux-gcc (GCC) 11.2.0
>>
>> If you fix the issue, kindly add following tag as appropriate
>> Reported-by: kernel test robot <lkp@intel.com>
>>
>>
>> cocci warnings: (new ones prefixed by >>)
>>>> drivers/soc/microchip/mpfs-sys-controller.c:73:1-6: WARNING: invalid free of devm_ allocated data
> 
> Silly mistake, should've changed this when the alloc was converted to
> devm_.
> 
> @Nicolas, rest of this week is public holiday, but I'll try to
> send a fix before the weekend. Should it go to you or to Arnd?

I cannot queue in my branches because otherwise it will conflict with 
the patch already taken by Arnd as an individual patch [1]:
"soc: microchip: make mpfs_sys_controller_put static"

What I propose is that we deal with this during the stabilization phase 
as a "fix" after 5.18-rc1 is released (in 2 weeks).

Is there a drawback to this approach in the sense that it can trigger 
other test robots?

This way, we won't have to disturb Arnd as I believe that his branches 
are all ready to be sent to Linus after this week-end. Additional 
benefit, you don't have to change your plans and enjoy St Patrick 
week-end ;-).

Best regards,
    Nicolas

[1]: 
https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git/commit/?h=for-next&id=17588e867e694e8a03234628145f7c4d738288e2


>> vim +73 drivers/soc/microchip/mpfs-sys-controller.c
>>
>>      66
>>      67        static void mpfs_sys_controller_delete(struct kref *kref)
>>      68        {
>>      69                struct mpfs_sys_controller *sys_controller = container_of(kref, struct mpfs_sys_controller,
>>      70                                                       consumers);
>>      71
>>      72                mbox_free_channel(sys_controller->chan);
>>    > 73                kfree(sys_controller);
>>      74        }
>>      75
>>
>> ---
>> 0-DAY CI Kernel Test Service
>> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org


-- 
Nicolas Ferre

Re: [linux-next:master 7918/13392] drivers/soc/microchip/mpfs-sys-controller.c:73:1-6: WARNING: invalid free of devm_ allocated data

[Conor Dooley]

On 18/03/2022 09:43, Nicolas Ferre wrote:
> (+ Arnd)
> 
> Conor, Arnd,
> 
> On 17/03/2022 at 22:17, conor dooley wrote:
>>> tree:   
>>> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git 
>>> master
>>> head:   97add482e9033a9386baa8824fbea34d0cbfe663
>>> commit: d0054a470c33902f5ae88835ed8a8ecc3cf8faa4 [7918/13392] soc: 
>>> add microchip polarfire soc system controller
>>> config: ia64-randconfig-c004-20220317 
>>> (https://download.01.org/0day-ci/archive/20220318/202203180259.lgIylRZV-lkp@intel.com/config) 
>>>
>>> compiler: ia64-linux-gcc (GCC) 11.2.0
>>>
>>> If you fix the issue, kindly add following tag as appropriate
>>> Reported-by: kernel test robot <lkp@intel.com>
>>>
>>>
>>> cocci warnings: (new ones prefixed by >>)
>>>>> drivers/soc/microchip/mpfs-sys-controller.c:73:1-6: WARNING: 
>>>>> invalid free of devm_ allocated data
>>
>> Silly mistake, should've changed this when the alloc was converted to
>> devm_.
>>
>> @Nicolas, rest of this week is public holiday, but I'll try to
>> send a fix before the weekend. Should it go to you or to Arnd?
> 
> I cannot queue in my branches because otherwise it will conflict with 
> the patch already taken by Arnd as an individual patch [1]:
> "soc: microchip: make mpfs_sys_controller_put static"
> 
> What I propose is that we deal with this during the stabilization phase 
> as a "fix" after 5.18-rc1 is released (in 2 weeks).

If that fine by you (plural) then that's fine by me too :)

> 
> Is there a drawback to this approach in the sense that it can trigger 
> other test robots?
> 
> This way, we won't have to disturb Arnd as I believe that his branches 
> are all ready to be sent to Linus after this week-end. Additional 
> benefit, you don't have to change your plans and enjoy St Patrick 
> week-end ;-).
> 
> Best regards,
>     Nicolas
> 
> [1]: 
> https://git.kernel.org/pub/scm/linux/kernel/git/soc/soc.git/commit/?h=for-next&id=17588e867e694e8a03234628145f7c4d738288e2 
> 
> 
> 
>>> vim +73 drivers/soc/microchip/mpfs-sys-controller.c
>>>
>>>      66
>>>      67        static void mpfs_sys_controller_delete(struct kref *kref)
>>>      68        {
>>>      69                struct mpfs_sys_controller *sys_controller = 
>>> container_of(kref, struct mpfs_sys_controller,
>>>      70                                                       
>>> consumers);
>>>      71
>>>      72                mbox_free_channel(sys_controller->chan);
>>>    > 73                kfree(sys_controller);
>>>      74        }
>>>      75
>>>
>>> ---
>>> 0-DAY CI Kernel Test Service
>>> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
> 
> 

Re: [linux-next:master 7918/13392] drivers/soc/microchip/mpfs-sys-controller.c:73:1-6: WARNING: invalid free of devm_ allocated data

[Arnd Bergmann]

On Fri, Mar 18, 2022 at 10:43 AM Nicolas Ferre
<nicolas.ferre@microchip.com> wrote:

> Is there a drawback to this approach in the sense that it can trigger
> other test robots?
>
> This way, we won't have to disturb Arnd as I believe that his branches
> are all ready to be sent to Linus after this week-end. Additional
> benefit, you don't have to change your plans and enjoy St Patrick
> week-end ;-).

I don't mind taking last minute fixes for buggy patches that I have queued
up, that seems better than sending the wrong code first and fixing it up
later. I would fix it myself, but I don't immediately see what the
correct fix it.
Do we just remove the kfree(), or does that conflict with the lifetime rules
in case we unbind the driver before the put() callback is called?

       Arnd

Re: [linux-next:master 7918/13392] drivers/soc/microchip/mpfs-sys-controller.c:73:1-6: WARNING: invalid free of devm_ allocated data

[Conor Dooley]

On 18/03/2022 11:02, Arnd Bergmann wrote:
> On Fri, Mar 18, 2022 at 10:43 AM Nicolas Ferre
> <nicolas.ferre@microchip.com> wrote:
> 
>> Is there a drawback to this approach in the sense that it can trigger
>> other test robots?
>>
>> This way, we won't have to disturb Arnd as I believe that his branches
>> are all ready to be sent to Linus after this week-end. Additional
>> benefit, you don't have to change your plans and enjoy St Patrick
>> week-end ;-).
> 
> I don't mind taking last minute fixes for buggy patches that I have queued
> up, that seems better than sending the wrong code first and fixing it up
> later. I would fix it myself, but I don't immediately see what the
> correct fix it.
> Do we just remove the kfree(), or does that conflict with the lifetime rules
> in case we unbind the driver before the put() callback is called?

I was just going to remove the kfree, but I don't think that's right.
I'll switch it back to using kzalloc & leave the kfree. I'll send a 
patch later today.

Thanks,
Conor.

Re: [linux-next:master 7918/13392] drivers/soc/microchip/mpfs-sys-controller.c:73:1-6: WARNING: invalid free of devm_ allocated data

[Conor.Dooley]

On 18/03/2022 11:59, Conor Dooley wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 18/03/2022 11:02, Arnd Bergmann wrote:
>> On Fri, Mar 18, 2022 at 10:43 AM Nicolas Ferre
>> <nicolas.ferre@microchip.com> wrote:
>>
>>> Is there a drawback to this approach in the sense that it can trigger
>>> other test robots?
>>>
>>> This way, we won't have to disturb Arnd as I believe that his branches
>>> are all ready to be sent to Linus after this week-end. Additional
>>> benefit, you don't have to change your plans and enjoy St Patrick
>>> week-end ;-).
>>
>> I don't mind taking last minute fixes for buggy patches that I have queued
>> up, that seems better than sending the wrong code first and fixing it up
>> later. I would fix it myself, but I don't immediately see what the
>> correct fix it.
>> Do we just remove the kfree(), or does that conflict with the lifetime rules
>> in case we unbind the driver before the put() callback is called?
> 
> I was just going to remove the kfree, but I don't think that's right.
> I'll switch it back to using kzalloc & leave the kfree. I'll send a
> patch later today.

Just to follow up, I sent a fix on Friday:
https://lore.kernel.org/linux-riscv/20220318172107.1599399-1-mail@conchuod.ie/

Re: [linux-next:master 7918/13392] drivers/soc/microchip/mpfs-sys-controller.c:73:1-6: WARNING: invalid free of devm_ allocated data

[Nicolas Ferre]

On 18/03/2022 at 12:02, Arnd Bergmann wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On Fri, Mar 18, 2022 at 10:43 AM Nicolas Ferre
> <nicolas.ferre@microchip.com> wrote:
> 
>> Is there a drawback to this approach in the sense that it can trigger
>> other test robots?
>>
>> This way, we won't have to disturb Arnd as I believe that his branches
>> are all ready to be sent to Linus after this week-end. Additional
>> benefit, you don't have to change your plans and enjoy St Patrick
>> week-end ;-).
> 
> I don't mind taking last minute fixes for buggy patches that I have queued
> up, that seems better than sending the wrong code first and fixing it up
> later.

Fair enough.

> I would fix it myself, but I don't immediately see what the
> correct fix it.
> Do we just remove the kfree(), or does that conflict with the lifetime rules
> in case we unbind the driver before the put() callback is called?

Thanks Arnd! Best regards,
   Nicolas


-- 
Nicolas Ferre