Re: [PATCH 2/5] smack: fix bug: SMACK64TRANSMUTE set on non-directory

[Roberto Sassu <roberto.sassu@huaweicloud.com>]

On Mon, 2025-06-16 at 04:07 +0300, Konstantin Andreev wrote:
> When a new file system object is created
> and the conditions for label transmutation are met,
> the SMACK64TRANSMUTE extended attribute is set
> on the object regardless of its type:
> file, pipe, socket, symlink, or directory.
> 
> However,
> SMACK64TRANSMUTE may only be set on directories.
> 
> This bug is a combined effect of the commits [1] and [2]
> which both transfer functionality
> from smack_d_instantiate() to smack_inode_init_security(),
> but only in part.
> 
> Commit [1] set blank  SMACK64TRANSMUTE on improper object types.

Hi Konstantin

I didn't see where, can you point where it is done?

> Commit [2] set "TRUE" SMACK64TRANSMUTE on improper object types.
> 
> [1] 2023-06-10,
> Fixes: baed456a6a2f ("smack: Set the SMACK64TRANSMUTE xattr in smack_inode_init_security()")
> Link: https://lore.kernel.org/linux-security-module/20230610075738.3273764-3-roberto.sassu@huaweicloud.com/
> 
> [2] 2023-11-16,
> Fixes: e63d86b8b764 ("smack: Initialize the in-memory inode in smack_inode_init_security()")
> Link: https://lore.kernel.org/linux-security-module/20231116090125.187209-5-roberto.sassu@huaweicloud.com/
> 
> Signed-off-by: Konstantin Andreev <andreev@swemel.ru>
> ---
>  security/smack/smack_lsm.c | 24 +++++++++++++-----------
>  1 file changed, 13 insertions(+), 11 deletions(-)
> 
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index a7292d286f7c..2d3186e50c62 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -1028,18 +1028,20 @@ static int smack_inode_init_security(struct inode *inode, struct inode *dir,
>  		if (!trans_cred)
>  			issp->smk_inode = dsp;
>  
> -		issp->smk_flags |= SMK_INODE_TRANSMUTE;
> -		xattr_transmute = lsm_get_xattr_slot(xattrs,
> -						     xattr_count);
> -		if (xattr_transmute) {
> -			xattr_transmute->value = kmemdup(TRANS_TRUE,
> -							 TRANS_TRUE_SIZE,
> -							 GFP_NOFS);
> -			if (!xattr_transmute->value)
> -				return -ENOMEM;

To avoid having another indentation, you could also set xattr_transmute
to NULL initially, and call lsm_get_xattr_slot() only for directories.
Setting SMK_INODE_TRANSMUTE in issp->smk_flags should be done of course
if xattr_transmute != NULL.

Either way is fine for me.

Roberto

> +		if (S_ISDIR(inode->i_mode)) {
> +			issp->smk_flags |= SMK_INODE_TRANSMUTE;
> +			xattr_transmute = lsm_get_xattr_slot(xattrs,
> +							     xattr_count);
> +			if (xattr_transmute) {
> +				xattr_transmute->value = kmemdup(TRANS_TRUE,
> +								 TRANS_TRUE_SIZE,
> +								 GFP_NOFS);
> +				if (!xattr_transmute->value)
> +					return -ENOMEM;
>  
> -			xattr_transmute->value_len = TRANS_TRUE_SIZE;
> -			xattr_transmute->name = XATTR_SMACK_TRANSMUTE;
> +				xattr_transmute->value_len = TRANS_TRUE_SIZE;
> +				xattr_transmute->name = XATTR_SMACK_TRANSMUTE;
> +			}
>  		}
>  	}
>