Messages in this thread Patch in this message |  | | From | (Eric W. Biederman) | Date | Wed, 16 Aug 2017 17:46:46 -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 1:30 PM, Linus Torvalds > <torvalds@linux-foundation.org> wrote: >> >> I suspect the easiest fix is to just add a "mnt" argument to >> devpts_acquire(), It shouldn't be too painful. Let me try. > > Ok, here's a *very* lightly tested patch. It might have new bugs, but > it makes your test program DTRT. > > Al, mind going over this and making sure I didn't miss anything? > > And Christian, if you can beat on this, that would be good.
Linus reading through this it looks like your error handling is wrong.
> diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c > index 284749fb0f6b..432f514e3f42 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,9 +850,14 @@ 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->dentry = dentry; > - path_get(pts_path); > + > + /* > + * The mnt already got a ref from devpts_acquire(), > + * so we only dget() on the dentry. > + */ > + pts_path->mnt = mnt; > + pts_path->dentry = dget(dentry); > + > tty->link->driver_data = pts_path; > > retval = ptm_driver->ops->open(tty, filp); ^^^^^^^
If this open fails the code jumps to err_put_path which falls through into out_put_fsi. But it also does path_put(pts_path). Which will result in a double mntput of mnt.
So I think err_path_put needs to be updated to just put the dentry, and let the later mntput put the mount.
> @@ -874,6 +880,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;
Eric
|  |