linux-stable.git/drivers/tty/pty.c, branch v4.13.2

pty: Repair TIOCGPTPEER

2017-08-24T20:23:03+00:00

The implementation of TIOCGPTPEER has two issues.

When /dev/ptmx (as opposed to /dev/pts/ptmx) is opened the wrong
vfsmount is passed to dentry_open.  Which results in the kernel displaying
the wrong pathname for the peer.

The second is simply by caching the vfsmount and dentry of the peer it leaves
them open, in a way they were not previously Which because of the inreased
reference counts can cause unnecessary behaviour differences resulting in
regressions.

To fix these move the ioctl into tty_io.c at a generic level allowing
the ioctl to have access to the struct file on which the ioctl is
being called.  This allows the path of the slave to be derived when
opening the slave through TIOCGPTPEER instead of requiring the path to
the slave be cached.  Thus removing the need for caching the path.

A new function devpts_ptmx_path is factored out of devpts_acquire and
used to implement a function devpts_mntget.   The new function devpts_mntget
takes a filp to perform the lookup on and fsi so that it can confirm
that the superblock that is found by devpts_ptmx_path is the proper superblock.

v2: Lots of fixes to make the code actually work
v3: Suggestions by Linus
    - Removed the unnecessary initialization of filp in ptm_open_peer
    - Simplified devpts_ptmx_path as gotos are no longer required

[ This is the fix for the issue that was reverted in commit
  143c97cc6529, but this time without breaking 'pbuilder' due to
  increased reference counts   - Linus ]

Fixes: 54ebbfb16034 ("tty: add TIOCGPTPEER ioctl")
Reported-by: Christian Brauner 
Reported-and-tested-by: Stefan Lippers-Hollmann 
Signed-off-by: "Eric W. Biederman" 
Signed-off-by: Linus Torvalds

Revert "pty: fix the cached path of the pty slave file descriptor in the master"

2017-08-24T01:16:11+00:00

This reverts commit c8c03f1858331e85d397bacccd34ef409aae993c.

It turns out that while fixing the ptmx file descriptor to have the
correct 'struct path' to the associated slave pty is a really good
thing, it breaks some user space tools for a very annoying reason.

The problem is that /dev/ptmx and its associated slave pty (/dev/pts/X)
are on different mounts.  That was what caused us to have the wrong path
in the first place (we would mix up the vfsmount of the 'ptmx' node,
with the dentry of the pty slave node), but it also means that now while
we use the right vfsmount, having the pty master open also keeps the pts
mount busy.

And it turn sout that that makes 'pbuilder' very unhappy, as noted by
Stefan Lippers-Hollmann:

 "This patch introduces a regression for me when using pbuilder
  0.228.7[2] (a helper to build Debian packages in a chroot and to
  create and update its chroots) when trying to umount /dev/ptmx (inside
  the chroot) on Debian/ unstable (full log and pbuilder configuration
  file[3] attached).

  [...]
  Setting up build-essential (12.3) ...
  Processing triggers for libc-bin (2.24-15) ...
  I: unmounting dev/ptmx filesystem
  W: Could not unmount dev/ptmx: umount: /var/cache/pbuilder/build/1340/dev/ptmx: target is busy
          (In some cases useful info about processes that
           use the device is found by lsof(8) or fuser(1).)"

apparently pbuilder tries to unmount the /dev/pts filesystem while still
holding at least one master node open, which is arguably not very nice,
but we don't break user space even when fixing other bugs.

So this commit has to be reverted.

I'll try to figure out a way to avoid caching the path to the slave pty
in the master pty.  The only thing that actually wants that slave pty
path is the "TIOCGPTPEER" ioctl, and I think we could just recreate the
path at that time.

Reported-by: Stefan Lippers-Hollmann 
Cc: Eric W Biederman 
Cc: Christian Brauner 
Cc: Al Viro 
Signed-off-by: Linus Torvalds

pty: fix the cached path of the pty slave file descriptor in the master

2017-08-17T16:10:48+00:00

