Messages in this thread |  | | From | (Eric W. Biederman) | Date | Wed, 16 Aug 2017 20:24:23 -0500 | Subject | Re: [PATCH 0/1] devpts: use dynamic_dname() to generate proc name |
| |
Linus Torvalds <torvalds@linux-foundation.org> writes:
> On Wed, Aug 16, 2017 at 4:51 PM, Eric W. Biederman > <ebiederm@xmission.com> wrote: >> >> *Blink* You are right I missed that. >> >> In which case I am concerned about failures that make it to err_release. >> Unless I am missing something (again) failures that jump to err_release >> won't call mntput and will result in a mnt leak. > > Yes, I think you're right. > > Maybe this attached patch is better anyway. It's smaller, because it > keeps more closely to the old code, and just adds a mntput() in all > the exit cases, and depends on the "path_get()" to have incremented > the mnt refcount one extra time. > > Can you find something in this one?
My eyeballs don't see any problems with the patch below.
Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
Eric
> ENTIRELY UNTESTED! > > Linus > > drivers/tty/pty.c | 7 +++++-- > fs/devpts/inode.c | 4 +++- > include/linux/devpts_fs.h | 2 +- > 3 files changed, 9 insertions(+), 4 deletions(-) > > diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c > index 284749fb0f6b..1fc80ea87c13 100644 > --- a/drivers/tty/pty.c > +++ b/drivers/tty/pty.c > @@ -793,6 +793,7 @@ static int ptmx_open(struct inode *inode, struct file *filp) > struct tty_struct *tty; > struct path *pts_path; > struct dentry *dentry; > + struct vfsmount *mnt; > int retval; > int index; > > @@ -805,7 +806,7 @@ static int ptmx_open(struct inode *inode, struct file *filp) > if (retval) > return retval; > > - fsi = devpts_acquire(filp); > + fsi = devpts_acquire(filp, &mnt); > if (IS_ERR(fsi)) { > retval = PTR_ERR(fsi); > goto out_free_file; > @@ -849,7 +850,7 @@ static int ptmx_open(struct inode *inode, struct file *filp) > pts_path = kmalloc(sizeof(struct path), GFP_KERNEL); > if (!pts_path) > goto err_release; > - pts_path->mnt = filp->f_path.mnt; > + pts_path->mnt = mnt; > pts_path->dentry = dentry; > path_get(pts_path); > tty->link->driver_data = pts_path; > @@ -866,6 +867,7 @@ static int ptmx_open(struct inode *inode, struct file *filp) > path_put(pts_path); > kfree(pts_path); > err_release: > + mntput(mnt); > tty_unlock(tty); > // This will also put-ref the fsi > tty_release(inode, filp); > @@ -874,6 +876,7 @@ static int ptmx_open(struct inode *inode, struct file *filp) > devpts_kill_index(fsi, index); > out_put_fsi: > devpts_release(fsi); > + mntput(mnt); > out_free_file: > tty_free_file(filp); > return retval; > diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c > index 108df2e3602c..44dfbca9306f 100644 > --- a/fs/devpts/inode.c > +++ b/fs/devpts/inode.c > @@ -133,7 +133,7 @@ static inline struct pts_fs_info *DEVPTS_SB(struct super_block *sb) > return sb->s_fs_info; > } > > -struct pts_fs_info *devpts_acquire(struct file *filp) > +struct pts_fs_info *devpts_acquire(struct file *filp, struct vfsmount **ptsmnt) > { > struct pts_fs_info *result; > struct path path; > @@ -142,6 +142,7 @@ struct pts_fs_info *devpts_acquire(struct file *filp) > > path = filp->f_path; > path_get(&path); > + *ptsmnt = NULL; > > /* Has the devpts filesystem already been found? */ > sb = path.mnt->mnt_sb; > @@ -165,6 +166,7 @@ struct pts_fs_info *devpts_acquire(struct file *filp) > * pty code needs to hold extra references in case of last /dev/tty close > */ > atomic_inc(&sb->s_active); > + *ptsmnt = mntget(path.mnt); > result = DEVPTS_SB(sb); > > out: > diff --git a/include/linux/devpts_fs.h b/include/linux/devpts_fs.h > index 277ab9af9ac2..7883e901f65c 100644 > --- a/include/linux/devpts_fs.h > +++ b/include/linux/devpts_fs.h > @@ -19,7 +19,7 @@ > > struct pts_fs_info; > > -struct pts_fs_info *devpts_acquire(struct file *); > +struct pts_fs_info *devpts_acquire(struct file *, struct vfsmount **ptsmnt); > void devpts_release(struct pts_fs_info *); > > int devpts_new_index(struct pts_fs_info *);
|  |