posix_acl: make posix_acl_to_xattr() alloc the buffer

Miklos Szeredi · brauner · commit 6cbfdf89470e · 2026-01-16T10:51:12.000+01:00
Without exception all caller do that. So move the allocation into the helper. This reduces boilerplate and removes unnecessary error checking. Signed-off-by: Miklos Szeredi <mszeredi@redhat.com> Link: https://patch.msgid.link/20260115122341.556026-1-mszeredi@redhat.com Signed-off-by: Christian Brauner <brauner@kernel.org>
diff --git a/fs/9p/acl.c b/fs/9p/acl.c
@@ -167,17 +167,11 @@ int v9fs_iop_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
 		if (retval)
 			goto err_out;
 
-		size = posix_acl_xattr_size(acl->a_count);
-
-		value = kzalloc(size, GFP_NOFS);
+		value = posix_acl_to_xattr(&init_user_ns, acl, &size, GFP_NOFS);
 		if (!value) {
 			retval = -ENOMEM;
 			goto err_out;
 		}
-
-		retval = posix_acl_to_xattr(&init_user_ns, acl, value, size);
-		if (retval < 0)
-			goto err_out;
 	}
 
 	/*
@@ -257,13 +251,10 @@ static int v9fs_set_acl(struct p9_fid *fid, int type, struct posix_acl *acl)
 		return 0;
 
 	/* Set a setxattr request to server */
-	size = posix_acl_xattr_size(acl->a_count);
-	buffer = kmalloc(size, GFP_KERNEL);
+	buffer = posix_acl_to_xattr(&init_user_ns, acl, &size, GFP_KERNEL);
 	if (!buffer)
 		return -ENOMEM;
-	retval = posix_acl_to_xattr(&init_user_ns, acl, buffer, size);
-	if (retval < 0)
-		goto err_free_out;
+
 	switch (type) {
 	case ACL_TYPE_ACCESS:
 		name = XATTR_NAME_POSIX_ACL_ACCESS;
@@ -275,7 +266,6 @@ static int v9fs_set_acl(struct p9_fid *fid, int type, struct posix_acl *acl)
 		BUG();
 	}
 	retval = v9fs_fid_xattr_set(fid, name, buffer, size, 0);
-err_free_out:
 	kfree(buffer);
 	return retval;
 }