Christian Brauner reported that if you use the TIOCGPTPEER ioctl() to
get a slave pty file descriptor, the resulting file descriptor doesn't
look right in /proc//fd/.  In particular, he wanted to use
readlink() on /proc/self/fd/ to get the pathname of the slave pty
(basically implementing "ptsname{_r}()").

The reason for that was that we had generated the wrong 'struct path'
when we create the pty in ptmx_open().

In particular, the dentry was correct, but the vfsmount pointed to the
mount of the ptmx node. That _can_ be correct - in case you use
"/dev/pts/ptmx" to open the master - but usually is not.  The normal
case is to use /dev/ptmx, which then looks up the pts/ directory, and
then the vfsmount of the ptmx node is obviously the /dev directory, not
the /dev/pts/ directory.

We actually did have the right vfsmount available, but in the wrong
place (it gets looked up in 'devpts_acquire()' when we get a reference
to the pts filesystem), and so ptmx_open() used the wrong mnt pointer.

The end result of this confusion was that the pty worked fine, but when
if you did TIOCGPTPEER to get the slave side of the pty, end end result
would also work, but have that dodgy 'struct path'.

And then when doing "d_path()" on to get the pathname, the vfsmount
would not match the root of the pts directory, and d_path() would return
an empty pathname thinking that the entry had escaped a bind mount into
another mount.

This fixes the problem by making devpts_acquire() return the vfsmount
for the pts filesystem, allowing ptmx_open() to trivially just use the
right mount for the pts dentry, and create the proper 'struct path'.

Reported-by: Christian Brauner 
Cc: Al Viro 
Acked-by: Eric Biederman 
Signed-off-by: Linus Torvalds

tty: hide unused pty_get_peer function

2017-07-17T15:04:41+00:00

TIOCGPTPEER is only used for unix98 PTYs, and we get a warning
when those are disabled:

drivers/tty/pty.c:466:12: error: 'pty_get_peer' defined but not used [-Werror=unused-function]

This moves the respective functions inside of the existing #ifdef.

Fixes: 54ebbfb16034 ("tty: add TIOCGPTPEER ioctl")
Signed-off-by: Arnd Bergmann 
Acked-by: Aleksa Sarai 
Reviewed-by: Kees Cook 
Signed-off-by: Greg Kroah-Hartman

tty: add TIOCGPTPEER ioctl

2017-06-09T10:27:54+00:00

When opening the slave end of a PTY, it is not possible for userspace to
safely ensure that /dev/pts/$num is actually a slave (in cases where the
mount namespace in which devpts was mounted is controlled by an
untrusted process). In addition, there are several unresolvable
race conditions if userspace were to attempt to detect attacks through
stat(2) and other similar methods [in addition it is not clear how
userspace could detect attacks involving FUSE].

Resolve this by providing an interface for userpace to safely open the
"peer" end of a PTY file descriptor by using the dentry cached by
devpts. Since it is not possible to have an open master PTY without
having its slave exposed in /dev/pts this interface is safe. This
interface currently does not provide a way to get the master pty (since
it is not clear whether such an interface is safe or even useful).

Cc: Christian Brauner 
Cc: Valentin Rothberg 
Signed-off-by: Aleksa Sarai 
Signed-off-by: Greg Kroah-Hartman

tty: add compat_ioctl callbacks

2017-06-09T09:27:20+00:00

In order to avoid future diversions between fs/compat_ioctl.c and
drivers/tty/pty.c, define .compat_ioctl callbacks for the relevant
tty_operations structs. Since both pty_unix98_ioctl() and
pty_bsd_ioctl() are compatible between 32-bit and 64-bit userspace no
special translation is required.

Signed-off-by: Aleksa Sarai 
Reviewed-by: Arnd Bergmann 
Signed-off-by: Greg Kroah-Hartman

tty: pty: Fix ldisc flush after userspace become aware of the data already

2017-03-17T05:16:09+00:00

