[PATCH 5/5] smack: fix bug: invalid label of unix socket file

[Konstantin Andreev]

Casey Schaufler, 16 Jun 2025 10:11:55 -0700:
> On 6/16/2025 4:46 AM, Konstantin Andreev wrote:
>> Roberto Sassu, 16 Jun 2025 11:05:11 +0200:
>>> On Mon, 2025-06-16 at 04:07 +0300, Konstantin Andreev wrote:
>>>>
>>>> [irrelevant portion of the message deleted]
>>>>
>>>> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
>>>> index fb23254c8a54..1b41ae053966 100644
>>>> --- a/security/smack/smack_lsm.c
>>>> +++ b/security/smack/smack_lsm.c
>>>> @@ -1021,6 +1021,16 @@ static int smack_inode_init_security(struct
>>>> inode *inode, struct inode *dir,
>>>>        bool trans_cred;
>>>>        bool trans_rule;
>>>>    +    /*
>>>> +     * UNIX domain sockets use lower level socket data. Let
>>>> +     * UDS inode have fixed * label to keep smack_inode_permission() calm
>>>> +     * when called from unix_find_bsd()
>>>> +     */
>>>> +    if (S_ISSOCK(inode->i_mode)) {
>>>> +        /* forced label, no need to save to xattrs */
>>>> +        issp->smk_inode = &smack_known_star;
>>>> +        goto instant_inode;
>>>
>>> Maybe you could also set SMK_INODE_INSTANT here and just return.
>>
>> Certainly.
>>
>> But I personally avoid duplication even of small cleanups
>> and avoid multiple returns at the price of few gotos.
> 
> I generally prefer
> 
> 	if (xyzzy)
> 		return -ESOMETHING;
> 
> to
> 
> 	if (xyzzy)
> 		goto err_out;
> 	...
> err_out:
> 	return -ESOMETHING;
> 
> I grew up in the era of "gotos considered harmful". When
> err_out does more than just return a goto is fine, but a goto
> that has nothing but a return is unnecessary and adds code that
> needn't be there.

Got it. There is one line more than just return here:

| instant_inode:
|	issp->smk_flags |= (SMK_INODE_INSTANT | transflag);
|	return rc;
| }

What would you prefer in this case, leave it at that or convert to

| if (S_ISSOCK(inode->i_mode)) {
|    /* forced label, no need to save to xattrs */
|    issp->smk_inode = &smack_known_star;
|    // goto instant_inode; // <<<< to be removed
|    issp->smk_flags |= SMK_INODE_INSTANT;
|    return 0;
| }


[the rest of the message deleted]