pty: fix use after free issues at pty_unix98_shutdown

pty: fix use after free issues at pty_unix98_shutdown

[Herton R. Krzesinski]

Hi,

Following this are fixes for two issues related to last close of /dev/tty at
pty_unix98_shutdown. This is a followup to thread/previous discussion with
subject "pty: fix use after free/oops at pty_unix98_shutdown".

The first problem can be reproduced with test case below:

$ cat test.sh
#!/bin/sh

while true; do
        find /sys
        ./dopty
        echo 2 > /proc/sys/vm/drop_caches
        ps aux
        sleep 40
done

$ cat dopty.c
#define _XOPEN_SOURCE
#include <unistd.h>
#include <stdlib.h>
#include <fcntl.h>
#include <sys/stat.h>
#include <sys/ioctl.h>
#include <termios.h>
#include <errno.h>

int main(int argc, char **argv)
{
        pid_t pid;
        int ptm_fd, pty_fd, tty_fd, tout;
        char *pty_name;
        int c = 0;

        while (c < 100) {
                c++;
                pid = fork();
                if (pid != 0)
                        continue;
                daemon(1, 0);
                ptm_fd = posix_openpt(O_RDWR);
                if (ptm_fd < 0)
                        return -1;
                grantpt(ptm_fd);
                unlockpt(ptm_fd);
                pty_name = ptsname(ptm_fd);
                pty_fd = open(pty_name, O_RDWR);
                tty_fd = open("/dev/tty", O_RDWR);
                pid = fork();
                if (pid == 0)  {
                        ioctl(tty_fd, TIOCNOTTY, NULL);
                        setsid();
                        close(pty_fd);
                        close(ptm_fd);
                        sleep(60);
                        return 0;
                }
                sleep(10);
                return 0;
        }
        sleep(30);
        return 0;
}

Running test.sh above, at some point you can get this crash:

[ 1741.842926] test.sh (8786): drop_caches: 2
[ 1812.417114] test.sh (8786): drop_caches: 2
[ 1842.405991] BUG: unable to handle kernel NULL pointer dereference at 0000000000000060
[ 1842.406055] IP: [<ffffffff8129f36d>] devpts_kill_index+0x1d/0x80
[ 1842.406110] PGD 0 
[ 1842.406123] Oops: 0000 [#1] SMP 
[ 1842.406148] Modules linked in: 8021q mrp garp stp llc nf_conntrack_ipv4 nf_defrag_ipv4 ip6t_REJECT nf_reject_ipv6 nf_conntrack_ipv6 nf_defrag_ipv6 xt_state nf_conntrack ip6t
able_filter ip6_tables binfmt_misc ppdev parport_pc parport floppy joydev tpm_tis tpm virtio_balloon serio_raw virtio_console virtio_net iosf_mbi crct10dif_pclmul crc32_pclmul 
pcspkr qxl ttm drm_kms_helper drm snd_hda_codec_generic snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep snd_seq snd_seq_device snd_pcm snd_timer snd soundcore i2c_piix4 virt
io_blk crc32c_intel virtio_pci virtio_ring virtio pata_acpi ata_generic [last unloaded: speedstep_lib]
[ 1842.406383] CPU: 2 PID: 9204 Comm: dopty Not tainted 4.4.0-rc5 #1
[ 1842.406404] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.8.1-20150318_183358- 04/01/2014
[ 1842.406427] task: ffff880039da3a00 ti: ffff880037a7c000 task.ti: ffff880037a7c000
[ 1842.406445] RIP: 0010:[<ffffffff8129f36d>]  [<ffffffff8129f36d>] devpts_kill_index+0x1d/0x80
[ 1842.406473] RSP: 0018:ffff880037a7fc98  EFLAGS: 00010282
[ 1842.406487] RAX: 0000000000000000 RBX: ffff880039e75c00 RCX: 00000001810000fd
[ 1842.406504] RDX: 00000000ffffffff RSI: 0000000000000004 RDI: ffff88003d0cc6a8
[ 1842.406521] RBP: ffff880037a7fca8 R08: ffffea0000ebcc50 R09: 0000000000000001
[ 1842.406539] R10: ffffea0000ebcc58 R11: 0000000000000000 R12: 0000000000000004
[ 1842.406556] R13: 0000000000000000 R14: 0000000000000000 R15: 0000000000000000
[ 1842.406573] FS:  0000000000000000(0000) GS:ffff88003fd00000(0000) knlGS:0000000000000000
[ 1842.406611] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1842.406634] CR2: 0000000000000060 CR3: 0000000001c0a000 CR4: 00000000001406e0
[ 1842.406665] Stack:
[ 1842.406677]  ffff880039e75c00 0000000000000000 ffff880037a7fcb8 ffffffff814802f8
[ 1842.406715]  ffff880037a7fce8 ffffffff814749fe ffff880000000000 ffffffff8177ce56
[ 1842.406785]  0000000000000000 ffff880039e75c00 ffff880037a7fd88 ffffffff81475ada
[ 1842.406821] Call Trace:
[ 1842.406821]  [<ffffffff814802f8>] pty_unix98_shutdown+0x18/0x20
[ 1842.406821]  [<ffffffff814749fe>] release_tty+0x3e/0xe0
[ 1842.406821]  [<ffffffff8177ce56>] ? mutex_lock+0x16/0x40
[ 1842.406821]  [<ffffffff81475ada>] tty_release+0x44a/0x580
[ 1842.406821]  [<ffffffff8121e3e5>] __fput+0xb5/0x200
[ 1842.406821]  [<ffffffff8121e5de>] ____fput+0xe/0x10
[ 1842.406821]  [<ffffffff810b2eb8>] task_work_run+0x68/0xa0
[ 1842.406821]  [<ffffffff8109a3e0>] do_exit+0x320/0x670
[ 1842.406821]  [<ffffffff810673d4>] ? __do_page_fault+0x1a4/0x450
[ 1842.406821]  [<ffffffff811361d0>] ? __audit_syscall_entry+0xb0/0x110
[ 1842.406821]  [<ffffffff81003376>] ? do_audit_syscall_entry+0x66/0x70
[ 1842.406821]  [<ffffffff8109a781>] do_group_exit+0x51/0xc0
[ 1842.406821]  [<ffffffff8109a807>] SyS_exit_group+0x17/0x20
[ 1842.406821]  [<ffffffff8177f2ee>] entry_SYSCALL_64_fastpath+0x12/0x71
[ 1842.406821] Code: 48 c7 c3 f4 ff ff ff eb 84 e8 10 7b df ff 55 48 89 e5 48 83 ec 10 48 89 1c 24 4c 89 64 24 08 66 66 66 66 90 48 8b 47 28 41 89 f4 <48> 81 78 60 d1 1c 00 00 74 12 48 8b 15 4a bd d6 00 31 c0 48 85 
[ 1842.406821] RIP  [<ffffffff8129f36d>] devpts_kill_index+0x1d/0x80
[ 1842.406821]  RSP <ffff880037a7fc98>
[ 1842.406821] CR2: 0000000000000060