While using emacs, cat or others' commands in konsole with recent
kernels, I have met many times that CTRL-C freeze konsole. After
konsole freeze I can't type anything, then I have to open a new one,
it is very annoying.

See bug report:
https://bugs.kde.org/show_bug.cgi?id=175283

The platform in that bug report is Solaris, but now the pty in linux
has the same problem or the same behavior as Solaris :)

It has high possibility to trigger the problem follow steps below:
Note: In my test, BigFile is a text file whose size is bigger than 1G
1:open konsole
1:cat BigFile
2:CTRL-C

After some digging, I find out the reason is that commit 1d1d14da12e7
("pty: Fix buffer flush deadlock") changes the behavior of pty_flush_buffer.

Thread A                                 Thread B
--------                                 --------
1:n_tty_poll return POLLIN
                                         2:CTRL-C trigger pty_flush_buffer
                                             tty_buffer_flush
                                               n_tty_flush_buffer
3:attempt to check count of chars:
  ioctl(fd, TIOCINQ, &available)
  available is equal to 0

4:read(fd, buffer, avaiable)
  return 0

5:konsole close fd

Yes, I know we could use the same patch included in the BUG report as
a workaround for linux platform too. But I think the data in ldisc is
belong to application of another side, we shouldn't clear it when we
want to flush write buffer of this side in pty_flush_buffer. So I think
it is better to disable ldisc flush in pty_flush_buffer, because its new
hehavior bring no benefit except that it mess up the behavior between
POLLIN, and TIOCINQ or FIONREAD.

Also I find no flush_buffer function in others' tty driver has the
same behavior as current pty_flush_buffer.

Fixes: 1d1d14da12e7 ("pty: Fix buffer flush deadlock")
CC: stable@vger.kernel.org # v4.0+
Signed-off-by: Wang YanQing 
Signed-off-by: Greg Kroah-Hartman

sched/headers: Prepare for new header dependencies before moving code to

2017-03-02T07:42:29+00:00

We are going to split  out of , which
will have to be picked up from other headers and a couple of .c files.

Create a trivial placeholder  file that just
maps to  to make this patch obviously correct and
bisectable.

Include the new header in the files that are going to need it.

Acked-by: Linus Torvalds 
Cc: Mike Galbraith 
Cc: Peter Zijlstra 
Cc: Thomas Gleixner 
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Ingo Molnar

pty: make ptmx file ops read-only after init

2016-09-15T10:47:03+00:00

The ptmx_fops structure is only changed during init, so mark it as such.

Signed-off-by: Kees Cook 
Signed-off-by: Greg Kroah-Hartman

devpts: fix null pointer dereference on failed memory allocation

2016-06-26T18:39:00+00:00

An ENOMEM when creating a pair tty in tty_ldisc_setup causes a null
pointer dereference in devpts_kill_index because tty->link->driver_data
is NULL.  The oops was triggered with the pty stressor in stress-ng when
in a low memory condition.

tty_init_dev tries to clean up a tty_ldisc_setup ENOMEM error by calling
release_tty, however, this ultimately tries to clean up the NULL pair'd
tty in pty_unix98_remove, triggering the Oops.

Add check to pty_unix98_remove to only clean up fsi if it is not NULL.

Ooops:

