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

[Konstantin Meskhidze <konstantin.meskhidze@xxxxxxxxxx>]

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