[PATCH] afs: Merge preference rule failure condition

Lizhi Xu posted 1 patch 1 year, 1 month ago
fs/afs/addr_prefs.c | 2 --
1 file changed, 2 deletions(-)
[PATCH] afs: Merge preference rule failure condition
Posted by Lizhi Xu 1 year, 1 month ago
syzbot reported a lock held when returning to user space. [1]

If argc is less than 0 and the function returns directly, the held inode
lock is not released. Combine it with less than 2.

[1]
WARNING: lock held when returning to user space!
6.13.0-rc3-syzkaller-00209-g499551201b5f #0 Not tainted
------------------------------------------------
syz-executor133/5823 is leaving the kernel with locks still held!
1 lock held by syz-executor133/5823:
 #0: ffff888071cffc00 (&sb->s_type->i_mutex_key#9){++++}-{4:4}, at: inode_lock include/linux/fs.h:818 [inline]
 #0: ffff888071cffc00 (&sb->s_type->i_mutex_key#9){++++}-{4:4}, at: afs_proc_addr_prefs_write+0x2bb/0x14e0 fs/afs/addr_prefs.c:388

Reported-by: syzbot+76f33569875eb708e575@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=76f33569875eb708e575
Tested-by: syzbot+76f33569875eb708e575@syzkaller.appspotmail.com
Signed-off-by: Lizhi Xu <lizhi.xu@windriver.com>
---
 fs/afs/addr_prefs.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/fs/afs/addr_prefs.c b/fs/afs/addr_prefs.c
index a189ff8a5034..ba391f8558d5 100644
--- a/fs/afs/addr_prefs.c
+++ b/fs/afs/addr_prefs.c
@@ -413,8 +413,6 @@ int afs_proc_addr_prefs_write(struct file *file, char *buf, size_t size)
 
 	do {
 		argc = afs_split_string(&buf, argv, ARRAY_SIZE(argv));
-		if (argc < 0)
-			return argc;
 		if (argc < 2)
 			goto inval;
 
-- 
2.43.0
Re: [PATCH] afs: Merge preference rule failure condition
Posted by David Howells 1 year, 1 month ago
Lizhi Xu <lizhi.xu@windriver.com> wrote:

>  		argc = afs_split_string(&buf, argv, ARRAY_SIZE(argv));
> -		if (argc < 0)
> -			return argc;
>  		if (argc < 2)
>  			goto inval;

I think this needs to be slightly different.  afs_split_string() will print
error messages and can return an error code, so we should go with that and set
ret to argc and go to done, not inval.

David