[   23.020961] Oops: 0000 [#1] SMP
[   23.020976] Modules linked in: ppdev snd_hda_codec_generic snd_hda_intel snd_hda_codec parport_pc snd_hda_core snd_hwdep parport snd_pcm input_leds joydev snd_timer serio_raw snd soundcore i2c_piix4 mac_hid ib_iser rdma_cm iw_cm ib_cm ib_core configfs iscsi_tcp libiscsi_tcp libiscsi scsi_transport_iscsi autofs4 btrfs raid10 raid456 async_raid6_recov async_memcpy async_pq async_xor async_tx xor raid6_pq libcrc32c raid1 raid0 multipath linear crct10dif_pclmul crc32_pclmul ghash_clmulni_intel aesni_intel qxl aes_x86_64 ttm lrw gf128mul glue_helper ablk_helper drm_kms_helper cryptd syscopyarea sysfillrect psmouse sysimgblt floppy fb_sys_fops drm pata_acpi jitterentropy_rng drbg ansi_cprng
[   23.020978] CPU: 0 PID: 1452 Comm: stress-ng-pty Not tainted 4.7.0-rc4+ #2
[   23.020978] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS Ubuntu-1.8.2-1ubuntu1 04/01/2014
[   23.020979] task: ffff88007ba30000 ti: ffff880078ea8000 task.ti: ffff880078ea8000
[   23.020981] RIP: 0010:[]  [] ida_remove+0x1f/0x120
[   23.020981] RSP: 0018:ffff880078eabb60  EFLAGS: 00010a03
[   23.020982] RAX: 4444444444444567 RBX: 0000000000000000 RCX: 000000000000001f
[   23.020982] RDX: 000000000000014c RSI: 000000000000026f RDI: 0000000000000000
[   23.020982] RBP: ffff880078eabb70 R08: 0000000000000004 R09: 0000000000000036
[   23.020983] R10: 000000000000026f R11: 0000000000000000 R12: 000000000000026f
[   23.020983] R13: 000000000000026f R14: ffff88007c944b40 R15: 000000000000026f
[   23.020984] FS:  00007f9a2f3cc700(0000) GS:ffff88007fc00000(0000) knlGS:0000000000000000
[   23.020984] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[   23.020985] CR2: 0000000000000010 CR3: 000000006c81b000 CR4: 00000000001406f0
[   23.020988] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[   23.020988] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[   23.020988] Stack:
[   23.020989]  0000000000000000 000000000000026f ffff880078eabb90 ffffffff812a5a99
[   23.020990]  0000000000000000 00000000fffffff4 ffff880078eabba8 ffffffff814f9cbe
[   23.020991]  ffff88007965c800 ffff880078eabbc8 ffffffff814eef43 fffffffffffffff4
[   23.020991] Call Trace:
[   23.021000]  [] devpts_kill_index+0x29/0x50
[   23.021002]  [] pty_unix98_remove+0x2e/0x50
[   23.021006]  [] release_tty+0xb3/0x1b0
[   23.021007]  [] tty_init_dev+0xd4/0x1c0
[   23.021011]  [] ptmx_open+0xae/0x190
[   23.021013]  [] chrdev_open+0xbf/0x1b0
[   23.021015]  [] do_dentry_open+0x203/0x310
[   23.021016]  [] ? cdev_put+0x30/0x30
[   23.021017]  [] vfs_open+0x54/0x80
[   23.021018]  [] ? may_open+0x8c/0x100
[   23.021019]  [] path_openat+0x2eb/0x1440
[   23.021020]  [] ? putname+0x54/0x60
[   23.021022]  [] ? n_tty_ioctl_helper+0x27/0x100
[   23.021023]  [] do_filp_open+0x91/0x100
[   23.021024]  [] ? getname_flags+0x56/0x1f0
[   23.021026]  [] ? __alloc_fd+0x46/0x190
[   23.021027]  [] do_sys_open+0x124/0x210
[   23.021028]  [] SyS_open+0x1e/0x20
[   23.021035]  [] entry_SYSCALL_64_fastpath+0x1e/0xa8
[   23.021044] Code: 63 28 45 31 e4 eb dd 0f 1f 44 00 00 55 4c 63 d6 48 ba 89 88 88 88 88 88 88 88 4c 89 d0 b9 1f 00 00 00 48 f7 e2 48 89 e5 41 54 53 <8b> 47 10 48 89 fb 8d 3c c5 00 00 00 00 48 c1 ea 09 b8 01 00 00
[   23.021045] RIP  [] ida_remove+0x1f/0x120
[   23.021045]  RSP 
[   23.021046] CR2: 0000000000000010

Signed-off-by: Colin Ian King 
Cc: stable 
Signed-off-by: Greg Kroah-Hartman