Re: For review: seccomp_user_notif(2) manual page [v2]

Tycho Andersen
2020-10-29T02:04:51Z

On Thu, Oct 29, 2020 at 02:42:58AM +0100, Jann Horn wrote:
> On Mon, Oct 26, 2020 at 10:55 AM Michael Kerrisk (man-pages)
> <mtk.manpages@gmail.com> wrote:
> > static bool
> > getTargetPathname(struct seccomp_notif *req, int notifyFd,
> > char *path, size_t len)
> > {
> > char procMemPath[PATH_MAX];
> >
> > snprintf(procMemPath, sizeof(procMemPath), "/proc/%d/mem", req->pid);
> >
> > int procMemFd = open(procMemPath, O_RDONLY);
> > if (procMemFd == -1)
> > errExit("\tS: open");
> >
> > /* Check that the process whose info we are accessing is still alive.
> > If the SECCOMP_IOCTL_NOTIF_ID_VALID operation (performed
> > in checkNotificationIdIsValid()) succeeds, we know that the
> > /proc/PID/mem file descriptor that we opened corresponds to the
> > process for which we received a notification. If that process
> > subsequently terminates, then read() on that file descriptor
> > will return 0 (EOF). */
> >
> > checkNotificationIdIsValid(notifyFd, req->id);
> >
> > /* Read bytes at the location containing the pathname argument
> > (i.e., the first argument) of the mkdir(2) call */
> >
> > ssize_t nread = pread(procMemFd, path, len, req->data.args[0]);
> > if (nread == -1)
> > errExit("pread");
>
> As discussed at
> <https://lore.kernel.org/r/CAG48ez0m4Y24ZBZCh+Tf4ORMm9_q4n7VOzpGjwGF7_Fe8EQH=Q@mail.gmail.com>,
> we need to re-check checkNotificationIdIsValid() after reading remote
> memory but before using the read value in any way. Otherwise, the
> syscall could in the meantime get interrupted by a signal handler, the
> signal handler could return, and then the function that performed the
> syscall could free() allocations or return (thereby freeing buffers on
> the stack).
>
> In essence, this pread() is (unavoidably) a potential use-after-free
> read; and to make that not have any security impact, we need to check
> whether UAF read occurred before using the read value. This should
> probably be called out elsewhere in the manpage, too...
>
> Now, of course, **reading** is the easy case. The difficult case is if
> we have to **write** to the remote process... because then we can't
> play games like that. If we write data to a freed pointer, we're
> screwed, that's it. (And for somewhat unrelated bonus fun, consider
> that /proc/$pid/mem is originally intended for process debugging,
> including installing breakpoints, and will therefore happily write
> over "readonly" private mappings, such as typical mappings of
> executable code.)
>
> So, uuuuh... I guess if anyone wants to actually write memory back to
> the target process, we'd better come up with some dedicated API for
> that, using an ioctl on the seccomp fd that magically freezes the

By freeze here you mean a killable wait instead of an interruptible
wait, right?

Not that I'm interested in actually doing this, just want to make sure
I understand correctly :)

Tycho