Some callers of vfs_getxattr_alloc() assume that on failure the allocated
buffer does not need to be freed.
Callers could be fixed, but fixing the semantics of vfs_getxattr_alloc() is
simpler and makes sure that this class of bugs does not occur again.
If this was called in a loop (i.e. xattr_value contains an already
allocated buffer), then caller will still need to clean up after an error.
Reported-and-tested-by: syzbot+942d5390db2d9624ced8@xxxxxxxxxxxxxxxxxxxxxxxxx
Fixes: 1601fbad2b14 ("xattr: define vfs_getxattr_alloc and vfs_xattr_cmp")
Signed-off-by: Miklos Szeredi <mszeredi@xxxxxxxxxx>
---
fs/xattr.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/fs/xattr.c b/fs/xattr.c
index e8dd03e4561e..c03a957902da 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -383,7 +383,10 @@ vfs_getxattr_alloc(struct user_namespace *mnt_userns, struct dentry *dentry,
}
error = handler->get(handler, dentry, inode, name, value, error);
- *xattr_value = value;
+ if (error < 0 && *xattr_value == NULL)
+ kfree(value);
+ else
+ *xattr_value = value;
return error;
}
--
2.35.3