The second problem is related to the fact you can umount any devpts instance
before the final /dev/tty close (where /dev/tty pointed to some previously
opened pty pair). Take for example test case below:

#define _XOPEN_SOURCE
#include <fcntl.h>
#include <stdlib.h>
#include <sys/ioctl.h>
#include <sys/stat.h>
#include <sys/types.h>
#include <unistd.h>

int main(int argc, char **argv)
{
        pid_t pid;
        int ptm_fd, pty_fd, tty_fd;

        system("mkdir -p /mnt/newpts");
        system("mount -t devpts -o newinstance none /mnt/newpts");
        pid = fork();
        if (pid != 0)
                exit(0);
        daemon(1, 0);
        ptm_fd = open("/mnt/newpts/ptmx", O_RDWR);
        unlockpt(ptm_fd);
        pty_fd = open("/mnt/newpts/0", O_RDWR);
        tty_fd = open("/dev/tty", O_RDWR);
        pid = fork();
        if (pid == 0) {
                ioctl(tty_fd, TIOCNOTTY, NULL);
                setsid();
                sleep(20);
                close(pty_fd);
                close(ptm_fd);
                system("umount /mnt/newpts");
                sleep(10);
                exit(0);
        }
        sleep(10);
        return 0;
}

The idea here is to umount a pts mount while still we have /dev/tty pointing to
a pty opened...

And of course it doesn't go well with the late devpts_kill_index:

[ 1326.233991] ------------[ cut here ]------------
[ 1326.234014] WARNING: CPU: 1 PID: 2668 at lib/idr.c:1051 ida_remove+0x9b/0x130()
[ 1326.234015] ida_remove called for id=0 which is not allocated.
[ 1326.234016] Modules linked in: 8021q mrp garp stp llc nf_conntrack_ipv4 nf_defrag_ipv4 ip6t_REJECT nf_reject_ipv6 nf_conntrack_ipv6 nf_defrag_ipv6 xt_state nf_conntrack ip6table_filter ip6_tables binfmt_misc ppdev joydev floppy parport_pc parport serio_raw tpm_tis tpm virtio_balloon virtio_console virtio_net iosf_mbi crct10dif_pclmul crc32_pclmul pcspkr snd_hda_codec_generic i2c_piix4 snd_hda_intel snd_hda_codec snd_hda_core snd_hwdep snd_seq snd_seq_device snd_pcm snd_timer snd soundcore qxl ttm drm_kms_helper drm virtio_blk crc32c_intel virtio_pci virtio_ring virtio pata_acpi ata_generic [last unloaded: speedstep_lib]
[ 1326.234061] CPU: 1 PID: 2668 Comm: newpty Not tainted 4.4.0-rc7 #1
[ 1326.234062] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.8.1-20150318_183358- 04/01/2014
[ 1326.234065]  000000000000041b ffff8800375ffbc8 ffffffff8139aed4 0000000000000009
[ 1326.234068]  ffff8800375ffc18 000000000000041b ffff8800375ffc18 ffff8800375ffc08
[ 1326.234069]  ffffffff81096f15 ffff8800375ffbf8 ffff8801399c47e0 0000000000000000
[ 1326.234071] Call Trace:
[ 1326.234083]  [<ffffffff8139aed4>] dump_stack+0x48/0x64
[ 1326.234092]  [<ffffffff81096f15>] warn_slowpath_common+0x95/0xe0
[ 1326.234094]  [<ffffffff81097016>] warn_slowpath_fmt+0x46/0x50
[ 1326.234104]  [<ffffffff811fc292>] ? kfree+0x112/0x150
[ 1326.234105]  [<ffffffff8139c75b>] ida_remove+0x9b/0x130
[ 1326.234111]  [<ffffffff8129f3c7>] devpts_kill_index+0x57/0x80
[ 1326.234120]  [<ffffffff81480338>] pty_unix98_shutdown+0x18/0x20
[ 1326.234124]  [<ffffffff81474a2e>] release_tty+0x3e/0xe0
[ 1326.234129]  [<ffffffff8177d476>] ? mutex_lock+0x16/0x40
[ 1326.234131]  [<ffffffff81475b0a>] tty_release+0x44a/0x580
[ 1326.234135]  [<ffffffff8121e3d5>] __fput+0xb5/0x200
[ 1326.234137]  [<ffffffff8121e5ce>] ____fput+0xe/0x10
[ 1326.234143]  [<ffffffff810b2eb8>] task_work_run+0x68/0xa0
[ 1326.234145]  [<ffffffff8109a3e0>] do_exit+0x320/0x670
[ 1326.234147]  [<ffffffff810673d4>] ? __do_page_fault+0x1a4/0x450
[ 1326.234155]  [<ffffffff811361d0>] ? __audit_syscall_entry+0xb0/0x110
[ 1326.234161]  [<ffffffff81003376>] ? do_audit_syscall_entry+0x66/0x70
[ 1326.234164]  [<ffffffff8109a781>] do_group_exit+0x51/0xc0
[ 1326.234166]  [<ffffffff8109a807>] SyS_exit_group+0x17/0x20
[ 1326.234169]  [<ffffffff8177f92e>] entry_SYSCALL_64_fastpath+0x12/0x71
[ 1326.234171] ---[ end trace 23cebcbb1a28e0e8 ]---

In this second case, devpts_kill_index ends up using a super_block which is
is already gone/destroyed.

Regards,
Herton.

[PATCH 1/2 v2] pty: fix possible use after free of tty->driver_data

[Herton R. Krzesinski]

This change fixes a bug for a corner case where we have the the last
release from a pty master/slave coming from a previously opened /dev/tty
file. When this happens, the tty->driver_data can be stale, due to all
ptmx or pts/N files having already been closed before (and thus the inode
related to these files, which tty->driver_data points to, being already
freed/destroyed).

