Linus Torvalds <[email protected]> writes:
> On Wed, Aug 16, 2017 at 1:30 PM, Linus Torvalds
> <[email protected]> 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
