[PATCH] ocfs2: retain all security xattrs during inode creation

[PATCH] ocfs2: retain all security xattrs during inode creation

[Joseph Qi]

When creating a new inode, ocfs2_init_security_get() stores the
security xattr returned by the LSM framework in
ocfs2_security_xattr_info so that it can be written later within the
same transaction.  But ocfs2_initxattrs() only ever looks at the
first element of the xattr array, and it keeps the name as a bare
pointer into memory owned by the security layer instead of copying
it.  The value, by contrast, is already duplicated with kmemdup().

With stacked LSMs, security_inode_init_security() provides one xattr
per LSM, so all labels but the first are silently dropped and never
written to disk, leaving newly created files without the security
xattrs the other LSMs rely on.

The borrowed name is not an outright bug with the LSMs in tree
today: security_inode_init_security() frees only ->value once the
callback returns, and documents ->name as the attribute name
suffix, so the pointer stays valid for as long as ocfs2 uses it.
It is fragile though, as an LSM that allocates the name together
with the value would turn it into a use-after-free, so the rework
below copies the names as well.

Fix this by storing copies of all the security xattrs: allocate an
array in ocfs2_initxattrs(), copy the names and values, account for
all of them in the credit calculations, and write each one in
ocfs2_init_security_set().

While at it, switch the allocations from GFP_KERNEL to GFP_NOFS.
The callback runs in the inode creation path with the parent
directory locked, and the security layer allocates the very same
xattr array with GFP_NOFS, so direct reclaim must not be allowed to
recurse into the filesystem from here.

Fixes: de3004c874e7 ("ocfs2: Switch to security_inode_init_security()")
Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
---
 fs/ocfs2/namei.c |   6 +--
 fs/ocfs2/xattr.c | 122 ++++++++++++++++++++++++++++++++++++-----------
 fs/ocfs2/xattr.h |  12 +++--
 3 files changed, 105 insertions(+), 35 deletions(-)

diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
index 58c6061ed983..ef03f90e7265 100644
--- a/fs/ocfs2/namei.c
+++ b/fs/ocfs2/namei.c
@@ -248,7 +248,6 @@ static int ocfs2_mknod(struct mnt_idmap *idmap,
 	int want_meta = 0;
 	int xattr_credits = 0;
 	struct ocfs2_security_xattr_info si = {
-		.name = NULL,
 		.enable = 1,
 	};
 	int did_quota_inode = 0;
@@ -475,7 +474,7 @@ static int ocfs2_mknod(struct mnt_idmap *idmap,
 
 	brelse(new_fe_bh);
 	brelse(parent_fe_bh);
-	kfree(si.value);
+	ocfs2_free_security_xattrs(&si);
 
 	ocfs2_acl_init_release(&acl_state);
 
@@ -1831,7 +1830,6 @@ static int ocfs2_symlink(struct mnt_idmap *idmap,
 	int want_clusters = 0;
 	int xattr_credits = 0;
 	struct ocfs2_security_xattr_info si = {
-		.name = NULL,
 		.enable = 1,
 	};
 	int did_quota = 0, did_quota_inode = 0;
@@ -2063,7 +2061,7 @@ static int ocfs2_symlink(struct mnt_idmap *idmap,
 
 	brelse(new_fe_bh);
 	brelse(parent_fe_bh);
-	kfree(si.value);
+	ocfs2_free_security_xattrs(&si);
 	ocfs2_free_dir_lookup_result(&lookup);
 	if (inode_ac)
 		ocfs2_free_alloc_context(inode_ac);
diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
index a2d6c3d0f2e8..f2b7a53ed969 100644
--- a/fs/ocfs2/xattr.c
+++ b/fs/ocfs2/xattr.c
@@ -643,10 +643,15 @@ int ocfs2_calc_security_init(struct inode *dir,
 			     int *xattr_credits,
 			     struct ocfs2_alloc_context **xattr_ac)
 {
+	int i;
 	int ret = 0;
 	struct ocfs2_super *osb = OCFS2_SB(dir->i_sb);
-	int s_size = ocfs2_xattr_entry_real_size(strlen(si->name),
-						 si->value_len);
+	int s_size = 0;
+
+	for (i = 0; i < si->count; i++)
+		s_size += ocfs2_xattr_entry_real_size(
+				strlen(si->xattrs[i].name),
+				si->xattrs[i].value_len);
 
 	/*
 	 * The max space of security xattr taken inline is
@@ -664,13 +669,15 @@ int ocfs2_calc_security_init(struct inode *dir,
 	}
 
 	/* reserve clusters for xattr value which will be set in B tree*/
-	if (si->value_len > OCFS2_XATTR_INLINE_SIZE) {
-		int new_clusters = ocfs2_clusters_for_bytes(dir->i_sb,
-							    si->value_len);
-
-		*xattr_credits += ocfs2_clusters_to_blocks(dir->i_sb,
-							   new_clusters);
-		*want_clusters += new_clusters;
+	for (i = 0; i < si->count; i++) {
+		if (si->xattrs[i].value_len > OCFS2_XATTR_INLINE_SIZE) {
+			int new_clusters = ocfs2_clusters_for_bytes(dir->i_sb,
+						si->xattrs[i].value_len);
+
+			*xattr_credits += ocfs2_clusters_to_blocks(dir->i_sb,
+								   new_clusters);
+			*want_clusters += new_clusters;
+		}
 	}
 	return ret;
 }
@@ -680,12 +687,16 @@ void ocfs2_calc_xattr_init(struct inode *dir, umode_t mode,
 			   int *want_clusters, int *xattr_credits,
 			   int *want_meta, struct ocfs2_acl_state *acl_state)
 {
+	int i;
 	struct ocfs2_super *osb = OCFS2_SB(dir->i_sb);
 	int s_size = 0, a_size = 0, acl_len = 0, new_clusters;
 
-	if (si->enable)
-		s_size = ocfs2_xattr_entry_real_size(strlen(si->name),
-						     si->value_len);
+	if (si->enable) {
+		for (i = 0; i < si->count; i++)
+			s_size += ocfs2_xattr_entry_real_size(
+					strlen(si->xattrs[i].name),
+					si->xattrs[i].value_len);
+	}
 
 	if (osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL) {
 		if (acl_state->default_acl && S_ISDIR(mode)) {
@@ -732,12 +743,14 @@ void ocfs2_calc_xattr_init(struct inode *dir, umode_t mode,
 	 * reserve credits and clusters for xattrs which has large value
 	 * and have to be set outside
 	 */
-	if (si->enable && si->value_len > OCFS2_XATTR_INLINE_SIZE) {
-		new_clusters = ocfs2_clusters_for_bytes(dir->i_sb,
-							si->value_len);
-		*xattr_credits += ocfs2_clusters_to_blocks(dir->i_sb,
-							   new_clusters);
-		*want_clusters += new_clusters;
+	for (i = 0; si->enable && i < si->count; i++) {
+		if (si->xattrs[i].value_len > OCFS2_XATTR_INLINE_SIZE) {
+			new_clusters = ocfs2_clusters_for_bytes(dir->i_sb,
+						si->xattrs[i].value_len);
+			*xattr_credits += ocfs2_clusters_to_blocks(dir->i_sb,
+								   new_clusters);
+			*want_clusters += new_clusters;
+		}
 	}
 	if (osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL) {
 		if (acl_state->default_acl && S_ISDIR(mode)) {
@@ -7630,17 +7643,46 @@ static int ocfs2_initxattrs(struct inode *inode, const struct xattr *xattr_array
 {
 	struct ocfs2_security_xattr_info *si = fs_info;
 	const struct xattr *xattr;
+	struct ocfs2_security_xattr *xattrs;
+	int count = 0, i;
 	int err = 0;
 
 	if (si) {
-		si->value = kmemdup(xattr_array->value, xattr_array->value_len,
-				    GFP_KERNEL);
-		if (!si->value)
+		for (xattr = xattr_array; xattr->name != NULL; xattr++)
+			count++;
+
+		xattrs = kcalloc(count, sizeof(*xattrs), GFP_NOFS);
+		if (!xattrs)
 			return -ENOMEM;
 
-		si->name = xattr_array->name;
-		si->value_len = xattr_array->value_len;
+		for (i = 0; i < count; i++) {
+			xattrs[i].name = kstrdup(xattr_array[i].name, GFP_NOFS);
+			if (!xattrs[i].name) {
+				err = -ENOMEM;
+				goto out_err;
+			}
+
+			xattrs[i].value = kmemdup(xattr_array[i].value,
+						  xattr_array[i].value_len,
+						  GFP_NOFS);
+			if (!xattrs[i].value) {
+				err = -ENOMEM;
+				goto out_err;
+			}
+			xattrs[i].value_len = xattr_array[i].value_len;
+		}
+
+		si->xattrs = xattrs;
+		si->count = count;
 		return 0;
+
+out_err:
+		for (; i >= 0; i--) {
+			kfree(xattrs[i].name);
+			kfree(xattrs[i].value);
+		}
+		kfree(xattrs);
+		return err;
 	}
 
 	for (xattr = xattr_array; xattr->name != NULL; xattr++) {
@@ -7653,6 +7695,19 @@ static int ocfs2_initxattrs(struct inode *inode, const struct xattr *xattr_array
 	return err;
 }
 
+void ocfs2_free_security_xattrs(struct ocfs2_security_xattr_info *si)
+{
+	int i;
+
+	for (i = 0; i < si->count; i++) {
+		kfree(si->xattrs[i].name);
+		kfree(si->xattrs[i].value);
+	}
+	kfree(si->xattrs);
+	si->xattrs = NULL;
+	si->count = 0;
+}
+
 int ocfs2_init_security_get(struct inode *inode,
 			    struct inode *dir,
 			    const struct qstr *qstr,
@@ -7670,7 +7725,7 @@ int ocfs2_init_security_get(struct inode *inode,
 		 * security_inode_init_security() does not return -EOPNOTSUPP,
 		 * we have to check the xattr ourselves.
 		 */
-		if (!ret && !si->name)
+		if (!ret && !si->count)
 			si->enable = 0;
 
 		return ret;
@@ -7687,10 +7742,21 @@ int ocfs2_init_security_set(handle_t *handle,
 			    struct ocfs2_alloc_context *xattr_ac,
 			    struct ocfs2_alloc_context *data_ac)
 {
-	return ocfs2_xattr_set_handle(handle, inode, di_bh,
-				     OCFS2_XATTR_INDEX_SECURITY,
-				     si->name, si->value, si->value_len, 0,
-				     xattr_ac, data_ac);
+	int i;
+	int ret = 0;
+
+	for (i = 0; i < si->count; i++) {
+		ret = ocfs2_xattr_set_handle(handle, inode, di_bh,
+					     OCFS2_XATTR_INDEX_SECURITY,
+					     si->xattrs[i].name,
+					     si->xattrs[i].value,
+					     si->xattrs[i].value_len, 0,
+					     xattr_ac, data_ac);
+		if (ret)
+			break;
+	}
+
+	return ret;
 }
 
 const struct xattr_handler ocfs2_xattr_security_handler = {
diff --git a/fs/ocfs2/xattr.h b/fs/ocfs2/xattr.h
index 887cc1a18b1a..1dd08f979e3c 100644
--- a/fs/ocfs2/xattr.h
+++ b/fs/ocfs2/xattr.h
@@ -20,13 +20,18 @@ enum ocfs2_xattr_type {
 	OCFS2_XATTR_MAX
 };
 
-struct ocfs2_security_xattr_info {
-	int enable;
-	const char *name;
+struct ocfs2_security_xattr {
+	char *name;
 	void *value;
 	size_t value_len;
 };
 
+struct ocfs2_security_xattr_info {
+	int enable;
+	struct ocfs2_security_xattr *xattrs;
+	int count;
+};
+
 extern const struct xattr_handler ocfs2_xattr_user_handler;
 extern const struct xattr_handler ocfs2_xattr_trusted_handler;
 extern const struct xattr_handler ocfs2_xattr_security_handler;
@@ -49,6 +54,7 @@ int ocfs2_xattr_remove(struct inode *, struct buffer_head *);
 int ocfs2_init_security_get(struct inode *, struct inode *,
 			    const struct qstr *,
 			    struct ocfs2_security_xattr_info *);
+void ocfs2_free_security_xattrs(struct ocfs2_security_xattr_info *);
 int ocfs2_init_security_set(handle_t *, struct inode *,
 			    struct buffer_head *,
 			    struct ocfs2_security_xattr_info *,
-- 
2.39.3

Re: [PATCH] ocfs2: retain all security xattrs during inode creation

[Heming Zhao]

On Wed, Sep 16, 2026 at 08:02:58PM +0800, Joseph Qi wrote:
> When creating a new inode, ocfs2_init_security_get() stores the
> security xattr returned by the LSM framework in
> ocfs2_security_xattr_info so that it can be written later within the
> same transaction.  But ocfs2_initxattrs() only ever looks at the
> first element of the xattr array, and it keeps the name as a bare
> pointer into memory owned by the security layer instead of copying
> it.  The value, by contrast, is already duplicated with kmemdup().
> 
> With stacked LSMs, security_inode_init_security() provides one xattr
> per LSM, so all labels but the first are silently dropped and never
> written to disk, leaving newly created files without the security
> xattrs the other LSMs rely on.
> 
> The borrowed name is not an outright bug with the LSMs in tree
> today: security_inode_init_security() frees only ->value once the
> callback returns, and documents ->name as the attribute name
> suffix, so the pointer stays valid for as long as ocfs2 uses it.
> It is fragile though, as an LSM that allocates the name together
> with the value would turn it into a use-after-free, so the rework
> below copies the names as well.
> 
> Fix this by storing copies of all the security xattrs: allocate an
> array in ocfs2_initxattrs(), copy the names and values, account for
> all of them in the credit calculations, and write each one in
> ocfs2_init_security_set().
> 
> While at it, switch the allocations from GFP_KERNEL to GFP_NOFS.
> The callback runs in the inode creation path with the parent
> directory locked, and the security layer allocates the very same
> xattr array with GFP_NOFS, so direct reclaim must not be allowed to
> recurse into the filesystem from here.
> 
> Fixes: de3004c874e7 ("ocfs2: Switch to security_inode_init_security()")
> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>

The code looks good to me.
Reviewed-by: Heming Zhao <heming.zhao@suse.com>
> ---
>  fs/ocfs2/namei.c |   6 +--
>  fs/ocfs2/xattr.c | 122 ++++++++++++++++++++++++++++++++++++-----------
>  fs/ocfs2/xattr.h |  12 +++--
>  3 files changed, 105 insertions(+), 35 deletions(-)
> 
> diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c
> index 58c6061ed983..ef03f90e7265 100644
> --- a/fs/ocfs2/namei.c
> +++ b/fs/ocfs2/namei.c
> @@ -248,7 +248,6 @@ static int ocfs2_mknod(struct mnt_idmap *idmap,
>  	int want_meta = 0;
>  	int xattr_credits = 0;
>  	struct ocfs2_security_xattr_info si = {
> -		.name = NULL,
>  		.enable = 1,
>  	};
>  	int did_quota_inode = 0;
> @@ -475,7 +474,7 @@ static int ocfs2_mknod(struct mnt_idmap *idmap,
>  
>  	brelse(new_fe_bh);
>  	brelse(parent_fe_bh);
> -	kfree(si.value);
> +	ocfs2_free_security_xattrs(&si);
>  
>  	ocfs2_acl_init_release(&acl_state);
>  
> @@ -1831,7 +1830,6 @@ static int ocfs2_symlink(struct mnt_idmap *idmap,
>  	int want_clusters = 0;
>  	int xattr_credits = 0;
>  	struct ocfs2_security_xattr_info si = {
> -		.name = NULL,
>  		.enable = 1,
>  	};
>  	int did_quota = 0, did_quota_inode = 0;
> @@ -2063,7 +2061,7 @@ static int ocfs2_symlink(struct mnt_idmap *idmap,
>  
>  	brelse(new_fe_bh);
>  	brelse(parent_fe_bh);
> -	kfree(si.value);
> +	ocfs2_free_security_xattrs(&si);
>  	ocfs2_free_dir_lookup_result(&lookup);
>  	if (inode_ac)
>  		ocfs2_free_alloc_context(inode_ac);
> diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c
> index a2d6c3d0f2e8..f2b7a53ed969 100644
> --- a/fs/ocfs2/xattr.c
> +++ b/fs/ocfs2/xattr.c
> @@ -643,10 +643,15 @@ int ocfs2_calc_security_init(struct inode *dir,
>  			     int *xattr_credits,
>  			     struct ocfs2_alloc_context **xattr_ac)
>  {
> +	int i;
>  	int ret = 0;
>  	struct ocfs2_super *osb = OCFS2_SB(dir->i_sb);
> -	int s_size = ocfs2_xattr_entry_real_size(strlen(si->name),
> -						 si->value_len);
> +	int s_size = 0;
> +
> +	for (i = 0; i < si->count; i++)
> +		s_size += ocfs2_xattr_entry_real_size(
> +				strlen(si->xattrs[i].name),
> +				si->xattrs[i].value_len);
>  
>  	/*
>  	 * The max space of security xattr taken inline is
> @@ -664,13 +669,15 @@ int ocfs2_calc_security_init(struct inode *dir,
>  	}
>  
>  	/* reserve clusters for xattr value which will be set in B tree*/
> -	if (si->value_len > OCFS2_XATTR_INLINE_SIZE) {
> -		int new_clusters = ocfs2_clusters_for_bytes(dir->i_sb,
> -							    si->value_len);
> -
> -		*xattr_credits += ocfs2_clusters_to_blocks(dir->i_sb,
> -							   new_clusters);
> -		*want_clusters += new_clusters;
> +	for (i = 0; i < si->count; i++) {
> +		if (si->xattrs[i].value_len > OCFS2_XATTR_INLINE_SIZE) {
> +			int new_clusters = ocfs2_clusters_for_bytes(dir->i_sb,
> +						si->xattrs[i].value_len);
> +
> +			*xattr_credits += ocfs2_clusters_to_blocks(dir->i_sb,
> +								   new_clusters);
> +			*want_clusters += new_clusters;
> +		}
>  	}
>  	return ret;
>  }
> @@ -680,12 +687,16 @@ void ocfs2_calc_xattr_init(struct inode *dir, umode_t mode,
>  			   int *want_clusters, int *xattr_credits,
>  			   int *want_meta, struct ocfs2_acl_state *acl_state)
>  {
> +	int i;
>  	struct ocfs2_super *osb = OCFS2_SB(dir->i_sb);
>  	int s_size = 0, a_size = 0, acl_len = 0, new_clusters;
>  
> -	if (si->enable)
> -		s_size = ocfs2_xattr_entry_real_size(strlen(si->name),
> -						     si->value_len);
> +	if (si->enable) {
> +		for (i = 0; i < si->count; i++)
> +			s_size += ocfs2_xattr_entry_real_size(
> +					strlen(si->xattrs[i].name),
> +					si->xattrs[i].value_len);
> +	}
>  
>  	if (osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL) {
>  		if (acl_state->default_acl && S_ISDIR(mode)) {
> @@ -732,12 +743,14 @@ void ocfs2_calc_xattr_init(struct inode *dir, umode_t mode,
>  	 * reserve credits and clusters for xattrs which has large value
>  	 * and have to be set outside
>  	 */
> -	if (si->enable && si->value_len > OCFS2_XATTR_INLINE_SIZE) {
> -		new_clusters = ocfs2_clusters_for_bytes(dir->i_sb,
> -							si->value_len);
> -		*xattr_credits += ocfs2_clusters_to_blocks(dir->i_sb,
> -							   new_clusters);
> -		*want_clusters += new_clusters;
> +	for (i = 0; si->enable && i < si->count; i++) {
> +		if (si->xattrs[i].value_len > OCFS2_XATTR_INLINE_SIZE) {
> +			new_clusters = ocfs2_clusters_for_bytes(dir->i_sb,
> +						si->xattrs[i].value_len);
> +			*xattr_credits += ocfs2_clusters_to_blocks(dir->i_sb,
> +								   new_clusters);
> +			*want_clusters += new_clusters;
> +		}
>  	}
>  	if (osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL) {
>  		if (acl_state->default_acl && S_ISDIR(mode)) {
> @@ -7630,17 +7643,46 @@ static int ocfs2_initxattrs(struct inode *inode, const struct xattr *xattr_array
>  {
>  	struct ocfs2_security_xattr_info *si = fs_info;
>  	const struct xattr *xattr;
> +	struct ocfs2_security_xattr *xattrs;
> +	int count = 0, i;
>  	int err = 0;
>  
>  	if (si) {
> -		si->value = kmemdup(xattr_array->value, xattr_array->value_len,
> -				    GFP_KERNEL);
> -		if (!si->value)
> +		for (xattr = xattr_array; xattr->name != NULL; xattr++)
> +			count++;
> +
> +		xattrs = kcalloc(count, sizeof(*xattrs), GFP_NOFS);
> +		if (!xattrs)
>  			return -ENOMEM;
>  
> -		si->name = xattr_array->name;
> -		si->value_len = xattr_array->value_len;
> +		for (i = 0; i < count; i++) {
> +			xattrs[i].name = kstrdup(xattr_array[i].name, GFP_NOFS);
> +			if (!xattrs[i].name) {
> +				err = -ENOMEM;
> +				goto out_err;
> +			}
> +
> +			xattrs[i].value = kmemdup(xattr_array[i].value,
> +						  xattr_array[i].value_len,
> +						  GFP_NOFS);
> +			if (!xattrs[i].value) {
> +				err = -ENOMEM;
> +				goto out_err;
> +			}
> +			xattrs[i].value_len = xattr_array[i].value_len;
> +		}
> +
> +		si->xattrs = xattrs;
> +		si->count = count;
>  		return 0;
> +
> +out_err:
> +		for (; i >= 0; i--) {
> +			kfree(xattrs[i].name);
> +			kfree(xattrs[i].value);
> +		}
> +		kfree(xattrs);
> +		return err;
>  	}
>  
>  	for (xattr = xattr_array; xattr->name != NULL; xattr++) {
> @@ -7653,6 +7695,19 @@ static int ocfs2_initxattrs(struct inode *inode, const struct xattr *xattr_array
>  	return err;
>  }
>  
> +void ocfs2_free_security_xattrs(struct ocfs2_security_xattr_info *si)
> +{
> +	int i;
> +
> +	for (i = 0; i < si->count; i++) {
> +		kfree(si->xattrs[i].name);
> +		kfree(si->xattrs[i].value);
> +	}
> +	kfree(si->xattrs);
> +	si->xattrs = NULL;
> +	si->count = 0;
> +}
> +
>  int ocfs2_init_security_get(struct inode *inode,
>  			    struct inode *dir,
>  			    const struct qstr *qstr,
> @@ -7670,7 +7725,7 @@ int ocfs2_init_security_get(struct inode *inode,
>  		 * security_inode_init_security() does not return -EOPNOTSUPP,
>  		 * we have to check the xattr ourselves.
>  		 */
> -		if (!ret && !si->name)
> +		if (!ret && !si->count)
>  			si->enable = 0;
>  
>  		return ret;
> @@ -7687,10 +7742,21 @@ int ocfs2_init_security_set(handle_t *handle,
>  			    struct ocfs2_alloc_context *xattr_ac,
>  			    struct ocfs2_alloc_context *data_ac)
>  {
> -	return ocfs2_xattr_set_handle(handle, inode, di_bh,
> -				     OCFS2_XATTR_INDEX_SECURITY,
> -				     si->name, si->value, si->value_len, 0,
> -				     xattr_ac, data_ac);
> +	int i;
> +	int ret = 0;
> +
> +	for (i = 0; i < si->count; i++) {
> +		ret = ocfs2_xattr_set_handle(handle, inode, di_bh,
> +					     OCFS2_XATTR_INDEX_SECURITY,
> +					     si->xattrs[i].name,
> +					     si->xattrs[i].value,
> +					     si->xattrs[i].value_len, 0,
> +					     xattr_ac, data_ac);
> +		if (ret)
> +			break;
> +	}
> +
> +	return ret;
>  }
>  
>  const struct xattr_handler ocfs2_xattr_security_handler = {
> diff --git a/fs/ocfs2/xattr.h b/fs/ocfs2/xattr.h
> index 887cc1a18b1a..1dd08f979e3c 100644
> --- a/fs/ocfs2/xattr.h
> +++ b/fs/ocfs2/xattr.h
> @@ -20,13 +20,18 @@ enum ocfs2_xattr_type {
>  	OCFS2_XATTR_MAX
>  };
>  
> -struct ocfs2_security_xattr_info {
> -	int enable;
> -	const char *name;
> +struct ocfs2_security_xattr {
> +	char *name;
>  	void *value;
>  	size_t value_len;
>  };
>  
> +struct ocfs2_security_xattr_info {
> +	int enable;
> +	struct ocfs2_security_xattr *xattrs;
> +	int count;
> +};
> +
>  extern const struct xattr_handler ocfs2_xattr_user_handler;
>  extern const struct xattr_handler ocfs2_xattr_trusted_handler;
>  extern const struct xattr_handler ocfs2_xattr_security_handler;
> @@ -49,6 +54,7 @@ int ocfs2_xattr_remove(struct inode *, struct buffer_head *);
>  int ocfs2_init_security_get(struct inode *, struct inode *,
>  			    const struct qstr *,
>  			    struct ocfs2_security_xattr_info *);
> +void ocfs2_free_security_xattrs(struct ocfs2_security_xattr_info *);
>  int ocfs2_init_security_set(handle_t *, struct inode *,
>  			    struct buffer_head *,
>  			    struct ocfs2_security_xattr_info *,
> -- 
> 2.39.3
> 

Re: [PATCH] ocfs2: retain all security xattrs during inode creation

[Joseph Qi]

On 9/16/26 9:54 PM, Heming Zhao wrote:
> On Wed, Sep 16, 2026 at 08:02:58PM +0800, Joseph Qi wrote:
>> When creating a new inode, ocfs2_init_security_get() stores the
>> security xattr returned by the LSM framework in
>> ocfs2_security_xattr_info so that it can be written later within the
>> same transaction.  But ocfs2_initxattrs() only ever looks at the
>> first element of the xattr array, and it keeps the name as a bare
>> pointer into memory owned by the security layer instead of copying
>> it.  The value, by contrast, is already duplicated with kmemdup().
>>
>> With stacked LSMs, security_inode_init_security() provides one xattr
>> per LSM, so all labels but the first are silently dropped and never
>> written to disk, leaving newly created files without the security
>> xattrs the other LSMs rely on.
>>
>> The borrowed name is not an outright bug with the LSMs in tree
>> today: security_inode_init_security() frees only ->value once the
>> callback returns, and documents ->name as the attribute name
>> suffix, so the pointer stays valid for as long as ocfs2 uses it.
>> It is fragile though, as an LSM that allocates the name together
>> with the value would turn it into a use-after-free, so the rework
>> below copies the names as well.
>>
>> Fix this by storing copies of all the security xattrs: allocate an
>> array in ocfs2_initxattrs(), copy the names and values, account for
>> all of them in the credit calculations, and write each one in
>> ocfs2_init_security_set().
>>
>> While at it, switch the allocations from GFP_KERNEL to GFP_NOFS.
>> The callback runs in the inode creation path with the parent
>> directory locked, and the security layer allocates the very same
>> xattr array with GFP_NOFS, so direct reclaim must not be allowed to
>> recurse into the filesystem from here.
>>
>> Fixes: de3004c874e7 ("ocfs2: Switch to security_inode_init_security()")
>> Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
> 
> The code looks good to me.
> Reviewed-by: Heming Zhao <heming.zhao@suse.com>

Thanks, sashiko has review comments for the case of 512B block size:
https://sashiko.dev/#/patchset/20260916120258.3583355-1-joseph.qi@linux.alibaba.com?part=1

It looks simple and I'll address it in v2.

BTW, I've sent the update policy including cc ocfs2-devel, but It hasn't
been merged yet.

Thanks,
Joseph