The fix here is to keep a reference on the opened master ptmx inode.
We maintain the inode referenced until the final pty_unix98_shutdown,
and only pass this inode to devpts_kill_index.

Signed-off-by: Herton R. Krzesinski <herton@redhat.com>
Cc: <stable@vger.kernel.org> # 2.6.29+
---
 drivers/tty/pty.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

v2: fixed patch changelog

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index a45660f..96016e5 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -681,7 +681,14 @@ static void pty_unix98_remove(struct tty_driver *driver, struct tty_struct *tty)
 /* this is called once with whichever end is closed last */
 static void pty_unix98_shutdown(struct tty_struct *tty)
 {
-	devpts_kill_index(tty->driver_data, tty->index);
+	struct inode *ptmx_inode;
+
+	if (tty->driver->subtype == PTY_TYPE_MASTER)
+		ptmx_inode = tty->driver_data;
+	else
+		ptmx_inode = tty->link->driver_data;
+	devpts_kill_index(ptmx_inode, tty->index);
+	iput(ptmx_inode); /* drop reference we acquired at ptmx_open */
 }
 
 static const struct tty_operations ptm_unix98_ops = {
@@ -773,6 +780,15 @@ static int ptmx_open(struct inode *inode, struct file *filp)
 	set_bit(TTY_PTY_LOCK, &tty->flags); /* LOCK THE SLAVE */
 	tty->driver_data = inode;
 
+	/*
+	 * In the case where all references to ptmx inode are dropped and we
+	 * still have /dev/tty opened pointing to the master/slave pair (ptmx
+	 * is closed/released before /dev/tty), we must make sure that the inode
+	 * is still valid when we call the final pty_unix98_shutdown, thus we
+	 * hold an additional reference to the ptmx inode
+	 */
+	ihold(inode);
+
 	tty_add_file(tty, filp);
 
 	slave_inode = devpts_pty_new(inode,
-- 
2.4.3

[PATCH 2/2] pty: make sure super_block is still valid in final /dev/tty close

[Herton R. Krzesinski]

Considering current pty code and multiple devpts instances, it's possible
to umount a devpts file system while a program still has /dev/tty opened
pointing to a previosuly closed pty pair in that instance. In the case all
ptmx and pts/N files are closed, umount can be done. If the program closes
/dev/tty after umount is done, devpts_kill_index will use now an invalid
super_block, which was already destroyed in the umount operation after
running ->kill_sb. This is another "use after free" type of issue, but now
related to the allocated super_block instance.

To avoid the problem (warning at ida_remove and potential crashes) for
this specific case, I added two functions in devpts which grabs additional
references to the super_block, which pty code now uses so it makes sure
the super block structure is still valid until pty shutdown is done.
I also moved the additional inode references to the same functions, which
also covered similar case with inode being freed before /dev/tty final
close/shutdown.

Signed-off-by: Herton R. Krzesinski <herton@redhat.com>
Cc: stable@vger.kernel.org # 2.6.29+
---
 drivers/tty/pty.c         |  9 ++++++---
 fs/devpts/inode.c         | 20 ++++++++++++++++++++
 include/linux/devpts_fs.h |  4 ++++
 3 files changed, 30 insertions(+), 3 deletions(-)

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index 96016e5..7fc1b3e 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -688,7 +688,7 @@ static void pty_unix98_shutdown(struct tty_struct *tty)
 	else
 		ptmx_inode = tty->link->driver_data;
 	devpts_kill_index(ptmx_inode, tty->index);
-	iput(ptmx_inode); /* drop reference we acquired at ptmx_open */
+	devpts_iput_sb_deactive(ptmx_inode);
 }
 
 static const struct tty_operations ptm_unix98_ops = {
@@ -785,9 +785,12 @@ static int ptmx_open(struct inode *inode, struct file *filp)
 	 * still have /dev/tty opened pointing to the master/slave pair (ptmx
 	 * is closed/released before /dev/tty), we must make sure that the inode
 	 * is still valid when we call the final pty_unix98_shutdown, thus we
-	 * hold an additional reference to the ptmx inode
+	 * hold an additional reference to the ptmx inode. For the same /dev/tty
+	 * last close case, we also need to make sure the super_block isn't
+	 * destroyed (devpts instance unmounted), before /dev/tty is closed and
+	 * on its release devpts_kill_index is called.
 	 */
-	ihold(inode);
+	devpts_ihold_sb_active(inode);
 
 	tty_add_file(tty, filp);
 
diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
index c35ffdc..66a5421 100644
--- a/fs/devpts/inode.c
+++ b/fs/devpts/inode.c
@@ -575,6 +575,26 @@ void devpts_kill_index(struct inode *ptmx_inode, int idx)
 	mutex_unlock(&allocated_ptys_lock);
 }
 
+/*
+ * pty code needs to hold extra references in case of last /dev/tty close
+ */
+
+void devpts_ihold_sb_active(struct inode *ptmx_inode)
+{
+	struct super_block *sb = pts_sb_from_inode(ptmx_inode);
+
+	atomic_inc(&sb->s_active);
+	ihold(ptmx_inode);
+}
+
+void devpts_iput_sb_deactive(struct inode *ptmx_inode)
+{
+	struct super_block *sb = pts_sb_from_inode(ptmx_inode);
+
+	iput(ptmx_inode);
+	deactivate_super(sb);
+}
+
 /**
  * devpts_pty_new -- create a new inode in /dev/pts/
  * @ptmx_inode: inode of the master
diff --git a/include/linux/devpts_fs.h b/include/linux/devpts_fs.h
index 251a209..f73ef49 100644
--- a/include/linux/devpts_fs.h
+++ b/include/linux/devpts_fs.h
@@ -19,6 +19,8 @@
 
 int devpts_new_index(struct inode *ptmx_inode);
 void devpts_kill_index(struct inode *ptmx_inode, int idx);
+void devpts_ihold_sb_active(struct inode *ptmx_inode);
+void devpts_iput_sb_deactive(struct inode *ptmx_inode);
 /* mknod in devpts */
 struct inode *devpts_pty_new(struct inode *ptmx_inode, dev_t device, int index,
 		void *priv);
@@ -32,6 +34,8 @@ void devpts_pty_kill(struct inode *inode);
 /* Dummy stubs in the no-pty case */
 static inline int devpts_new_index(struct inode *ptmx_inode) { return -EINVAL; }
 static inline void devpts_kill_index(struct inode *ptmx_inode, int idx) { }
+static inline void devpts_ihold_sb_active(struct inode *ptmx_inode) { }
+static inline void devpts_iput_sb_deactive(struct inode *ptmx_inode) { }
 static inline struct inode *devpts_pty_new(struct inode *ptmx_inode,
 		dev_t device, int index, void *priv)
 {
-- 
2.4.3

Re: [PATCH 1/2 v2] pty: fix possible use after free of tty->driver_data

[Peter Hurley]

On 01/11/2016 06:07 AM, Herton R. Krzesinski wrote:
> This change fixes a bug for a corner case where we have the the last
> release from a pty master/slave coming from a previously opened /dev/tty
> file. When this happens, the tty->driver_data can be stale, due to all
> ptmx or pts/N files having already been closed before (and thus the inode
> related to these files, which tty->driver_data points to, being already
> freed/destroyed).
> 
> The fix here is to keep a reference on the opened master ptmx inode.
> We maintain the inode referenced until the final pty_unix98_shutdown,
> and only pass this inode to devpts_kill_index.

Ideally, the tty core should be bumping the inode count for the underlying
controlling tty but I'm not sure how to make that work atm, and this
fixes the (overwhelmingly) most common use-case.

Thanks again,

Reviewed-by: Peter Hurley <peter@hurleysoftware.com>

Re: [PATCH 2/2] pty: make sure super_block is still valid in final /dev/tty close

[Peter Hurley]

Hi Herton,

On 01/11/2016 06:07 AM, Herton R. Krzesinski wrote:
> Considering current pty code and multiple devpts instances, it's possible
> to umount a devpts file system while a program still has /dev/tty opened
> pointing to a previosuly closed pty pair in that instance. In the case all
> ptmx and pts/N files are closed, umount can be done. If the program closes
> /dev/tty after umount is done, devpts_kill_index will use now an invalid
> super_block, which was already destroyed in the umount operation after
> running ->kill_sb. This is another "use after free" type of issue, but now
> related to the allocated super_block instance.
> 
> To avoid the problem (warning at ida_remove and potential crashes) for
> this specific case, I added two functions in devpts which grabs additional
> references to the super_block, which pty code now uses so it makes sure
> the super block structure is still valid until pty shutdown is done.
> I also moved the additional inode references to the same functions, which
> also covered similar case with inode being freed before /dev/tty final
> close/shutdown.

Thanks for discovering and working this problem.
Comments below.


> Signed-off-by: Herton R. Krzesinski <herton@redhat.com>
> Cc: stable@vger.kernel.org # 2.6.29+
> ---
>  drivers/tty/pty.c         |  9 ++++++---
>  fs/devpts/inode.c         | 20 ++++++++++++++++++++
>  include/linux/devpts_fs.h |  4 ++++
>  3 files changed, 30 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
> index 96016e5..7fc1b3e 100644
> --- a/drivers/tty/pty.c
> +++ b/drivers/tty/pty.c
> @@ -688,7 +688,7 @@ static void pty_unix98_shutdown(struct tty_struct *tty)
>  	else
>  		ptmx_inode = tty->link->driver_data;
>  	devpts_kill_index(ptmx_inode, tty->index);
> -	iput(ptmx_inode); /* drop reference we acquired at ptmx_open */
> +	devpts_iput_sb_deactive(ptmx_inode);
>  }
>  
>  static const struct tty_operations ptm_unix98_ops = {
> @@ -785,9 +785,12 @@ static int ptmx_open(struct inode *inode, struct file *filp)
>  	 * still have /dev/tty opened pointing to the master/slave pair (ptmx
>  	 * is closed/released before /dev/tty), we must make sure that the inode
>  	 * is still valid when we call the final pty_unix98_shutdown, thus we
> -	 * hold an additional reference to the ptmx inode
> +	 * hold an additional reference to the ptmx inode. For the same /dev/tty
> +	 * last close case, we also need to make sure the super_block isn't
> +	 * destroyed (devpts instance unmounted), before /dev/tty is closed and
> +	 * on its release devpts_kill_index is called.
>  	 */
> -	ihold(inode);
> +	devpts_ihold_sb_active(inode);
>  
>  	tty_add_file(tty, filp);
>  
> diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
> index c35ffdc..66a5421 100644
> --- a/fs/devpts/inode.c
> +++ b/fs/devpts/inode.c
> @@ -575,6 +575,26 @@ void devpts_kill_index(struct inode *ptmx_inode, int idx)
>  	mutex_unlock(&allocated_ptys_lock);
>  }
>  
> +/*
> + * pty code needs to hold extra references in case of last /dev/tty close
> + */
> +
> +void devpts_ihold_sb_active(struct inode *ptmx_inode)
> +{
> +	struct super_block *sb = pts_sb_from_inode(ptmx_inode);
> +
> +	atomic_inc(&sb->s_active);
> +	ihold(ptmx_inode);
> +}
> +
> +void devpts_iput_sb_deactive(struct inode *ptmx_inode)
> +{
> +	struct super_block *sb = pts_sb_from_inode(ptmx_inode);
> +
> +	iput(ptmx_inode);
> +	deactivate_super(sb);
> +}

We might as well roll in this functionality into
devpts_new_index() and devpts_kill_index().

I realize that's muddying the separation of concern.

Alternatively, name the functions for the logical operation
rather than specifically for what they do (eg. devpts_add_ref())

Regards,
Peter Hurley


> +
>  /**
>   * devpts_pty_new -- create a new inode in /dev/pts/
>   * @ptmx_inode: inode of the master
> diff --git a/include/linux/devpts_fs.h b/include/linux/devpts_fs.h
> index 251a209..f73ef49 100644
> --- a/include/linux/devpts_fs.h
> +++ b/include/linux/devpts_fs.h
> @@ -19,6 +19,8 @@
>  
>  int devpts_new_index(struct inode *ptmx_inode);
>  void devpts_kill_index(struct inode *ptmx_inode, int idx);
> +void devpts_ihold_sb_active(struct inode *ptmx_inode);
> +void devpts_iput_sb_deactive(struct inode *ptmx_inode);
>  /* mknod in devpts */
>  struct inode *devpts_pty_new(struct inode *ptmx_inode, dev_t device, int index,
>  		void *priv);
> @@ -32,6 +34,8 @@ void devpts_pty_kill(struct inode *inode);
>  /* Dummy stubs in the no-pty case */
>  static inline int devpts_new_index(struct inode *ptmx_inode) { return -EINVAL; }
>  static inline void devpts_kill_index(struct inode *ptmx_inode, int idx) { }
> +static inline void devpts_ihold_sb_active(struct inode *ptmx_inode) { }
> +static inline void devpts_iput_sb_deactive(struct inode *ptmx_inode) { }
>  static inline struct inode *devpts_pty_new(struct inode *ptmx_inode,
>  		dev_t device, int index, void *priv)
>  {
> 

Re: [PATCH 1/2 v2] pty: fix possible use after free of tty->driver_data

[Josh Triplett]

On Wed, Jan 13, 2016 at 09:39:29AM -0800, Peter Hurley wrote:
> On 01/11/2016 06:07 AM, Herton R. Krzesinski wrote:
> > This change fixes a bug for a corner case where we have the the last
> > release from a pty master/slave coming from a previously opened /dev/tty
> > file. When this happens, the tty->driver_data can be stale, due to all
> > ptmx or pts/N files having already been closed before (and thus the inode
> > related to these files, which tty->driver_data points to, being already
> > freed/destroyed).
> > 
> > The fix here is to keep a reference on the opened master ptmx inode.
> > We maintain the inode referenced until the final pty_unix98_shutdown,
> > and only pass this inode to devpts_kill_index.
> 
> Ideally, the tty core should be bumping the inode count for the underlying
> controlling tty

That does indeed sound like the right fix.  /dev/tty doesn't act exactly
like opening the underlying device (as it also supports the TIOCNOTTY
ioctl), but it should definitely hold a reference to that underlying
device.

[PATCH 2/2 v2] pty: make sure super_block is still valid in final /dev/tty close

[Herton R. Krzesinski]

Considering current pty code and multiple devpts instances, it's possible
to umount a devpts file system while a program still has /dev/tty opened
pointing to a previosuly closed pty pair in that instance. In the case all
ptmx and pts/N files are closed, umount can be done. If the program closes
/dev/tty after umount is done, devpts_kill_index will use now an invalid
super_block, which was already destroyed in the umount operation after
running ->kill_sb. This is another "use after free" type of issue, but now
related to the allocated super_block instance.

To avoid the problem (warning at ida_remove and potential crashes) for
this specific case, I added two functions in devpts which grabs additional
references to the super_block, which pty code now uses so it makes sure
the super block structure is still valid until pty shutdown is done.
I also moved the additional inode references to the same functions, which
also covered similar case with inode being freed before /dev/tty final
close/shutdown.

Signed-off-by: Herton R. Krzesinski <herton@redhat.com>
Cc: stable@vger.kernel.org # 2.6.29+
---
 drivers/tty/pty.c         |  9 ++++++---
 fs/devpts/inode.c         | 20 ++++++++++++++++++++
 include/linux/devpts_fs.h |  4 ++++
 3 files changed, 30 insertions(+), 3 deletions(-)

v2: rename devpts_ihold_sb_active to devpts_add_ref
    rename devpts_iput_sb_deactive to devpts_del_ref

diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
index 3b5cde8..2348fa6 100644
--- a/drivers/tty/pty.c
+++ b/drivers/tty/pty.c
@@ -688,7 +688,7 @@ static void pty_unix98_shutdown(struct tty_struct *tty)
 	else
 		ptmx_inode = tty->link->driver_data;
 	devpts_kill_index(ptmx_inode, tty->index);
-	iput(ptmx_inode); /* drop reference we acquired at ptmx_open */
+	devpts_del_ref(ptmx_inode);
 }
 
 static const struct tty_operations ptm_unix98_ops = {
@@ -785,9 +785,12 @@ static int ptmx_open(struct inode *inode, struct file *filp)
 	 * still have /dev/tty opened pointing to the master/slave pair (ptmx
 	 * is closed/released before /dev/tty), we must make sure that the inode
 	 * is still valid when we call the final pty_unix98_shutdown, thus we
-	 * hold an additional reference to the ptmx inode
+	 * hold an additional reference to the ptmx inode. For the same /dev/tty
+	 * last close case, we also need to make sure the super_block isn't
+	 * destroyed (devpts instance unmounted), before /dev/tty is closed and
+	 * on its release devpts_kill_index is called.
 	 */
-	ihold(inode);
+	devpts_add_ref(inode);
 
 	tty_add_file(tty, filp);
 
diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
index c35ffdc..706de32 100644
--- a/fs/devpts/inode.c
+++ b/fs/devpts/inode.c
@@ -575,6 +575,26 @@ void devpts_kill_index(struct inode *ptmx_inode, int idx)
 	mutex_unlock(&allocated_ptys_lock);
 }
 
+/*
+ * pty code needs to hold extra references in case of last /dev/tty close
+ */
+
+void devpts_add_ref(struct inode *ptmx_inode)
+{
+	struct super_block *sb = pts_sb_from_inode(ptmx_inode);
+
+	atomic_inc(&sb->s_active);
+	ihold(ptmx_inode);
+}
+
+void devpts_del_ref(struct inode *ptmx_inode)
+{
+	struct super_block *sb = pts_sb_from_inode(ptmx_inode);
+
+	iput(ptmx_inode);
+	deactivate_super(sb);
+}
+
 /**
  * devpts_pty_new -- create a new inode in /dev/pts/
  * @ptmx_inode: inode of the master
diff --git a/include/linux/devpts_fs.h b/include/linux/devpts_fs.h
index 251a209..e0ee0b3 100644
--- a/include/linux/devpts_fs.h
+++ b/include/linux/devpts_fs.h
@@ -19,6 +19,8 @@
 
 int devpts_new_index(struct inode *ptmx_inode);
 void devpts_kill_index(struct inode *ptmx_inode, int idx);
+void devpts_add_ref(struct inode *ptmx_inode);
+void devpts_del_ref(struct inode *ptmx_inode);
 /* mknod in devpts */
 struct inode *devpts_pty_new(struct inode *ptmx_inode, dev_t device, int index,
 		void *priv);
@@ -32,6 +34,8 @@ void devpts_pty_kill(struct inode *inode);
 /* Dummy stubs in the no-pty case */
 static inline int devpts_new_index(struct inode *ptmx_inode) { return -EINVAL; }
 static inline void devpts_kill_index(struct inode *ptmx_inode, int idx) { }
+static inline void devpts_add_ref(struct inode *ptmx_inode) { }
+static inline void devpts_del_ref(struct inode *ptmx_inode) { }
 static inline struct inode *devpts_pty_new(struct inode *ptmx_inode,
 		dev_t device, int index, void *priv)
 {
-- 
2.4.3

Re: [PATCH 2/2] pty: make sure super_block is still valid in final /dev/tty close

[Herton R. Krzesinski]

On Wed, Jan 13, 2016 at 09:54:03AM -0800, Peter Hurley wrote:
> Hi Herton,
> 
> On 01/11/2016 06:07 AM, Herton R. Krzesinski wrote:
> > Considering current pty code and multiple devpts instances, it's possible
> > to umount a devpts file system while a program still has /dev/tty opened
> > pointing to a previosuly closed pty pair in that instance. In the case all
> > ptmx and pts/N files are closed, umount can be done. If the program closes
> > /dev/tty after umount is done, devpts_kill_index will use now an invalid
> > super_block, which was already destroyed in the umount operation after
> > running ->kill_sb. This is another "use after free" type of issue, but now
> > related to the allocated super_block instance.
> > 
> > To avoid the problem (warning at ida_remove and potential crashes) for
> > this specific case, I added two functions in devpts which grabs additional
> > references to the super_block, which pty code now uses so it makes sure
> > the super block structure is still valid until pty shutdown is done.
> > I also moved the additional inode references to the same functions, which
> > also covered similar case with inode being freed before /dev/tty final
> > close/shutdown.
> 
> Thanks for discovering and working this problem.
> Comments below.

Thanks for looking/reviewing the patches! :)

> 
> 
> > Signed-off-by: Herton R. Krzesinski <herton@redhat.com>
> > Cc: stable@vger.kernel.org # 2.6.29+
> > ---
> >  drivers/tty/pty.c         |  9 ++++++---
> >  fs/devpts/inode.c         | 20 ++++++++++++++++++++
> >  include/linux/devpts_fs.h |  4 ++++
> >  3 files changed, 30 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
> > index 96016e5..7fc1b3e 100644
> > --- a/drivers/tty/pty.c
> > +++ b/drivers/tty/pty.c
> > @@ -688,7 +688,7 @@ static void pty_unix98_shutdown(struct tty_struct *tty)
> >  	else
> >  		ptmx_inode = tty->link->driver_data;
> >  	devpts_kill_index(ptmx_inode, tty->index);
> > -	iput(ptmx_inode); /* drop reference we acquired at ptmx_open */
> > +	devpts_iput_sb_deactive(ptmx_inode);
> >  }
> >  
> >  static const struct tty_operations ptm_unix98_ops = {
> > @@ -785,9 +785,12 @@ static int ptmx_open(struct inode *inode, struct file *filp)
> >  	 * still have /dev/tty opened pointing to the master/slave pair (ptmx
> >  	 * is closed/released before /dev/tty), we must make sure that the inode
> >  	 * is still valid when we call the final pty_unix98_shutdown, thus we
> > -	 * hold an additional reference to the ptmx inode
> > +	 * hold an additional reference to the ptmx inode. For the same /dev/tty
> > +	 * last close case, we also need to make sure the super_block isn't
> > +	 * destroyed (devpts instance unmounted), before /dev/tty is closed and
> > +	 * on its release devpts_kill_index is called.
> >  	 */
> > -	ihold(inode);
> > +	devpts_ihold_sb_active(inode);
> >  
> >  	tty_add_file(tty, filp);
> >  
> > diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
> > index c35ffdc..66a5421 100644
> > --- a/fs/devpts/inode.c
> > +++ b/fs/devpts/inode.c
> > @@ -575,6 +575,26 @@ void devpts_kill_index(struct inode *ptmx_inode, int idx)
> >  	mutex_unlock(&allocated_ptys_lock);
> >  }
> >  
> > +/*
> > + * pty code needs to hold extra references in case of last /dev/tty close
> > + */
> > +
> > +void devpts_ihold_sb_active(struct inode *ptmx_inode)
> > +{
> > +	struct super_block *sb = pts_sb_from_inode(ptmx_inode);
> > +
> > +	atomic_inc(&sb->s_active);
> > +	ihold(ptmx_inode);
> > +}
> > +
> > +void devpts_iput_sb_deactive(struct inode *ptmx_inode)
> > +{
> > +	struct super_block *sb = pts_sb_from_inode(ptmx_inode);
> > +
> > +	iput(ptmx_inode);
> > +	deactivate_super(sb);
> > +}
> 
> We might as well roll in this functionality into
> devpts_new_index() and devpts_kill_index().
> 
> I realize that's muddying the separation of concern.
> 
> Alternatively, name the functions for the logical operation
> rather than specifically for what they do (eg. devpts_add_ref())

