lkml.org 
[lkml]   [2017]   [Mar]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [RFC] Add option to mount only a pids subset
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.


\
 
 \ /
  Last update: 2017-03-07 18:52    [W:0.542 / U:0.004 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site