diff --git a/fs/btrfs/acl.c b/fs/btrfs/acl.c
@@ -57,7 +57,8 @@ struct posix_acl *btrfs_get_acl(struct inode *inode, int type, bool rcu)
 int __btrfs_set_acl(struct btrfs_trans_handle *trans, struct inode *inode,
 		    struct posix_acl *acl, int type)
 {
-	int ret, size = 0;
+	int ret;
+	size_t size = 0;
 	const char *name;
 	char AUTO_KFREE(value);
 
@@ -77,20 +78,15 @@ int __btrfs_set_acl(struct btrfs_trans_handle *trans, struct inode *inode,
 	if (acl) {
 		unsigned int nofs_flag;
 
-		size = posix_acl_xattr_size(acl->a_count);
 		/*
 		 * We're holding a transaction handle, so use a NOFS memory
 		 * allocation context to avoid deadlock if reclaim happens.
 		 */
 		nofs_flag = memalloc_nofs_save();
-		value = kmalloc(size, GFP_KERNEL);
+		value = posix_acl_to_xattr(&init_user_ns, acl, &size, GFP_KERNEL);
 		memalloc_nofs_restore(nofs_flag);
 		if (!value)
 			return -ENOMEM;
-
-		ret = posix_acl_to_xattr(&init_user_ns, acl, value, size);
-		if (ret < 0)
-			return ret;
 	}
 
 	if (trans)
diff --git a/fs/ceph/acl.c b/fs/ceph/acl.c
@@ -90,7 +90,8 @@ struct posix_acl *ceph_get_acl(struct inode *inode, int type, bool rcu)
 int ceph_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
 		 struct posix_acl *acl, int type)
 {
-	int ret = 0, size = 0;
+	int ret = 0;
+	size_t size = 0;
 	const char *name = NULL;
 	char *value = NULL;
 	struct iattr newattrs;
@@ -126,16 +127,11 @@ int ceph_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
 	}
 
 	if (acl) {
-		size = posix_acl_xattr_size(acl->a_count);
-		value = kmalloc(size, GFP_NOFS);
+		value = posix_acl_to_xattr(&init_user_ns, acl, &size, GFP_NOFS);
 		if (!value) {
 			ret = -ENOMEM;
 			goto out;
 		}
-
-		ret = posix_acl_to_xattr(&init_user_ns, acl, value, size);
-		if (ret < 0)
-			goto out_free;
 	}
 
 	if (new_mode != old_mode) {
@@ -172,7 +168,7 @@ int ceph_pre_init_acls(struct inode *dir, umode_t *mode,
 	struct posix_acl *acl, *default_acl;
 	size_t val_size1 = 0, val_size2 = 0;
 	struct ceph_pagelist *pagelist = NULL;
-	void *tmp_buf = NULL;
+	void *tmp_buf1 = NULL, *tmp_buf2 = NULL;
 	int err;
 
 	err = posix_acl_create(dir, mode, &default_acl, &acl);
@@ -192,15 +188,7 @@ int ceph_pre_init_acls(struct inode *dir, umode_t *mode,
 	if (!default_acl && !acl)
 		return 0;
 
-	if (acl)
-		val_size1 = posix_acl_xattr_size(acl->a_count);
-	if (default_acl)
-		val_size2 = posix_acl_xattr_size(default_acl->a_count);
-
 	err = -ENOMEM;
-	tmp_buf = kmalloc(max(val_size1, val_size2), GFP_KERNEL);
-	if (!tmp_buf)
-		goto out_err;
 	pagelist = ceph_pagelist_alloc(GFP_KERNEL);
 	if (!pagelist)
 		goto out_err;
@@ -213,34 +201,39 @@ int ceph_pre_init_acls(struct inode *dir, umode_t *mode,
 
 	if (acl) {
 		size_t len = strlen(XATTR_NAME_POSIX_ACL_ACCESS);
+
+		err = -ENOMEM;
+		tmp_buf1 = posix_acl_to_xattr(&init_user_ns, acl,
+					      &val_size1, GFP_KERNEL);
+		if (!tmp_buf1)
+			goto out_err;
 		err = ceph_pagelist_reserve(pagelist, len + val_size1 + 8);
 		if (err)
 			goto out_err;
 		ceph_pagelist_encode_string(pagelist, XATTR_NAME_POSIX_ACL_ACCESS,
 					    len);
-		err = posix_acl_to_xattr(&init_user_ns, acl,
-					 tmp_buf, val_size1);
-		if (err < 0)
-			goto out_err;
 		ceph_pagelist_encode_32(pagelist, val_size1);
-		ceph_pagelist_append(pagelist, tmp_buf, val_size1);
+		ceph_pagelist_append(pagelist, tmp_buf1, val_size1);
 	}
 	if (default_acl) {
 		size_t len = strlen(XATTR_NAME_POSIX_ACL_DEFAULT);
+
+		err = -ENOMEM;
+		tmp_buf2 = posix_acl_to_xattr(&init_user_ns, default_acl,
+					      &val_size2, GFP_KERNEL);
+		if (!tmp_buf2)
+			goto out_err;
 		err = ceph_pagelist_reserve(pagelist, len + val_size2 + 8);
 		if (err)
 			goto out_err;
 		ceph_pagelist_encode_string(pagelist,
 					  XATTR_NAME_POSIX_ACL_DEFAULT, len);
-		err = posix_acl_to_xattr(&init_user_ns, default_acl,
-					 tmp_buf, val_size2);
-		if (err < 0)
-			goto out_err;
 		ceph_pagelist_encode_32(pagelist, val_size2);
-		ceph_pagelist_append(pagelist, tmp_buf, val_size2);
+		ceph_pagelist_append(pagelist, tmp_buf2, val_size2);
 	}
 
-	kfree(tmp_buf);
+	kfree(tmp_buf1);
+	kfree(tmp_buf2);
 
 	as_ctx->acl = acl;
 	as_ctx->default_acl = default_acl;
@@ -250,7 +243,8 @@ int ceph_pre_init_acls(struct inode *dir, umode_t *mode,
 out_err:
 	posix_acl_release(acl);
 	posix_acl_release(default_acl);
-	kfree(tmp_buf);
+	kfree(tmp_buf1);
+	kfree(tmp_buf2);
 	if (pagelist)
 		ceph_pagelist_release(pagelist);
 	return err;
diff --git a/fs/fuse/acl.c b/fs/fuse/acl.c
@@ -122,20 +122,16 @@ int fuse_set_acl(struct mnt_idmap *idmap, struct dentry *dentry,
 		 * them to be refreshed the next time they are used,
 		 * and it also updates i_ctime.
 		 */
-		size_t size = posix_acl_xattr_size(acl->a_count);
+		size_t size;
 		void *value;
 
-		if (size > PAGE_SIZE)
-			return -E2BIG;
-
-		value = kmalloc(size, GFP_KERNEL);
+		value = posix_acl_to_xattr(fc->user_ns, acl, &size, GFP_KERNEL);
 		if (!value)
 			return -ENOMEM;
 
-		ret = posix_acl_to_xattr(fc->user_ns, acl, value, size);
-		if (ret < 0) {
+		if (size > PAGE_SIZE) {
 			kfree(value);
-			return ret;
+			return -E2BIG;
 		}
 
 		/*
diff --git a/fs/gfs2/acl.c b/fs/gfs2/acl.c
@@ -83,21 +83,14 @@ struct posix_acl *gfs2_get_acl(struct inode *inode, int type, bool rcu)
 int __gfs2_set_acl(struct inode *inode, struct posix_acl *acl, int type)
 {
 	int error;
-	size_t len;
-	char *data;
+	size_t len = 0;
+	char *data = NULL;
 	const char *name = gfs2_acl_name(type);
 
 	if (acl) {
-		len = posix_acl_xattr_size(acl->a_count);
-		data = kmalloc(len, GFP_NOFS);
+		data = posix_acl_to_xattr(&init_user_ns, acl, &len, GFP_NOFS);
 		if (data == NULL)
 			return -ENOMEM;
-		error = posix_acl_to_xattr(&init_user_ns, acl, data, len);
-		if (error < 0)
-			goto out;
-	} else {
-		data = NULL;
-		len = 0;
 	}
 
 	error = __gfs2_xattr_set(inode, name, data, len, 0, GFS2_EATYPE_SYS);
diff --git a/fs/jfs/acl.c b/fs/jfs/acl.c
@@ -61,7 +61,7 @@ static int __jfs_set_acl(tid_t tid, struct inode *inode, int type,
 {
 	char *ea_name;
 	int rc;
-	int size = 0;
+	size_t size = 0;
 	char *value = NULL;
 
 	switch (type) {
@@ -76,16 +76,11 @@ static int __jfs_set_acl(tid_t tid, struct inode *inode, int type,
 	}
 
 	if (acl) {
-		size = posix_acl_xattr_size(acl->a_count);
-		value = kmalloc(size, GFP_KERNEL);
+		value = posix_acl_to_xattr(&init_user_ns, acl, &size, GFP_KERNEL);
 		if (!value)
 			return -ENOMEM;
-		rc = posix_acl_to_xattr(&init_user_ns, acl, value, size);
-		if (rc < 0)
-			goto out;
 	}
 	rc = __jfs_setxattr(tid, inode, ea_name, value, size, 0);
-out:
 	kfree(value);
 
 	if (!rc)
diff --git a/fs/ntfs3/xattr.c b/fs/ntfs3/xattr.c
@@ -641,13 +641,9 @@ static noinline int ntfs_set_acl_ex(struct mnt_idmap *idmap,
 		value = NULL;
 		flags = XATTR_REPLACE;
 	} else {
-		size = posix_acl_xattr_size(acl->a_count);
-		value = kmalloc(size, GFP_NOFS);
+		value = posix_acl_to_xattr(&init_user_ns, acl, &size, GFP_NOFS);
 		if (!value)
 			return -ENOMEM;
-		err = posix_acl_to_xattr(&init_user_ns, acl, value, size);
-		if (err < 0)
-			goto out;
 		flags = 0;
 	}
 
diff --git a/fs/orangefs/acl.c b/fs/orangefs/acl.c
@@ -90,14 +90,9 @@ int __orangefs_set_acl(struct inode *inode, struct posix_acl *acl, int type)
 		     type);
 
 	if (acl) {
-		size = posix_acl_xattr_size(acl->a_count);
-		value = kmalloc(size, GFP_KERNEL);
+		value = posix_acl_to_xattr(&init_user_ns, acl, &size, GFP_KERNEL);
 		if (!value)
 			return -ENOMEM;
-
-		error = posix_acl_to_xattr(&init_user_ns, acl, value, size);
-		if (error < 0)
-			goto out;
 	}
 
 	gossip_debug(GOSSIP_ACL_DEBUG,
@@ -111,7 +106,6 @@ int __orangefs_set_acl(struct inode *inode, struct posix_acl *acl, int type)
 	 */
 	error = orangefs_inode_setxattr(inode, name, value, size, 0);
 
-out:
 	kfree(value);
 	if (!error)
 		set_cached_acl(inode, type, acl);
diff --git a/fs/posix_acl.c b/fs/posix_acl.c
@@ -829,19 +829,19 @@ EXPORT_SYMBOL (posix_acl_from_xattr);
 /*
  * Convert from in-memory to extended attribute representation.
  */
-int
+void *
 posix_acl_to_xattr(struct user_namespace *user_ns, const struct posix_acl *acl,
-		   void *buffer, size_t size)
+		   size_t *sizep, gfp_t gfp)
 {
-	struct posix_acl_xattr_header *ext_acl = buffer;
+	struct posix_acl_xattr_header *ext_acl;
 	struct posix_acl_xattr_entry *ext_entry;
-	int real_size, n;
+	size_t size;
+	int n;
 
-	real_size = posix_acl_xattr_size(acl->a_count);
-	if (!buffer)
-		return real_size;
-	if (real_size > size)
-		return -ERANGE;
+	size = posix_acl_xattr_size(acl->a_count);
+	ext_acl = kmalloc(size, gfp);
+	if (!ext_acl)
+		return NULL;
 
 	ext_entry = (void *)(ext_acl + 1);
 	ext_acl->a_version = cpu_to_le32(POSIX_ACL_XATTR_VERSION);
@@ -864,7 +864,8 @@ posix_acl_to_xattr(struct user_namespace *user_ns, const struct posix_acl *acl,
 			break;
 		}
 	}
-	return real_size;
+	*sizep = size;
+	return ext_acl;
 }
 EXPORT_SYMBOL (posix_acl_to_xattr);
 
diff --git a/include/linux/posix_acl_xattr.h b/include/linux/posix_acl_xattr.h
@@ -44,8 +44,9 @@ posix_acl_from_xattr(struct user_namespace *user_ns, const void *value,
 }
 #endif
 
-int posix_acl_to_xattr(struct user_namespace *user_ns,
-		       const struct posix_acl *acl, void *buffer, size_t size);
+extern void *posix_acl_to_xattr(struct user_namespace *user_ns, const struct posix_acl *acl,
+				size_t *sizep, gfp_t gfp);
+
 static inline const char *posix_acl_xattr_name(int type)
 {
 	switch (type) {

Original file line number	Diff line number	Diff line change
`@@ -44,8 +44,9 @@ posix_acl_from_xattr(struct user_namespace user_ns, const void value,`
`44`	`44`	`}`
`45`	`45`	`#endif`
`46`	`46`
`47`		`-int posix_acl_to_xattr(struct user_namespace *user_ns,`
`48`		`- const struct posix_acl acl, void buffer, size_t size);`
	`47`	`+extern void posix_acl_to_xattr(struct user_namespace user_ns, const struct posix_acl *acl,`
	`48`	`+ size_t *sizep, gfp_t gfp);`
	`49`	`+`
`49`	`50`	`static inline const char *posix_acl_xattr_name(int type)`
`50`	`51`	`{`
`51`	`52`	`switch (type) {`