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

[Konstantin Andreev]

Roberto Sassu, 16 Jun 2025 10:42:16 +0200:
> 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?

Hi, Roberto.

I didnt investigate this thoroughly, but, I guess,
this proceeds like:

1) smack_inode_init_security() indeed sets "TRUE"
    for SMACK64TRANSMUTE in xattr for inode of any type.

2) smack_d_instantiate() then “confirms” "TRUE"
    only for directories.

At the moment of [1] both smack_inode_init_security()
and smack_d_instantiate() had the responsibility for
initialization of the inode Smack security.

So, user-visible SMACK64TRANSMUTE exists, but blank,
while on-disk xattr still "TRUE".

After [2] smack_d_instantiate() no longer participates
in the initialization of new filesystem objects,
so user-visible SMACK64TRANSMUTE became "TRUE".

You can check the user-visible effect
by building the kernel of version [1].

Konstantin.

>> 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 at 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;
>> +			}
>>   		}
>>   	}