I renamed the functions and submitted a v2. Not sure if you also want the
move to devpts_new_index()/devpts_kill_index(), I left that as is for now.
I personally don't have much preference here, I left as separate functions
which are called at pty.c because that seemed to be "more logical", in that it's
a tty specific requirement and not tied to devpts fs itself, not sure if it makes
sense (and anyway devpts is only used by tty anyway). It's mostly a cosmetic/
different way of doing thigs, but if required or preferred I can do a v3 and
move/incorporate it into devpts_new_index and devpts_kill_index.

> 
> Regards,
> Peter Hurley
> 
> 
> > +
> >  /**
> >   * devpts_pty_new -- create a new inode in /dev/pts/
> >   * @ptmx_inode: inode of the master
> > diff --git a/include/linux/devpts_fs.h b/include/linux/devpts_fs.h
> > index 251a209..f73ef49 100644
> > --- a/include/linux/devpts_fs.h
> > +++ b/include/linux/devpts_fs.h
> > @@ -19,6 +19,8 @@
> >  
> >  int devpts_new_index(struct inode *ptmx_inode);
> >  void devpts_kill_index(struct inode *ptmx_inode, int idx);
> > +void devpts_ihold_sb_active(struct inode *ptmx_inode);
> > +void devpts_iput_sb_deactive(struct inode *ptmx_inode);
> >  /* mknod in devpts */
> >  struct inode *devpts_pty_new(struct inode *ptmx_inode, dev_t device, int index,
> >  		void *priv);
> > @@ -32,6 +34,8 @@ void devpts_pty_kill(struct inode *inode);
> >  /* Dummy stubs in the no-pty case */
> >  static inline int devpts_new_index(struct inode *ptmx_inode) { return -EINVAL; }
> >  static inline void devpts_kill_index(struct inode *ptmx_inode, int idx) { }
> > +static inline void devpts_ihold_sb_active(struct inode *ptmx_inode) { }
> > +static inline void devpts_iput_sb_deactive(struct inode *ptmx_inode) { }
> >  static inline struct inode *devpts_pty_new(struct inode *ptmx_inode,
> >  		dev_t device, int index, void *priv)
> >  {
> > 
> 

-- 
Regards,
Herton

Re: [PATCH 1/2 v2] pty: fix possible use after free of tty->driver_data

[Herton R. Krzesinski]

On Wed, Jan 13, 2016 at 10:28:44AM -0800, Josh Triplett wrote:
> On Wed, Jan 13, 2016 at 09:39:29AM -0800, Peter Hurley wrote:
> > On 01/11/2016 06:07 AM, Herton R. Krzesinski wrote:
> > > This change fixes a bug for a corner case where we have the the last
> > > release from a pty master/slave coming from a previously opened /dev/tty
> > > file. When this happens, the tty->driver_data can be stale, due to all
> > > ptmx or pts/N files having already been closed before (and thus the inode
> > > related to these files, which tty->driver_data points to, being already
> > > freed/destroyed).
> > > 
> > > The fix here is to keep a reference on the opened master ptmx inode.
> > > We maintain the inode referenced until the final pty_unix98_shutdown,
> > > and only pass this inode to devpts_kill_index.
> > 
> > Ideally, the tty core should be bumping the inode count for the underlying
> > controlling tty
> 
> That does indeed sound like the right fix.  /dev/tty doesn't act exactly
> like opening the underlying device (as it also supports the TIOCNOTTY
> ioctl), but it should definitely hold a reference to that underlying
> device.

Yeah, I thought previously as well that this should go to tty core (my thinking
though was to get extra references to the opened files instead of the inode).

However, what inode should I choose to increment the reference count in the
tty->tty_files list? I know most cases the device file will always be at the
/dev, but if it's opened from another place/file?

And handling this would overcomplicate a case which is pty specific, unless
I miss something here pty is the only inode user, and seem not worth or
useful to have at the moment this at tty core.

thanks,
-- 
Herton.

Re: [PATCH 1/2 v2] pty: fix possible use after free of tty->driver_data

[Peter Hurley]

On 01/14/2016 12:09 PM, Herton R. Krzesinski wrote:
> On Wed, Jan 13, 2016 at 10:28:44AM -0800, Josh Triplett wrote:
>> On Wed, Jan 13, 2016 at 09:39:29AM -0800, Peter Hurley wrote:
>>> On 01/11/2016 06:07 AM, Herton R. Krzesinski wrote:
>>>> This change fixes a bug for a corner case where we have the the last
>>>> release from a pty master/slave coming from a previously opened /dev/tty
>>>> file. When this happens, the tty->driver_data can be stale, due to all
>>>> ptmx or pts/N files having already been closed before (and thus the inode
>>>> related to these files, which tty->driver_data points to, being already
>>>> freed/destroyed).
>>>>
>>>> The fix here is to keep a reference on the opened master ptmx inode.
>>>> We maintain the inode referenced until the final pty_unix98_shutdown,
>>>> and only pass this inode to devpts_kill_index.
>>>
>>> Ideally, the tty core should be bumping the inode count for the underlying
>>> controlling tty
>>
>> That does indeed sound like the right fix.  /dev/tty doesn't act exactly
>> like opening the underlying device (as it also supports the TIOCNOTTY
>> ioctl), but it should definitely hold a reference to that underlying
>> device.

I'm ok with this fix for the moment because it's ideal for -stable;
simple, straightforward and addresses the one known use-case.


> Yeah, I thought previously as well that this should go to tty core (my thinking
> though was to get extra references to the opened files instead of the inode).
> 
> However, what inode should I choose to increment the reference count in the
> tty->tty_files list? I know most cases the device file will always be at the
> /dev, but if it's opened from another place/file?

I think the long-term solution would be to bump the inode ref at
controlling tty association because the appropriate inode is supplied for
either the ioctl(TIOCSCTTY) or the first qualifying open(). This inode
would be stored in the tty_struct, justifying the reference.

At dissociation, the inode reference would be dropped.

This approach also magically fixes the superblock pinning (because the inode
would be the slave inode so would be on the devpts fs, preventing umount).


> And handling this would overcomplicate a case which is pty specific, unless
> I miss something here pty is the only inode user, and seem not worth or
> useful to have at the moment this at tty core.

Any tty could be the process's controlling tty, and the tty core should be
pinning any fs objects related to that.

Regards,
Peter Hurley

Re: [PATCH 2/2 v2] pty: make sure super_block is still valid in final /dev/tty close

[Peter Hurley]

On 01/14/2016 11:56 AM, Herton R. Krzesinski wrote:
> Considering current pty code and multiple devpts instances, it's possible
> to umount a devpts file system while a program still has /dev/tty opened
> pointing to a previosuly closed pty pair in that instance. In the case all
> ptmx and pts/N files are closed, umount can be done. If the program closes
> /dev/tty after umount is done, devpts_kill_index will use now an invalid
> super_block, which was already destroyed in the umount operation after
> running ->kill_sb. This is another "use after free" type of issue, but now
> related to the allocated super_block instance.
> 
> To avoid the problem (warning at ida_remove and potential crashes) for
> this specific case, I added two functions in devpts which grabs additional
> references to the super_block, which pty code now uses so it makes sure
> the super block structure is still valid until pty shutdown is done.
> I also moved the additional inode references to the same functions, which
> also covered similar case with inode being freed before /dev/tty final
> close/shutdown.

Thanks again.

Reviewed-by: Peter Hurley <peter@hurleysoftware.com>

Re: [PATCH 2/2] pty: make sure super_block is still valid in final /dev/tty close

[Peter Hurley]

On 01/14/2016 12:03 PM, Herton R. Krzesinski wrote:
> On Wed, Jan 13, 2016 at 09:54:03AM -0800, Peter Hurley wrote:
>> Hi Herton,
>>
>> On 01/11/2016 06:07 AM, Herton R. Krzesinski wrote:
>>> Considering current pty code and multiple devpts instances, it's possible
>>> to umount a devpts file system while a program still has /dev/tty opened
>>> pointing to a previosuly closed pty pair in that instance. In the case all
>>> ptmx and pts/N files are closed, umount can be done. If the program closes
>>> /dev/tty after umount is done, devpts_kill_index will use now an invalid
>>> super_block, which was already destroyed in the umount operation after
>>> running ->kill_sb. This is another "use after free" type of issue, but now
>>> related to the allocated super_block instance.
>>>
>>> To avoid the problem (warning at ida_remove and potential crashes) for
>>> this specific case, I added two functions in devpts which grabs additional
>>> references to the super_block, which pty code now uses so it makes sure
>>> the super block structure is still valid until pty shutdown is done.
>>> I also moved the additional inode references to the same functions, which
>>> also covered similar case with inode being freed before /dev/tty final
>>> close/shutdown.
>>
>> Thanks for discovering and working this problem.
>> Comments below.
> 
> Thanks for looking/reviewing the patches! :)
> 
>>
>>
>>> Signed-off-by: Herton R. Krzesinski <herton@redhat.com>
>>> Cc: stable@vger.kernel.org # 2.6.29+
>>> ---
>>>  drivers/tty/pty.c         |  9 ++++++---
>>>  fs/devpts/inode.c         | 20 ++++++++++++++++++++
>>>  include/linux/devpts_fs.h |  4 ++++
>>>  3 files changed, 30 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/drivers/tty/pty.c b/drivers/tty/pty.c
>>> index 96016e5..7fc1b3e 100644
>>> --- a/drivers/tty/pty.c
>>> +++ b/drivers/tty/pty.c
>>> @@ -688,7 +688,7 @@ static void pty_unix98_shutdown(struct tty_struct *tty)
>>>  	else
>>>  		ptmx_inode = tty->link->driver_data;
>>>  	devpts_kill_index(ptmx_inode, tty->index);
>>> -	iput(ptmx_inode); /* drop reference we acquired at ptmx_open */
>>> +	devpts_iput_sb_deactive(ptmx_inode);
>>>  }
>>>  
>>>  static const struct tty_operations ptm_unix98_ops = {
>>> @@ -785,9 +785,12 @@ static int ptmx_open(struct inode *inode, struct file *filp)
>>>  	 * still have /dev/tty opened pointing to the master/slave pair (ptmx
>>>  	 * is closed/released before /dev/tty), we must make sure that the inode
>>>  	 * is still valid when we call the final pty_unix98_shutdown, thus we
>>> -	 * hold an additional reference to the ptmx inode
>>> +	 * hold an additional reference to the ptmx inode. For the same /dev/tty
>>> +	 * last close case, we also need to make sure the super_block isn't
>>> +	 * destroyed (devpts instance unmounted), before /dev/tty is closed and
>>> +	 * on its release devpts_kill_index is called.
>>>  	 */
>>> -	ihold(inode);
>>> +	devpts_ihold_sb_active(inode);
>>>  
>>>  	tty_add_file(tty, filp);
>>>  
>>> diff --git a/fs/devpts/inode.c b/fs/devpts/inode.c
>>> index c35ffdc..66a5421 100644
>>> --- a/fs/devpts/inode.c
>>> +++ b/fs/devpts/inode.c
>>> @@ -575,6 +575,26 @@ void devpts_kill_index(struct inode *ptmx_inode, int idx)
>>>  	mutex_unlock(&allocated_ptys_lock);
>>>  }
>>>  
>>> +/*
>>> + * pty code needs to hold extra references in case of last /dev/tty close
>>> + */
>>> +
>>> +void devpts_ihold_sb_active(struct inode *ptmx_inode)
>>> +{
>>> +	struct super_block *sb = pts_sb_from_inode(ptmx_inode);
>>> +
>>> +	atomic_inc(&sb->s_active);
>>> +	ihold(ptmx_inode);
>>> +}
>>> +
>>> +void devpts_iput_sb_deactive(struct inode *ptmx_inode)
>>> +{
>>> +	struct super_block *sb = pts_sb_from_inode(ptmx_inode);
>>> +
>>> +	iput(ptmx_inode);
>>> +	deactivate_super(sb);
>>> +}
>>
>> We might as well roll in this functionality into
>> devpts_new_index() and devpts_kill_index().
>>
>> I realize that's muddying the separation of concern.
>>
>> Alternatively, name the functions for the logical operation
>> rather than specifically for what they do (eg. devpts_add_ref())
> 
> I renamed the functions and submitted a v2. Not sure if you also want the
> move to devpts_new_index()/devpts_kill_index(), I left that as is for now.
> I personally don't have much preference here, I left as separate functions
> which are called at pty.c because that seemed to be "more logical", in that it's
> a tty specific requirement and not tied to devpts fs itself, not sure if it makes
> sense (and anyway devpts is only used by tty anyway). It's mostly a cosmetic/
> different way of doing thigs, but if required or preferred I can do a v3 and
> move/incorporate it into devpts_new_index and devpts_kill_index.

