2023-03-01 17:20:30

by Jakob Koschel

Subject: [PATCH] locks: avoid usage of list iterator after loop in generic_delete_lease()

'victim' and 'fl' are ensured to be equal at this point. For consistency
both should use the same variable.

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
the loop [1].

Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1]
Signed-off-by: Jakob Koschel <[email protected]>
---
fs/locks.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/locks.c b/fs/locks.c
index 66b4eef09db5..3f46d21a95f4 100644
--- a/fs/locks.c
+++ b/fs/locks.c
@@ -1841,7 +1841,7 @@ static int generic_delete_lease(struct file *filp, void *owner)
}
trace_generic_delete_lease(inode, victim);
if (victim)
- error = fl->fl_lmops->lm_change(victim, F_UNLCK, &dispose);
+ error = victim->fl_lmops->lm_change(victim, F_UNLCK, &dispose);
spin_unlock(&ctx->flc_lock);
percpu_up_read(&file_rwsem);
locks_dispose_list(&dispose);

---
base-commit: c0927a7a5391f7d8e593e5e50ead7505a23cadf9
change-id: 20230301-locks-avoid-iter-after-loop-0c6cc0fbd295

Best regards,
--
Jakob Koschel <[email protected]>

2023-03-02 18:15:13

by Al Viro

Subject: Re: [PATCH] locks: avoid usage of list iterator after loop in generic_delete_lease()

On Wed, Mar 01, 2023 at 06:20:18PM +0100, Jakob Koschel wrote:
> 'victim' and 'fl' are ensured to be equal at this point. For consistency
> both should use the same variable.
>
> 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
> the loop [1].
>
> Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1]
> Signed-off-by: Jakob Koschel <[email protected]>

Looks sane. Jeff, which tree do you want that to go through?

2023-03-02 20:35:24

by Jeffrey Layton

Subject: Re: [PATCH] locks: avoid usage of list iterator after loop in generic_delete_lease()

On Thu, 2023-03-02 at 18:14 +0000, Al Viro wrote:
> On Wed, Mar 01, 2023 at 06:20:18PM +0100, Jakob Koschel wrote:
> > 'victim' and 'fl' are ensured to be equal at this point. For consistency
> > both should use the same variable.
> >
> > 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
> > the loop [1].
> >
> > Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/ [1]
> > Signed-off-by: Jakob Koschel <[email protected]>
>
> Looks sane. Jeff, which tree do you want that to go through?

Looks good to me too. I'm fine if you want to pick up that one, but
otherwise I can take it. You can also add:

Reviewed-by: Jeff Layton <[email protected]>

Thanks!