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

[Casey Schaufler]

On 6/16/2025 10:53 AM, Konstantin Andreev wrote:
> 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;
> | }

Leave it as is. I haven't tried out the changes yet,
and will review more fully once I've verified they work
correctly.

>
> 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]
>