linux-kernel - Re: [PATCH -next 0/2] lsm: Change inode

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c6e84076-9134-8c27-75bb-9191da6c23c3@huawei.com>
Date:   Wed, 10 May 2023 08:58:17 +0800
From:   xiujianfeng <xiujianfeng@...wei.com>
To:     <gregkh@...uxfoundation.org>, <rafael@...nel.org>,
        <viro@...iv.linux.org.uk>, <brauner@...nel.org>,
        <dhowells@...hat.com>, <code@...icks.com>,
        <hirofumi@...l.parknet.co.jp>, <linkinjeon@...nel.org>,
        <sfrench@...ba.org>, <senozhatsky@...omium.org>, <tom@...pey.com>,
        <chuck.lever@...cle.com>, <jlayton@...nel.org>,
        <miklos@...redi.hu>, <paul@...l-moore.com>, <jmorris@...ei.org>,
        <serge@...lyn.com>, <stephen.smalley.work@...il.com>,
        <eparis@...isplace.org>, <casey@...aufler-ca.com>,
        <dchinner@...hat.com>, <john.johansen@...onical.com>,
        <mcgrof@...nel.org>, <mortonm@...omium.org>, <fred@...udflare.com>,
        <mic@...ikod.net>, <mpe@...erman.id.au>, <nathanl@...ux.ibm.com>,
        <gnoack3000@...il.com>, <roberto.sassu@...wei.com>
CC:     <linux-kernel@...r.kernel.org>, <linux-fsdevel@...r.kernel.org>,
        <linux-cachefs@...hat.com>, <ecryptfs@...r.kernel.org>,
        <linux-cifs@...r.kernel.org>, <linux-nfs@...r.kernel.org>,
        <linux-unionfs@...r.kernel.org>,
        <linux-security-module@...r.kernel.org>, <selinux@...r.kernel.org>,
        <wangweiyang2@...wei.com>
Subject: Re: [PATCH -next 0/2] lsm: Change inode_setattr() to take struct

sorry, I forgot to add the link to preview discussion:

https://lore.kernel.org/all/20220827111215.131442-1-xiujianfeng@huawei.com/

On 2023/5/5 16:11, Xiu Jianfeng wrote:
> Hi,
> 
> I am working on adding xattr/attr support for landlock [1], so we can
> control fs accesses such as chmod, chown, uptimes, setxattr, etc.. inside
> landlock sandbox. the LSM hooks as following are invoved:
> 1.inode_setattr
> 2.inode_setxattr
> 3.inode_removexattr
> 4.inode_set_acl
> 5.inode_remove_acl
> which are controlled by LANDLOCK_ACCESS_FS_WRITE_METADATA.
> 
> and
> 1.inode_getattr
> 2.inode_get_acl
> 3.inode_getxattr
> 4.inode_listxattr
> which are controlled by LANDLOCK_ACCESS_FS_READ_METADATA
> 
> Some of these hooks only take struct dentry as a argument, However, for
> path-based LSMs such Landlock, Apparmor and Tomoyo, struct path instead
> of struct dentry required to make sense of attr/xattr accesses. So we
> need to refactor these hooks to take a struct path argument.
> 
> This patchset only refators inode_setattr hook as part of whole work.
> 
> Also, I have a problem about file_dentry() in __file_remove_privs() of the
> first patch, before changes in commit c1892c37769cf ("vfs: fix deadlock in
> file_remove_privs() on overlayfs"), it gets dentry and inode as belows:
> 
> struct dentry *dentry = file->f_path.dentry;
> struct inode *inode = d_inode(dentry);
> 
> That would be clear to change it to pass &file->f_path to
> __remove_privs()->notify_change()->inode_setattr().
> After that commit, it has been changed to:
> 
> struct dentry *dentry = file_dentry(file);
> struct inode *inode = file_inode(file);
> 
> If I understand correctly, the dentry from file_dentry() maybe the upper
> or the lower, it can be different from file->f_path.dentry. It can't just
> go back to use &file->f_path otherwise the bug will come back for
> overlayfs. So for such scenario, how to get a path from file if the file
> maybe or not from overlayfs, and which kind of overlayfs path is ok for
> Landlock?
> 
> Xiu Jianfeng (2):
>   fs: Change notify_change() to take struct path argument
>   lsm: Change inode_setattr hook to take struct path argument
> 
>  drivers/base/devtmpfs.c       |  5 +++--
>  fs/attr.c                     |  7 ++++---
>  fs/cachefiles/interface.c     |  4 ++--
>  fs/coredump.c                 |  2 +-
>  fs/ecryptfs/inode.c           | 18 +++++++++---------
>  fs/fat/file.c                 |  2 +-
>  fs/inode.c                    |  8 +++++---
>  fs/ksmbd/smb2pdu.c            |  6 +++---
>  fs/ksmbd/smbacl.c             |  2 +-
>  fs/namei.c                    |  2 +-
>  fs/nfsd/vfs.c                 | 12 ++++++++----
>  fs/open.c                     | 19 ++++++++++---------
>  fs/overlayfs/overlayfs.h      |  4 +++-
>  fs/utimes.c                   |  2 +-
>  include/linux/fs.h            |  4 ++--
>  include/linux/lsm_hook_defs.h |  2 +-
>  include/linux/security.h      |  4 ++--
>  security/security.c           | 10 +++++-----
>  security/selinux/hooks.c      |  3 ++-
>  security/smack/smack_lsm.c    |  5 +++--
>  20 files changed, 67 insertions(+), 54 deletions(-)
>