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

Jakob Koschel posted 1 patch 2 years, 6 months ago
fs/locks.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
[PATCH] locks: avoid usage of list iterator after loop in generic_delete_lease()
Posted by Jakob Koschel 2 years, 6 months ago
'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 <jkl820.git@gmail.com>
---
 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 <jkl820.git@gmail.com>
Re: [PATCH] locks: avoid usage of list iterator after loop in generic_delete_lease()
Posted by Al Viro 2 years, 6 months ago
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 <jkl820.git@gmail.com>

Looks sane.  Jeff, which tree do you want that to go through?
Re: [PATCH] locks: avoid usage of list iterator after loop in generic_delete_lease()
Posted by Jeff Layton 2 years, 6 months ago
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 <jkl820.git@gmail.com>
> 
> 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 <jlayton@kernel.org>

Thanks!