Re: [PATCH v5 07/15] landlock: add support network rules

[Konstantin Meskhidze <konstantin.meskhidze@huawei.com>]

5/19/2022 5:42 PM, Mickaël Salaün пишет:
> 
> On 19/05/2022 11:27, Konstantin Meskhidze wrote:
>>
>>
>> 5/17/2022 11:27 AM, Mickaël Salaün пишет:
> 
> [...]
> 
> 
>>>>
>>>> @@ -275,21 +281,17 @@ static int get_path_from_fd(const s32 fd, 
>>>> struct path *const path)
>>>>       return err;
>>>>   }
>>>>
>>>> -static int add_rule_path_beneath(const int ruleset_fd, const void 
>>>> *const rule_attr)
>>>> +static int add_rule_path_beneath(struct landlock_ruleset *const 
>>>> ruleset,
>>>> +                 const void *const rule_attr)
>>>>   {
>>>>       struct landlock_path_beneath_attr path_beneath_attr;
>>>>       struct path path;
>>>> -    struct landlock_ruleset *ruleset;
>>>>       int res, err;
>>>> -
>>>> -    /* Gets and checks the ruleset. */
>>>> -    ruleset = get_ruleset_from_fd(ruleset_fd, FMODE_CAN_WRITE);
>>>> -    if (IS_ERR(ruleset))
>>>> -        return PTR_ERR(ruleset);
>>>> +    u32 mask;
>>>>
>>>>       /* Copies raw user space buffer, only one type for now. */
>>>>       res = copy_from_user(&path_beneath_attr, rule_attr,
>>>> -                sizeof(path_beneath_attr));
>>>> +            sizeof(path_beneath_attr));
>>>>       if (res)
>>>>           return -EFAULT;
>>>>
>>>> @@ -298,32 +300,26 @@ static int add_rule_path_beneath(const int 
>>>> ruleset_fd, const void *const rule_at
>>>>        * are ignored in path walks.
>>>>        */
>>>>       if (!path_beneath_attr.allowed_access) {
>>>> -        err = -ENOMSG;
>>>> -        goto out_put_ruleset;
>>>> +        return -ENOMSG;
>>>>       }
>>>>       /*
>>>>        * Checks that allowed_access matches the @ruleset constraints
>>>>        * (ruleset->access_masks[0] is automatically upgraded to 
>>>> 64-bits).
>>>>        */
>>>> -    if ((path_beneath_attr.allowed_access |
>>>> -        landlock_get_fs_access_mask(ruleset, 0)) !=
>>>> -                landlock_get_fs_access_mask(ruleset, 0)) {
>>>> -        err = -EINVAL;
>>>> -        goto out_put_ruleset;
>>>> -    }
>>>> +    mask = landlock_get_fs_access_mask(ruleset, 0);
>>>> +    if ((path_beneath_attr.allowed_access | mask) != mask)
>>>> +        return -EINVAL;
>>>>
>>>>       /* Gets and checks the new rule. */
>>>>       err = get_path_from_fd(path_beneath_attr.parent_fd, &path);
>>>>       if (err)
>>>> -        goto out_put_ruleset;
>>>> +        return err;
>>>>
>>>>       /* Imports the new rule. */
>>>>       err = landlock_append_fs_rule(ruleset, &path,
>>>>                         path_beneath_attr.allowed_access);
>>>>       path_put(&path);
>>>>
>>>> -out_put_ruleset:
>>>> -    landlock_put_ruleset(ruleset);
>>>>       return err;
>>>>   }
>>>>
>>>> @@ -360,6 +356,7 @@ SYSCALL_DEFINE4(landlock_add_rule,
>>>>           const int, ruleset_fd, const enum landlock_rule_type, 
>>>> rule_type,
>>>>           const void __user *const, rule_attr, const __u32, flags)
>>>>   {
>>>> +    struct landlock_ruleset *ruleset;
>>>>       int err;
>>>>
>>>>       if (!landlock_initialized)
>>>> @@ -369,14 +366,20 @@ SYSCALL_DEFINE4(landlock_add_rule,
>>>>       if (flags)
>>>>           return -EINVAL;
>>>>
>>>> +    /* Gets and checks the ruleset. */
>>>> +    ruleset = get_ruleset_from_fd(ruleset_fd, FMODE_CAN_WRITE);
>>>> +    if (IS_ERR(ruleset))
>>>> +        return PTR_ERR(ruleset);
>>>
>>> This shouldn't be part of this patch.
>>>
>>    I agree. I will move it into another patch.
> 
> To be clear, it is kind of a partial revert of patch 5/15.

   Yep. Thank you for noticing that.
> .