Messages in this thread | | | Date | Wed, 1 Mar 2023 10:31:38 -0800 | Subject | Re: [PATCH] watchdog: avoid usage of iterator after loop | From | Guenter Roeck <> |
| |
On 3/1/23 09:17, Jakob Koschel wrote: > If potentially no valid element is found, 'p' would contain an invalid > pointer past the iterator loop. To ensure 'p' is valid under any > circumstances, the kfree() should be within the loop body. > > Additionally, Linus proposed to avoid any use of the list iterator > variable after the loop, in the attempt to move the list iterator > variable declaration into the marcro to avoid any potential misuse after
macro
> the loop [1]. > > Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1] > Signed-off-by: Jakob Koschel <jkl820.git@gmail.com> > --- > drivers/watchdog/watchdog_pretimeout.c | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/drivers/watchdog/watchdog_pretimeout.c b/drivers/watchdog/watchdog_pretimeout.c > index 376a495ab80c..d8c78696eaf5 100644 > --- a/drivers/watchdog/watchdog_pretimeout.c > +++ b/drivers/watchdog/watchdog_pretimeout.c > @@ -207,10 +207,10 @@ void watchdog_unregister_pretimeout(struct watchdog_device *wdd) > list_for_each_entry_safe(p, t, &pretimeout_list, entry) { > if (p->wdd == wdd) { > list_del(&p->entry); > - break; > + spin_unlock_irq(&pretimeout_lock); > + kfree(p); > + return;
Please just make it kfree(p); break;
There is no need to drop the spinlock here and/or to return directly.
Thanks, Guenter
> } > } > spin_unlock_irq(&pretimeout_lock); > - > - kfree(p); > } > > --- > base-commit: c0927a7a5391f7d8e593e5e50ead7505a23cadf9 > change-id: 20230301-watchdog-avoid-iter-after-loop-a197bf201435 > > Best regards,
| |