Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1594479

Re: [RFC] Add option to mount only a pids subset

From Oleg Nesterov <oleg@redhat.com>
Newsgroups linux.kernel
Subject Re: [RFC] Add option to mount only a pids subset
Date 2017-03-07 19:00 +0100
Message-ID <tiryV-2vy-15@gated-at.bofh.it> (permalink)
References <tdk53-3Wc-5@gated-at.bofh.it> <tiaop-73N-1@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


I can't really review this... but in any case I think you should split
this patch to separate the vfs and proc changes.

On 03/07, Alexey Gladkov wrote:
>
> @@ -962,6 +963,14 @@ vfs_kern_mount(struct file_system_type *type, int flags, const char *name, void
>  	mnt->mnt.mnt_sb = root->d_sb;
>  	mnt->mnt_mountpoint = mnt->mnt.mnt_root;
>  	mnt->mnt_parent = mnt;
> +
> +	err = do_mount_sb(&mnt->mnt, flags, data);
> +	if(err) {
> +		mnt_free_id(mnt);
> +		free_vfsmnt(mnt);
> +		return ERR_PTR(err);
> +	}

This duplicates the error handling, we do the same if mount_fs() fails.
Perhaps you should move these 2 lines into cleanup block and add goto's.

> +int proc_getattrfs(struct vfsmount *mnt, struct dentry *dentry, struct kstat *stat)
> +{
> +	struct inode *inode = d_inode(dentry);
> +	struct pid *pid = proc_pid(dentry->d_inode);
> +	struct proc_options *opts = mnt->fs_data;
> +
> +	if (opts && opts->pid_only && mnt->mnt_root != dentry && !pid)
> +		return -ENOENT;

Hmm. I don't quite understand why do we need this, and how this should work.

Yes, "/bin/ls /pidonly-proc/sys" or opendir(/pidonly-proc/sys) should fail,
but only because they both do stat() ?

Afaics you still can do open("/pidonly-proc/sys") + getdents() and this should
work ?

I still think proc_dir_operations.open() makes more sense. Yes, as you pointed
out we also need to update proc_sys_dir_file_operations too and may be something
else...

> +
> +	if (!inode->i_op->getattr) {
> +		generic_fillattr(inode, stat);
> +		return 0;
> +	}
> +
> +	return inode->i_op->getattr(mnt, dentry, stat);
> +}

Oh, it would be nice to not duplicate the code from the caller, imo.

Oleg.

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[RFC] Add option to mount only a pids subset Alexey Gladkov <gladkov.alexey@gmail.com> - 2017-03-07 00:40 +0100
  Re: [RFC] Add option to mount only a pids subset Andy Lutomirski <luto@amacapital.net> - 2017-03-07 17:30 +0100
    Re: [RFC] Add option to mount only a pids subset Djalal Harouni <tixxdz@gmail.com> - 2017-03-09 12:30 +0100
      Re: [RFC] Add option to mount only a pids subset ebiederm@xmission.com (Eric W. Biederman) - 2017-03-09 22:00 +0100
      Re: [RFC] Add option to mount only a pids subset Alexey Gladkov <gladkov.alexey@gmail.com> - 2017-03-11 22:50 +0100
    Re: [RFC] Add option to mount only a pids subset Alexey Gladkov <gladkov.alexey@gmail.com> - 2017-03-11 01:00 +0100
  Re: [RFC] Add option to mount only a pids subset Oleg Nesterov <oleg@redhat.com> - 2017-03-07 19:00 +0100
    Re: [RFC] Add option to mount only a pids subset Alexey Gladkov <gladkov.alexey@gmail.com> - 2017-03-11 00:40 +0100
  Re: [RFC] Add option to mount only a pids subset Al Viro <viro@ZenIV.linux.org.uk> - 2017-03-12 03:00 +0100
    Re: [RFC] Add option to mount only a pids subset Al Viro <viro@ZenIV.linux.org.uk> - 2017-03-12 03:20 +0100
      Re: [RFC] Add option to mount only a pids subset Andy Lutomirski <luto@amacapital.net> - 2017-03-13 04:30 +0100
        Re: [RFC] Add option to mount only a pids subset Al Viro <viro@ZenIV.linux.org.uk> - 2017-03-13 14:30 +0100
          Re: [RFC] Add option to mount only a pids subset Andy Lutomirski <luto@kernel.org> - 2017-03-13 16:30 +0100

csiph-web