Yeah, that fine. Thanks again for your work on this.

Regards,
Peter Hurley


>>> +
>>>  /**
>>>   * devpts_pty_new -- create a new inode in /dev/pts/
>>>   * @ptmx_inode: inode of the master
>>> diff --git a/include/linux/devpts_fs.h b/include/linux/devpts_fs.h
>>> index 251a209..f73ef49 100644
>>> --- a/include/linux/devpts_fs.h
>>> +++ b/include/linux/devpts_fs.h
>>> @@ -19,6 +19,8 @@
>>>  
>>>  int devpts_new_index(struct inode *ptmx_inode);
>>>  void devpts_kill_index(struct inode *ptmx_inode, int idx);
>>> +void devpts_ihold_sb_active(struct inode *ptmx_inode);
>>> +void devpts_iput_sb_deactive(struct inode *ptmx_inode);
>>>  /* mknod in devpts */
>>>  struct inode *devpts_pty_new(struct inode *ptmx_inode, dev_t device, int index,
>>>  		void *priv);
>>> @@ -32,6 +34,8 @@ void devpts_pty_kill(struct inode *inode);
>>>  /* Dummy stubs in the no-pty case */
>>>  static inline int devpts_new_index(struct inode *ptmx_inode) { return -EINVAL; }
>>>  static inline void devpts_kill_index(struct inode *ptmx_inode, int idx) { }
>>> +static inline void devpts_ihold_sb_active(struct inode *ptmx_inode) { }
>>> +static inline void devpts_iput_sb_deactive(struct inode *ptmx_inode) { }
>>>  static inline struct inode *devpts_pty_new(struct inode *ptmx_inode,
>>>  		dev_t device, int index, void *priv)
>>>  {
>>>
>>
>