|
Message-ID: <20180427134936.GA31171@kroah.com> Date: Fri, 27 Apr 2018 15:49:37 +0200 From: Greg KH <gregkh@...uxfoundation.org> To: Kees Cook <keescook@...omium.org>, Thomas Richter <tmricht@...ux.ibm.com> Cc: kernel-hardening@...ts.openwall.com, brueckner@...ux.vnet.ibm.com, schwidefsky@...ibm.com, heiko.carstens@...ibm.com, linux-kernel@...r.kernel.org Subject: Re: [PATCH v2] inode: debugfs_create_dir uses mode permission from parent I'm going to add Kees and the kernel-hardning list here, as I'd like their opinions for the patch below. Kees, do you have any problems with this patch? I know you worked on making debugfs more "secure" from non-root users, this should still keep the intial mount permissions all fine, right? Anything I'm not considering here? thanks, greg k-h On Fri, Apr 27, 2018 at 02:35:47PM +0200, Thomas Richter wrote: > Currently function debugfs_create_dir() creates a new > directory in the debugfs (usually mounted /sys/kernel/debug) > with permission rwxr-xr-x. This is hard coded. > > Change this to use the parent directory permission. > > Output before the patch: > root@...60047 ~]# tree -dp -L 1 /sys/kernel/debug/ > /sys/kernel/debug/ > ├── [drwxr-xr-x] bdi > ├── [drwxr-xr-x] block > ├── [drwxr-xr-x] dasd > ├── [drwxr-xr-x] device_component > ├── [drwxr-xr-x] extfrag > ├── [drwxr-xr-x] hid > ├── [drwxr-xr-x] kprobes > ├── [drwxr-xr-x] kvm > ├── [drwxr-xr-x] memblock > ├── [drwxr-xr-x] pm_qos > ├── [drwxr-xr-x] qdio > ├── [drwxr-xr-x] s390 > ├── [drwxr-xr-x] s390dbf > └── [drwx------] tracing > > 14 directories > [root@...60047 linux]# > > Output after the patch: > [root@...60047 ~]# tree -dp -L 1 /sys/kernel/debug/ > sys/kernel/debug/ > ├── [drwx------] bdi > ├── [drwx------] block > ├── [drwx------] dasd > ├── [drwx------] device_component > ├── [drwx------] extfrag > ├── [drwx------] hid > ├── [drwx------] kprobes > ├── [drwx------] kvm > ├── [drwx------] memblock > ├── [drwx------] pm_qos > ├── [drwx------] qdio > ├── [drwx------] s390 > ├── [drwx------] s390dbf > └── [drwx------] tracing > > 14 directories > [root@...60047 linux]# > > Here is the full diff output done with: > [root@...60047 ~]# diff -u treefull.before treefull.after | > sed 's-^- # -' > treefull.diff > # --- treefull.before 2018-04-27 13:22:04.532824564 +0200 > # +++ treefull.after 2018-04-27 13:24:12.106182062 +0200 > # @@ -1,55 +1,55 @@ > # /sys/kernel/debug/ > # -├── [drwxr-xr-x] bdi > # -│ ├── [drwxr-xr-x] 1:0 > # -│ ├── [drwxr-xr-x] 1:1 > # -│ ├── [drwxr-xr-x] 1:10 > # -│ ├── [drwxr-xr-x] 1:11 > # -│ ├── [drwxr-xr-x] 1:12 > # -│ ├── [drwxr-xr-x] 1:13 > # -│ ├── [drwxr-xr-x] 1:14 > # -│ ├── [drwxr-xr-x] 1:15 > # -│ ├── [drwxr-xr-x] 1:2 > # -│ ├── [drwxr-xr-x] 1:3 > # -│ ├── [drwxr-xr-x] 1:4 > # -│ ├── [drwxr-xr-x] 1:5 > # -│ ├── [drwxr-xr-x] 1:6 > # -│ ├── [drwxr-xr-x] 1:7 > # -│ ├── [drwxr-xr-x] 1:8 > # -│ ├── [drwxr-xr-x] 1:9 > # -│ └── [drwxr-xr-x] 94:0 > # -├── [drwxr-xr-x] block > # -├── [drwxr-xr-x] dasd > # -│ ├── [drwxr-xr-x] 0.0.e18a > # -│ ├── [drwxr-xr-x] dasda > # -│ └── [drwxr-xr-x] global > # -├── [drwxr-xr-x] device_component > # -├── [drwxr-xr-x] extfrag > # -├── [drwxr-xr-x] hid > # -├── [drwxr-xr-x] kprobes > # -├── [drwxr-xr-x] kvm > # -├── [drwxr-xr-x] memblock > # -├── [drwxr-xr-x] pm_qos > # -├── [drwxr-xr-x] qdio > # -│ └── [drwxr-xr-x] 0.0.f5f2 > # -├── [drwxr-xr-x] s390 > # -│ └── [drwxr-xr-x] stsi > # -├── [drwxr-xr-x] s390dbf > # -│ ├── [drwxr-xr-x] 0.0.e18a > # -│ ├── [drwxr-xr-x] cio_crw > # -│ ├── [drwxr-xr-x] cio_msg > # -│ ├── [drwxr-xr-x] cio_trace > # -│ ├── [drwxr-xr-x] dasd > # -│ ├── [drwxr-xr-x] kvm-trace > # -│ ├── [drwxr-xr-x] lgr > # -│ ├── [drwxr-xr-x] qdio_0.0.f5f2 > # -│ ├── [drwxr-xr-x] qdio_error > # -│ ├── [drwxr-xr-x] qdio_setup > # -│ ├── [drwxr-xr-x] qeth_card_0.0.f5f0 > # -│ ├── [drwxr-xr-x] qeth_control > # -│ ├── [drwxr-xr-x] qeth_msg > # -│ ├── [drwxr-xr-x] qeth_setup > # -│ ├── [drwxr-xr-x] vmcp > # -│ └── [drwxr-xr-x] vmur > # +├── [drwx------] bdi > # +│ ├── [drwx------] 1:0 > # +│ ├── [drwx------] 1:1 > # +│ ├── [drwx------] 1:10 > # +│ ├── [drwx------] 1:11 > # +│ ├── [drwx------] 1:12 > # +│ ├── [drwx------] 1:13 > # +│ ├── [drwx------] 1:14 > # +│ ├── [drwx------] 1:15 > # +│ ├── [drwx------] 1:2 > # +│ ├── [drwx------] 1:3 > # +│ ├── [drwx------] 1:4 > # +│ ├── [drwx------] 1:5 > # +│ ├── [drwx------] 1:6 > # +│ ├── [drwx------] 1:7 > # +│ ├── [drwx------] 1:8 > # +│ ├── [drwx------] 1:9 > # +│ └── [drwx------] 94:0 > # +├── [drwx------] block > # +├── [drwx------] dasd > # +│ ├── [drwx------] 0.0.e18a > # +│ ├── [drwx------] dasda > # +│ └── [drwx------] global > # +├── [drwx------] device_component > # +├── [drwx------] extfrag > # +├── [drwx------] hid > # +├── [drwx------] kprobes > # +├── [drwx------] kvm > # +├── [drwx------] memblock > # +├── [drwx------] pm_qos > # +├── [drwx------] qdio > # +│ └── [drwx------] 0.0.f5f2 > # +├── [drwx------] s390 > # +│ └── [drwx------] stsi > # +├── [drwx------] s390dbf > # +│ ├── [drwx------] 0.0.e18a > # +│ ├── [drwx------] cio_crw > # +│ ├── [drwx------] cio_msg > # +│ ├── [drwx------] cio_trace > # +│ ├── [drwx------] dasd > # +│ ├── [drwx------] kvm-trace > # +│ ├── [drwx------] lgr > # +│ ├── [drwx------] qdio_0.0.f5f2 > # +│ ├── [drwx------] qdio_error > # +│ ├── [drwx------] qdio_setup > # +│ ├── [drwx------] qeth_card_0.0.f5f0 > # +│ ├── [drwx------] qeth_control > # +│ ├── [drwx------] qeth_msg > # +│ ├── [drwx------] qeth_setup > # +│ ├── [drwx------] vmcp > # +│ └── [drwx------] vmur > # └── [drwx------] tracing > # ├── [drwxr-xr-x] events > # │ ├── [drwxr-xr-x] alarmtimer > > Fixes: edac65eaf8d5c ("debugfs: take mode-dependent parts of debugfs_get_inode() into callers") > Signed-off-by: Thomas Richter <tmricht@...ux.ibm.com> > Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org> > --- > fs/debugfs/inode.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c > index 13b0135..a913b12 100644 > --- a/fs/debugfs/inode.c > +++ b/fs/debugfs/inode.c > @@ -512,7 +512,9 @@ struct dentry *debugfs_create_dir(const char *name, struct dentry *parent) > if (unlikely(!inode)) > return failed_creating(dentry); > > - inode->i_mode = S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO; > + if (!parent) > + parent = debugfs_mount->mnt_root; > + inode->i_mode = S_IFDIR | ((d_inode(parent)->i_mode & 0770)); > inode->i_op = &simple_dir_inode_operations; > inode->i_fop = &simple_dir_operations; > > -- > 2.9.3
Powered by blists - more mailing lists
Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.