On Wed, Oct 31, 2018 at 6:17 PM, Tycho Andersen <ty...@tycho.ws> wrote: > On Wed, Oct 31, 2018 at 06:00:49PM +0000, Daniel Colascione wrote: >> On Wed, Oct 31, 2018 at 5:54 PM, Tycho Andersen <ty...@tycho.ws> wrote: >> > Why not just use an ioctl() like Jann suggested instead of this big >> > security check? Then we avoid the whole setuid writer thing entirely, >> >> Don't you think a system call would be better than a new ioctl? > > We already have a kill() system call :)
kill(2) is useless this purpose: it accepts a numeric PID, but we'd need it to accept a process file descriptor instead. It's true that the existing kill(1) binary might be the vehicle for using a hypothetical new system call, but that's a separate matter. >> With either an ioctl or a new system call, though, the shell would >> need a helper program to use the facility, whereas with the existing >> approach, the shell can use the new facility without any additional >> binaries. > > ...and a binary to use it! > > The nice thing about an ioctl is that it avoids this class of attacks > entirely. Let's stop talking about adding an ioctl. Ioctls have problems with namespacing of the request argument; it's not safe, in general, to issue an ioctl against a file descriptor of an unknown type. You don't know how that FD will interpret your request code. The two good options before us are a write(2) interface and a new system call. I think both are defensible. But I don't see a good reason to consider adding an ioctl instead of a system call. >> > and we can pass the fd around if we want to. >> >> You can pass the FD around today --- specifically, you just pass the >> /proc/pid directory FD, not the /proc/pid/kill FD. The /proc/pid >> directory FD acts as a process handle. (It's literally a reference to >> a struct pid.) Anyone who receives one of these process handle FDs and >> who wants to use the corresponding kill file can open the kill fd with >> openat(2). What you can't do is pass the /proc/pid/kill FD to another >> security context and use it, but when would you ever want to do that? > > Perhaps I don't have a good imagination, because it's not clear to me > when I'd ever use this from a shell instead of the kill binary, I'm not against a new system call per se; I'd just prefer a write(2) interface if we can get away with it. The trouble with a system call is that it would have to accept a /proc/pid file descriptor, and file descriptor management in the shell is awkward. I imagine the interface would look something like kill -f PATH, where PATH is a PATH to a /proc/pid directory. You'd be able to cd into /proc/$SOMETHING, inspect state, and then, if you wanted to kill the process, you'd run kill -f . 9 (or whatever signal number you want). It seems to be about as ergonomic as 'echo 9 > ./kill'. But a new system call means new kernel headers, coordination with procps, and bash, and every other shell that has a kill builtin. You could provide a different, non-kill binary, but then who'd distribute it? A new proc file, OTOH, would Just Work. I agree that a system call interface would avoid the need for the security check thing in the patch, but is avoiding this check worth the coordination flowing from adding a new system call? I don't know. All of this is moot if the new comprehensive process interface that comes out of LPC ends up being better anyway. > either. Using this from the shell is still racy, because if I do > something like: > > echo 9 > /proc/$pid/kill > > There's exactly the same race that there is with kill, that $pid might > be something else. > Of course I could do some magic with bind mounts or > my pwd or something to keep it alive, but I can already do that today > with kill. You can't do it today with kill. The idea that keeping a open file descriptor to a /proc/pid or a file within it prevents PID reuse is widespread, but incorrect.