lkml.org 
[lkml]   [2022]   [Aug]   [30]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    SubjectRe: [PATCH -next v2 2/6] landlock: abstract walk_to_visible_parent() helper
    From

    On 27/08/2022 13:12, Xiu Jianfeng wrote:
    > This helper will be used in the next commit which supports chmod and
    > chown access rights restriction.
    >
    > Signed-off-by: Xiu Jianfeng <xiujianfeng@huawei.com>
    > ---
    > security/landlock/fs.c | 67 ++++++++++++++++++++++++++++++------------
    > 1 file changed, 49 insertions(+), 18 deletions(-)
    >
    > diff --git a/security/landlock/fs.c b/security/landlock/fs.c
    > index c57f581a9cd5..4ef614a4ea22 100644
    > --- a/security/landlock/fs.c
    > +++ b/security/landlock/fs.c
    > @@ -38,6 +38,44 @@
    > #include "ruleset.h"
    > #include "setup.h"
    >
    > +enum walk_result {
    > + WALK_CONTINUE,
    > + WALK_TO_REAL_ROOT,
    > + WALK_TO_DISCONN_ROOT,

    Why did you created these results instead of the ones I proposed?


    > +};
    > +
    > +/*
    > + * walk to the visible parent, caller should call path_get()/path_put()
    > + * before/after this helpler.
    > + *
    > + * Returns:
    > + * - WALK_TO_REAL_ROOT if walk to the real root;
    > + * - WALK_TO_DISCONN_ROOT if walk to disconnected root;
    > + * - WALK_CONTINUE otherwise.
    > + */
    > +static enum walk_result walk_to_visible_parent(struct path *path)
    > +{
    > + struct dentry *parent_dentry;
    > +jump_up:
    > + if (path->dentry == path->mnt->mnt_root) {
    > + if (follow_up(path)) {
    > + /* Ignores hidden mount points. */
    > + goto jump_up;
    > + } else {
    > + /* Stop at the real root. */
    > + return WALK_TO_REAL_ROOT;
    > + }
    > + }
    > + /* Stops at disconnected root directories. */
    > + if (unlikely(IS_ROOT(path->dentry)))
    > + return WALK_TO_DISCONN_ROOT;
    > + parent_dentry = dget_parent(path->dentry);
    > + dput(path->dentry);
    > + path->dentry = parent_dentry;
    > +
    > + return WALK_CONTINUE;
    > +}
    > +
    > /* Underlying object management */
    >
    > static void release_inode(struct landlock_object *const object)
    > @@ -539,8 +577,8 @@ static int check_access_path_dual(
    > * restriction.
    > */
    > while (true) {
    > - struct dentry *parent_dentry;
    > const struct landlock_rule *rule;
    > + enum walk_result wr;

    Please make the names understandable. In this case this variable may not
    be needed anyway.


    >
    > /*
    > * If at least all accesses allowed on the destination are
    > @@ -588,20 +626,12 @@ static int check_access_path_dual(
    > if (allowed_parent1 && allowed_parent2)
    > break;
    >
    > -jump_up:
    > - if (walker_path.dentry == walker_path.mnt->mnt_root) {
    > - if (follow_up(&walker_path)) {
    > - /* Ignores hidden mount points. */
    > - goto jump_up;
    > - } else {
    > - /*
    > - * Stops at the real root. Denies access
    > - * because not all layers have granted access.
    > - */
    > - break;
    > - }
    > - }
    > - if (unlikely(IS_ROOT(walker_path.dentry))) {
    > + wr = walk_to_visible_parent(&walker_path);
    > + switch (wr) {
    > + case WALK_TO_REAL_ROOT:
    > + /* Stop at the real root. */
    > + goto out;
    > + case WALK_TO_DISCONN_ROOT:
    > /*
    > * Stops at disconnected root directories. Only allows
    > * access to internal filesystems (e.g. nsfs, which is
    > @@ -609,12 +639,13 @@ static int check_access_path_dual(
    > */
    > allowed_parent1 = allowed_parent2 =
    > !!(walker_path.mnt->mnt_flags & MNT_INTERNAL);

    Why not include this check in the helper? This is then not checked in
    patch 3 with current_check_access_path_context_only(), which is a bug.


    > + goto out;
    > + case WALK_CONTINUE:
    > + default:
    > break;
    > }
    > - parent_dentry = dget_parent(walker_path.dentry);
    > - dput(walker_path.dentry);
    > - walker_path.dentry = parent_dentry;
    > }
    > +out:
    > path_put(&walker_path);
    >
    > if (allowed_parent1 && allowed_parent2)

    \
     
     \ /
      Last update: 2022-08-30 13:23    [W:4.339 / U:0.012 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site