* Re: [PATCH] proc: allow killing processes via file descriptors
From: Christian Brauner @ 2018-11-18 21:23 UTC (permalink / raw)
To: Daniel Colascione
Cc: Andy Lutomirski, Aleksa Sarai, Andy Lutomirski, Randy Dunlap,
Eric W. Biederman, LKML, Serge E. Hallyn, Jann Horn,
Andrew Morton, Oleg Nesterov, Al Viro, Linux FS Devel, Linux API,
Tim Murray, Kees Cook, Jan Engelhardt
In-Reply-To: <CAKOZueuWzu2GuJ-w3yb01P9uyO1WniKG+i=BUsweVdA-KgEjhw@mail.gmail.com>
On Sun, Nov 18, 2018 at 12:54:10PM -0800, Daniel Colascione wrote:
> On Sun, Nov 18, 2018 at 12:43 PM, Christian Brauner
> <christian@brauner.io> wrote:
> > On Sun, Nov 18, 2018 at 01:28:41PM -0700, Andy Lutomirski wrote:
> >>
> >>
> >> > On Nov 18, 2018, at 12:44 PM, Daniel Colascione <dancol@google.com> wrote:
> >> >
> >>
> >> >
> >> > That is, I'm proposing an API that looks like this:
> >> >
> >> > int process_kill(int procfs_dfd, int signo, const union sigval value)
> >> >
> >> > If, later, process_kill were to *also* accept process-capability FDs,
> >> > nothing would break.
> >>
> >> Except that this makes it ambiguous to the caller as to whether their current creds are considered. So it would need to be a different syscall or at least a flag. Otherwise a lot of those nice theoretical properties go away.
> >
> > I can add a flag argument
> > int process_signal(int procfs_dfd, int signo, siginfo_t *info, int flags)
> > The way I see it process_signal() should be equivalent to kill(pid, signal) for now.
> > That is siginfo_t is cleared and set to:
> >
> > info.si_signo = sig;
> > info.si_errno = 0;
> > info.si_code = SI_USER;
> > info.si_pid = task_tgid_vnr(current);
> > info.si_uid = from_kuid_munged(current_user_ns(), current_uid());
>
> That makes sense. I just don't want to get into a situation where
> callers feel that they *have* to use the PID-based APIs to send a
> signal because process_kill doesn't offer some bit of functionality.
Yeah.
>
> Are you imagining something like requiring info t be NULL unless flags
> contains some "I have a siginfo_t" value?
Well, I was actually thinking about something like:
/**
* sys_process_signal - send a signal to a process trough a process file descriptor
* @fd: the file descriptor of the process
* @sig: signal to be sent
* @info: the signal info
* @flags: future flags to be passed
*/
SYSCALL_DEFINE4(process_signal, int, fd, int, sig, siginfo_t __user *, info,
int, flags)
{
struct pid *pid;
struct fd *f;
kernel_siginfo_t kinfo;
/* Do not allow users to pass garbage. */
if (flags)
return -EINVAL;
int ret = __copy_siginfo_from_user(sig, &kinfo, info);
if (unlikely(ret))
return ret;
/* For now, enforce that caller's creds are used. */
kinfo.si_pid = task_tgid_vnr(current);
kinfo.si_uid = from_kuid_munged(current_user_ns(), current_uid());
if (signal_impersonates_kernel(kinfo))
return -EPERM;
f = fdget(fd);
if (!f.file)
return -EBADF;
pid = f.file->private_data;
if (!pid)
return -EBADF;
return kill_pid_info(sig, kinfo, pid);
}
>
> BTW: passing SI_USER to rt_sigqueueinfo *should* as long as the
> passed-in si_pid and si_uid match what the kernel would set them to in
> the kill(2) case. The whole point of SI_USER is that the recipient
> knows that it can trust the origin information embedded in the
> siginfo_t in the signal handler. If the kernel verifies that a signal
> sender isn't actually lying, why not let people send SI_USER with
> rt_sigqueueinfo?
^ permalink raw reply
* Re: [PATCH] proc: allow killing processes via file descriptors
From: Christian Brauner @ 2018-11-18 21:30 UTC (permalink / raw)
To: Daniel Colascione
Cc: Andy Lutomirski, Aleksa Sarai, Andy Lutomirski, Randy Dunlap,
Eric W. Biederman, LKML, Serge E. Hallyn, Jann Horn,
Andrew Morton, Oleg Nesterov, Al Viro, Linux FS Devel, Linux API,
Tim Murray, Kees Cook, Jan Engelhardt
In-Reply-To: <20181118212336.53hh3qbjughrtc2l@brauner.io>
On Sun, Nov 18, 2018 at 10:23:36PM +0100, Christian Brauner wrote:
> On Sun, Nov 18, 2018 at 12:54:10PM -0800, Daniel Colascione wrote:
> > On Sun, Nov 18, 2018 at 12:43 PM, Christian Brauner
> > <christian@brauner.io> wrote:
> > > On Sun, Nov 18, 2018 at 01:28:41PM -0700, Andy Lutomirski wrote:
> > >>
> > >>
> > >> > On Nov 18, 2018, at 12:44 PM, Daniel Colascione <dancol@google.com> wrote:
> > >> >
> > >>
> > >> >
> > >> > That is, I'm proposing an API that looks like this:
> > >> >
> > >> > int process_kill(int procfs_dfd, int signo, const union sigval value)
> > >> >
> > >> > If, later, process_kill were to *also* accept process-capability FDs,
> > >> > nothing would break.
> > >>
> > >> Except that this makes it ambiguous to the caller as to whether their current creds are considered. So it would need to be a different syscall or at least a flag. Otherwise a lot of those nice theoretical properties go away.
> > >
> > > I can add a flag argument
> > > int process_signal(int procfs_dfd, int signo, siginfo_t *info, int flags)
> > > The way I see it process_signal() should be equivalent to kill(pid, signal) for now.
> > > That is siginfo_t is cleared and set to:
> > >
> > > info.si_signo = sig;
> > > info.si_errno = 0;
> > > info.si_code = SI_USER;
> > > info.si_pid = task_tgid_vnr(current);
> > > info.si_uid = from_kuid_munged(current_user_ns(), current_uid());
> >
> > That makes sense. I just don't want to get into a situation where
> > callers feel that they *have* to use the PID-based APIs to send a
> > signal because process_kill doesn't offer some bit of functionality.
>
> Yeah.
>
> >
> > Are you imagining something like requiring info t be NULL unless flags
> > contains some "I have a siginfo_t" value?
>
> Well, I was actually thinking about something like:
>
> /**
> * sys_process_signal - send a signal to a process trough a process file descriptor
> * @fd: the file descriptor of the process
> * @sig: signal to be sent
> * @info: the signal info
> * @flags: future flags to be passed
> */
> SYSCALL_DEFINE4(process_signal, int, fd, int, sig, siginfo_t __user *, info,
> int, flags)
> {
> struct pid *pid;
> struct fd *f;
> kernel_siginfo_t kinfo;
>
> /* Do not allow users to pass garbage. */
> if (flags)
> return -EINVAL;
>
> int ret = __copy_siginfo_from_user(sig, &kinfo, info);
> if (unlikely(ret))
> return ret;
>
> /* For now, enforce that caller's creds are used. */
> kinfo.si_pid = task_tgid_vnr(current);
> kinfo.si_uid = from_kuid_munged(current_user_ns(), current_uid());
>
> if (signal_impersonates_kernel(kinfo))
> return -EPERM;
>
> f = fdget(fd);
> if (!f.file)
> return -EBADF;
>
> pid = f.file->private_data;
> if (!pid)
> return -EBADF;
>
> return kill_pid_info(sig, kinfo, pid);
> }
Just jotted this down here briefly. This will need an fput and so on
obvs.
>
> >
> > BTW: passing SI_USER to rt_sigqueueinfo *should* as long as the
> > passed-in si_pid and si_uid match what the kernel would set them to in
> > the kill(2) case. The whole point of SI_USER is that the recipient
> > knows that it can trust the origin information embedded in the
> > siginfo_t in the signal handler. If the kernel verifies that a signal
> > sender isn't actually lying, why not let people send SI_USER with
> > rt_sigqueueinfo?
^ permalink raw reply
* Re: [PATCH] proc: allow killing processes via file descriptors
From: Aleksa Sarai @ 2018-11-19 0:08 UTC (permalink / raw)
To: Daniel Colascione
Cc: Christian Brauner, Andy Lutomirski, Eric W. Biederman, LKML,
Serge E. Hallyn, Jann Horn, Andrew Morton, Oleg Nesterov, Al Viro,
Linux FS Devel, Linux API, Tim Murray, Kees Cook, David Howells
In-Reply-To: <CAKOZueto1CmUSWFKjOkazcDcZH6CiXXCr4U=ghewLh8GjE2-=A@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 5573 bytes --]
On 2018-11-18, Daniel Colascione <dancol@google.com> wrote:
> > The gist is to have file descriptors for processes which is obviously not a new
> > idea. This has been done before in other OSes and it has been tried before in
> > Linux [2], [3] (Thanks to Kees for pointing out these patches.). So I want to
> > make it very clear that I'm not laying claim to this being my or even a novel
> > idea in any way. However, I want to diverge from previous approaches with my
> > suggestion. (Though I can't be sure that there's not something similar in other
> > OSes already.)
>
> Windows works basically as you describe. You can create a process is
> suspended state, configure it however you want, then let it run.
> CreateProcess (and even moreso, NtCreateProcess) also provide a rich
> (and *extensible*) interface for pre-creation process configuration.
>
> >> One of the main motivations for having procfds is to have a race-free way of
> > configuring, starting, polling, and killing a process. Basically, a process
> > lifecycle api if you want to think about it that way. The api should also be
> > easily extendable in the future to avoid running into the limitations we
> > currently see with the clone*() syscall(s) again.
> >
> > One of the crucial points of the api is to *separate the configuration
> > of a process through a procfd from actually creating the process*.
> > This is a crucial property expressed in the open*() system calls. First, get a
> > stable handle on an object then allow for ways to configure it. As such the
> > procfd api shares the same insight with Al's and David's new mount api.
> > (Fwiw, Andy also pointed out similarities with posix_spawn().)
> > What I envisioned was to have the following syscalls (multiple name suggestions):
> >
> > 1. int process_open / proc_open / procopen
> > 2. int process_config / proc_config / procconfig or ioctl()-based
> > 3. int process_info / proc_info / procinfo or ioctl()-based
> > 4. int process_manage / proc_manage / procmanage or ioctl()-based
>
> The API you've proposed seems fine to me, although I'd either 1)
> consolidate operations further into one system call, or 2) separate
> the different management operations into more and different system
> calls that can be audited independently. The grouping you've proposed
> seems to have the worst aspects of API splitting and API multiplexing.
> But I have no objection to it in spirit.
I think combining it all into one API is going to be a soft re-invention
of ioctls, but specifically for procfds. This would be an improvement
over just ioctls (since the current ioctl namespacing is based on
well-behaved drivers and hoping we never have more than 256 ioctl
drivers), but I'm not sure it would help make the API nicer than having
separate syscalls (we'd have to do something similar to bpf(2) which I'm
not a huge fan of).
> That said, while I do want to fix process configuration and startup
> generally, I want to fix specific holes in the existing API surface
> first. The two patches I've already sent do that, and this work
> shouldn't wait on an ideal larger process-API overhaul that may or may
> not arrive. Based on previous history, I suspect that an API of the
> scope you're proposing would take years to overcome all LKML
> objections and land. I don't want to wait that long when we can make
> smaller fixes that would not conflict with the general architecture.
I believe this is precisely what Christian is trying to do with this
patch (and you say as much later in your mail).
I think that adding all of {sighand,sighand_exitcode,kill,...} would not
help the path of landing a much larger API change. We should instead
think about the API we want at the end of the day, and then land smaller
changes which are long-term compatible (and won't just become deprecated
APIs -- there's far too many of them, let's not add more needlessly).
If the plan is to have an ioctl API we should merge minor ioctls first.
If the idea is to have an explosion of syscalls, then we should merge
minor syscalls first. We shouldn't merge procfiles if the end goal is to
not use them.
> Next, I want to merge my exithand proposal, or something like it. It's
> likewise a simple change that, in a minimal way, addresses a
> longstanding API deficiency. I'm very strongly against the
> POLLERR-on-directory variant of the idea.
I agree with you on this need. I will admit I do somewhat like the EOF
solution (mainly because it seamlessly deals with the multi-reader case)
but I'm still not sure I like /proc/$pid/exit_sighand. As mentioned in
the other discussion, ideally we would be only ever operating with the
An ugly strawman of an alternative would be an interface that gave you
an fd that you could similarly wait-until-EOF on, but that's probably
not a major API improvement unless we also make said API provide exit
status information in a way that works with the
multiple-readers-with-different-creds usecase.
One other thing I think we should eventually consider is to provide an
API which pings a listener whenever a process does an execve() (and
possibly fork()). This is something you can get from FreeBSD's kqueue --
and is something that we have in a really neutered form in the "proc
connector". But of course we can discuss this separately, especially if
we have an extensible API idea in mind when we start.
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] proc: allow killing processes via file descriptors
From: Aleksa Sarai @ 2018-11-19 0:09 UTC (permalink / raw)
To: Daniel Colascione
Cc: Andy Lutomirski, Randy Dunlap, Christian Brauner,
Eric W. Biederman, LKML, Serge E. Hallyn, Jann Horn,
Andrew Morton, Oleg Nesterov, Al Viro, Linux FS Devel, Linux API,
Tim Murray, Kees Cook, Jan Engelhardt
In-Reply-To: <CAKOZuetfqvn1uVqKJ=16iEzG4g449YOjC_tLM60eKBSkv9u+bQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 9375 bytes --]
On 2018-11-18, Daniel Colascione <dancol@google.com> wrote:
> On Sun, Nov 18, 2018 at 11:05 AM, Aleksa Sarai <cyphar@cyphar.com> wrote:
> > On 2018-11-18, Daniel Colascione <dancol@google.com> wrote:
> >> > Here's my point: if we're really going to make a new API to manipulate
> >> > processes by their fd, I think we should have at least a decent idea
> >> > of how that API will get extended in the future. Right now, we have
> >> > an extremely awkward situation where opening an fd in /proc requires
> >> > certain capabilities or uids, and using those fds often also checks
> >> > current's capabilities, and the target process may have changed its
> >> > own security context, including gaining privilege via SUID, SGID, or
> >> > LSM transition rules in the mean time. This has been a huge source of
> >> > security bugs. It would be nice to have a model for future APIs that
> >> > avoids these problems.
> >> >
> >> > And I didn't say in my proposal that a process's identity should
> >> > fundamentally change when it calls execve(). I'm suggesting that
> >> > certain operations that could cause a process to gain privilege or
> >> > otherwise require greater permission to introspect (mainly execve)
> >> > could be handled by invalidating the new process management fds.
> >> > Sure, if init re-execs itself, it's still PID 1, but that doesn't
> >> > necessarily mean that:
> >> >
> >> > fd = process_open_management_fd(1);
> >> > [init reexecs]
> >> > process_do_something(fd);
> >> >
> >> > needs to work.
> >>
> >> PID 1 is a bad example here, because it doesn't get recycled. Other
> >> PIDs do. The snippet you gave *does* need to work, in general, because
> >> if exec invalidates the handle, and you need to reopen by PID to
> >> re-establish your right to do something with the process, that process
> >> may in fact have died between the invalidation and your reopen, and
> >> your reopened FD may refer to some other random process.
> >
> > I imagine the error would be -EPERM rather than -ESRCH in this case,
> > which would be incredibly trivial for userspace to differentiate
> > between.
>
> Why would userspace necessarily see EPERM? The PID might get recycled
> into a different random process that the caller has the ability to
> affect.
I'm not sure what you're talking about. execve() doesn't change the PID
of a process, and in the case we are talking about:
pidX_handle = open_pid_handle(pidX);
[ pidX execs a setuid binary ]
do_something(pidX_handle);
pidX still has the same PID (so PID recycling is irrelevant in this
case). The key point is whether do_something() should give you an error
in such a state transition, and in that case I would say you'd get
-EPERM which would indicate (obviously) insufficient privileges.
If the PID has died you'd get -ESRCH. Even if it was eventually
recycled. Because you've pinned a 'struct pid'.
> > If you wish to re-open the path that is also trivial by
> > re-opening through /proc/self/fd/$fd -- which will re-do any permission
> > checks and will guarantee that you are re-opening the same 'struct file'
> > and thus the same 'struct pid'.
>
> When you reopen via /proc/self/fd, you get a new struct file
> referencing the existing inode, not the same struct file. A new
> reference to the old struct file would just be dup.
I don't think this is really relevant to what I'm trying to say...
> Anyway: what other API requires, for correct operation, occasional
> reopening through /proc/self/fd? It's cumbersome, and it doesn't add
> anything. If we invalidate process handles on execve, and processes
> are legally allowed to re-exec themselves for arbitrary reasons at any
> time, that's tantamount to saying that handles might become invalid at
> any time and that all callers must be prepared to go through the
> reopen-and-retry path before any operation.
O_PATH. In container runtimes this is necessary for several reasons to
protect against malicious container root filesystems as well as avoiding
exposing a dirfd to the container.
In LXC, O_PATH re-opening is used for /dev/ptmx as well as some other
operations. In runc we use it for FIFO re-opening so that we can signal
pid1 in a container to execve() into user code.
So this isn't a new thing.
> Why are we making them do that? So that a process can have an open FD
> that represents a process-operation capability? Which capability does
> the open FD represent?
The re-opening part was just an argument to show that there isn't a
condition where you wouldn't be able to get access to the 'struct pid'.
I doubt that anyone would actually need to use this -- since you'd need
to pass "/proc/pid/fd/..." to a more privileged process in order to use
the re-opening.
But this also means that we don't need to have a concept of a pidfd that
isn't actually associated with a PID but is instead associated with
current->mm (which is what you appear to be proposing with the whole
"identity fd" concept).
> I think when you and Andy must be talking about is an API that looks like this:
>
> int open_process_operation_handle(int procfs_dirfd, int capability_bitmask)
>
> capability_bitmask would have bits like
>
> PROCESS_CAPABILITY_KILL --- send a signal
> PROCESS_CAPABILITY_PTRACE --- attach to a process
> PROCESS_CAPABILITY_READ_EXIT_STATUS --- what it says on the tin
> PROCESS_CAPABILITY_READ_CMDLINE --- etc.
>
> Then you'd have system calls like
>
> int process_kill(int process_capability_fd, int signo, const union sigval data)
> int process_ptrace_attach(int process_capability_fd)
> int process_wait_for_exit(int process_capability_fd, siginfo_t* exit_info)
>
> that worked on these capability bits. If a process execs or does
> something else to change its security capabilities, operations on
> these capability FDs would fail with ESTALE or something and callers
> would have to re-acquire their capabilities.
>
> This approach works fine. It has some nice theoretical properties, and
> could allow for things like nicer privilege separation for debuggers.
> I wouldn't mind something like this getting into the kernel.
Andy might be arguing for this (and as you said, I can see the benefit
of doing it this way).
I'm not convinced that doing permission checks on-open is necessary here
-- I get Andy's point about write(2) semantics but I think a new set of
proc_* syscalls wouldn't need to follow those semantics. I might be
wrong though.
> I just don't think this model is necessary right now. I want a small
> change from what we have today, one likely to actually make it into
> the tree. And bypassing the capability FDs and just allowing callers
> to operate directly on process *identity* FDs, using privileges in
> effect at the time of all, is at least no worse than what we have now.
>
> That is, I'm proposing an API that looks like this:
>
> int process_kill(int procfs_dfd, int signo, const union sigval value)
>
> If, later, process_kill were to *also* accept process-capability FDs,
> nothing would break.
Again, I think we should agree on whether it's necessary to have both
types of fds before we commit to maintaining both APIs forever...
> >> The only way around this problem is to have two separate FDs --- one
> >> to represent process identity, which *must* be continuous across
> >> execve, and the other to represent some specific capability, some
> >> ability to do something to that process. It's reasonable to invalidate
> >> capability after execve, but it's not reasonable to invalidate
> >> identity. In concrete terms, I don't see a big advantage to this
> >> separation, and I think a single identity FD combined with
> >> per-operation capability checks is sufficient. And much simpler.
> >
> > I think that the error separation above would trivially allow user-space
> > to know whether the identity or capability of a process being monitored
> > has changed.
> >
> > Currently, all operations on a '/proc/$pid' which you've previously
> > opened and has died will give you -ESRCH.
>
> Not the case. Zombies have died, but profs operations work fine on zombies.
It is the case if the process is dead in the sense that the PID might be
re-used. That is what I meant be "dead" here, not semi-dead in the sense
that zombies are.
> >> > Similarly, it seems like
> >> > it's probably safe to be able to open an fd that lets you watch the
> >> > exit status of a process, have the process call setresuid(), and still
> >> > see the exit status.
> >>
> >> Is it? That's an open question.
> >
> > Well, if we consider wait4(2) it seems that this is already the case.
> > If you fork+exec a setuid binary you can definitely see its exit code.
>
> Only if you're the parent. Otherwise, you can't see the process exit
> status unless you pass a ptrace access check and consult
> /proc/pid/stat after the process dies, but before the zombie
> disappears. Random unrelated and unprivileged processes can't see exit
> statuses from distant parts of the system.
Sure, I'd propose that ptrace_may_access() is what we should use for
operation permission checks.
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH] proc: allow killing processes via file descriptors
From: Daniel Colascione @ 2018-11-19 0:31 UTC (permalink / raw)
To: Christian Brauner
Cc: Andy Lutomirski, Aleksa Sarai, Andy Lutomirski, Randy Dunlap,
Eric W. Biederman, LKML, Serge E. Hallyn, Jann Horn,
Andrew Morton, Oleg Nesterov, Al Viro, Linux FS Devel, Linux API,
Tim Murray, Kees Cook, Jan Engelhardt
In-Reply-To: <20181118213021.24asgwkci3do6oby@brauner.io>
On Sun, Nov 18, 2018 at 1:30 PM, Christian Brauner <christian@brauner.io> wrote:
> On Sun, Nov 18, 2018 at 10:23:36PM +0100, Christian Brauner wrote:
>> On Sun, Nov 18, 2018 at 12:54:10PM -0800, Daniel Colascione wrote:
>> > On Sun, Nov 18, 2018 at 12:43 PM, Christian Brauner
>> > <christian@brauner.io> wrote:
>> > > On Sun, Nov 18, 2018 at 01:28:41PM -0700, Andy Lutomirski wrote:
>> > >>
>> > >>
>> > >> > On Nov 18, 2018, at 12:44 PM, Daniel Colascione <dancol@google.com> wrote:
>> > >> >
>> > >>
>> > >> >
>> > >> > That is, I'm proposing an API that looks like this:
>> > >> >
>> > >> > int process_kill(int procfs_dfd, int signo, const union sigval value)
>> > >> >
>> > >> > If, later, process_kill were to *also* accept process-capability FDs,
>> > >> > nothing would break.
>> > >>
>> > >> Except that this makes it ambiguous to the caller as to whether their current creds are considered. So it would need to be a different syscall or at least a flag. Otherwise a lot of those nice theoretical properties go away.
>> > >
>> > > I can add a flag argument
>> > > int process_signal(int procfs_dfd, int signo, siginfo_t *info, int flags)
>> > > The way I see it process_signal() should be equivalent to kill(pid, signal) for now.
>> > > That is siginfo_t is cleared and set to:
>> > >
>> > > info.si_signo = sig;
>> > > info.si_errno = 0;
>> > > info.si_code = SI_USER;
>> > > info.si_pid = task_tgid_vnr(current);
>> > > info.si_uid = from_kuid_munged(current_user_ns(), current_uid());
>> >
>> > That makes sense. I just don't want to get into a situation where
>> > callers feel that they *have* to use the PID-based APIs to send a
>> > signal because process_kill doesn't offer some bit of functionality.
>>
>> Yeah.
>>
>> >
>> > Are you imagining something like requiring info t be NULL unless flags
>> > contains some "I have a siginfo_t" value?
>>
>> Well, I was actually thinking about something like:
>>
>> /**
>> * sys_process_signal - send a signal to a process trough a process file descriptor
>> * @fd: the file descriptor of the process
>> * @sig: signal to be sent
>> * @info: the signal info
>> * @flags: future flags to be passed
>> */
>> SYSCALL_DEFINE4(process_signal, int, fd, int, sig, siginfo_t __user *, info,
>> int, flags)
>> {
>> struct pid *pid;
>> struct fd *f;
>> kernel_siginfo_t kinfo;
>>
>> /* Do not allow users to pass garbage. */
>> if (flags)
>> return -EINVAL;
>>
>> int ret = __copy_siginfo_from_user(sig, &kinfo, info);
>> if (unlikely(ret))
>> return ret;
>>
>> /* For now, enforce that caller's creds are used. */
>> kinfo.si_pid = task_tgid_vnr(current);
>> kinfo.si_uid = from_kuid_munged(current_user_ns(), current_uid());
How about doing it this way? If info is NULL, act like kill(2);
otherwise, act like rt_sigqueueinfo(2).
(Not actual working or compiled code.)
SYSCALL_DEFINE4(process_signal, int, fd, int, sig, siginfo_t __user *, info,
int, flags)
{
struct fd f = { 0 };
kernel_siginfo_t kinfo;
int ret;
/* Make API extension possible. */
ret = -EINVAL;
if (flags)
goto out;
ret = -EBADF;
f = fdget(fd);
if (!f.file)
goto out;
ret = mumble_mumble_check_real_proc_file(f.file);
if (ret)
goto out;
/* Act like kill(2) or rt_sigqueueinfo(2) depending on whether
* the user gave us a siginfo structure.
*/
if (info) {
ret = __copy_siginfo_from_user(sig, &kinfo, info);
if (ret)
goto out;
/* Combine this logic with rt_sigqueueinfo(2) */
ret = -EPERM;
if ((info->si_code >= 0 || info->si_code == SI_TKILL) &&
(task_pid_vnr(current) != pid))
goto out;
} else {
/* Combine this logic with kill(2) */
clear_siginfo(&kinfo);
kinfo.si_signo = sig;
kinfo.si_errno = 0;
kinfo.si_code = SI_USER;
kinfo.si_pid = task_tgid_vnr(current);
kinfo.si_uid = from_kuid_munged(current_user_ns(),
current_uid());
}
ret = kill_pid_info(sig, &kinfo, proc_pid(file_inode(f.file)));
out:
if (f.file)
fput(f);
return ret;
}
^ permalink raw reply
* Re: [PATCH] proc: allow killing processes via file descriptors
From: Christian Brauner @ 2018-11-19 0:40 UTC (permalink / raw)
To: Daniel Colascione
Cc: Andy Lutomirski, Aleksa Sarai, Andy Lutomirski, Randy Dunlap,
Eric W. Biederman, LKML, Serge E. Hallyn, Jann Horn,
Andrew Morton, Oleg Nesterov, Al Viro, Linux FS Devel, Linux API,
Tim Murray, Kees Cook, Jan Engelhardt
In-Reply-To: <CAKOZuetT8mzZJR-K_W0VM7Dg1c1SnH8tW-HXxMT77yS0DE453w@mail.gmail.com>
On Sun, Nov 18, 2018 at 04:31:22PM -0800, Daniel Colascione wrote:
> On Sun, Nov 18, 2018 at 1:30 PM, Christian Brauner <christian@brauner.io> wrote:
> > On Sun, Nov 18, 2018 at 10:23:36PM +0100, Christian Brauner wrote:
> >> On Sun, Nov 18, 2018 at 12:54:10PM -0800, Daniel Colascione wrote:
> >> > On Sun, Nov 18, 2018 at 12:43 PM, Christian Brauner
> >> > <christian@brauner.io> wrote:
> >> > > On Sun, Nov 18, 2018 at 01:28:41PM -0700, Andy Lutomirski wrote:
> >> > >>
> >> > >>
> >> > >> > On Nov 18, 2018, at 12:44 PM, Daniel Colascione <dancol@google.com> wrote:
> >> > >> >
> >> > >>
> >> > >> >
> >> > >> > That is, I'm proposing an API that looks like this:
> >> > >> >
> >> > >> > int process_kill(int procfs_dfd, int signo, const union sigval value)
> >> > >> >
> >> > >> > If, later, process_kill were to *also* accept process-capability FDs,
> >> > >> > nothing would break.
> >> > >>
> >> > >> Except that this makes it ambiguous to the caller as to whether their current creds are considered. So it would need to be a different syscall or at least a flag. Otherwise a lot of those nice theoretical properties go away.
> >> > >
> >> > > I can add a flag argument
> >> > > int process_signal(int procfs_dfd, int signo, siginfo_t *info, int flags)
> >> > > The way I see it process_signal() should be equivalent to kill(pid, signal) for now.
> >> > > That is siginfo_t is cleared and set to:
> >> > >
> >> > > info.si_signo = sig;
> >> > > info.si_errno = 0;
> >> > > info.si_code = SI_USER;
> >> > > info.si_pid = task_tgid_vnr(current);
> >> > > info.si_uid = from_kuid_munged(current_user_ns(), current_uid());
> >> >
> >> > That makes sense. I just don't want to get into a situation where
> >> > callers feel that they *have* to use the PID-based APIs to send a
> >> > signal because process_kill doesn't offer some bit of functionality.
> >>
> >> Yeah.
> >>
> >> >
> >> > Are you imagining something like requiring info t be NULL unless flags
> >> > contains some "I have a siginfo_t" value?
> >>
> >> Well, I was actually thinking about something like:
> >>
> >> /**
> >> * sys_process_signal - send a signal to a process trough a process file descriptor
> >> * @fd: the file descriptor of the process
> >> * @sig: signal to be sent
> >> * @info: the signal info
> >> * @flags: future flags to be passed
> >> */
> >> SYSCALL_DEFINE4(process_signal, int, fd, int, sig, siginfo_t __user *, info,
> >> int, flags)
> >> {
> >> struct pid *pid;
> >> struct fd *f;
> >> kernel_siginfo_t kinfo;
> >>
> >> /* Do not allow users to pass garbage. */
> >> if (flags)
> >> return -EINVAL;
> >>
> >> int ret = __copy_siginfo_from_user(sig, &kinfo, info);
> >> if (unlikely(ret))
> >> return ret;
> >>
> >> /* For now, enforce that caller's creds are used. */
> >> kinfo.si_pid = task_tgid_vnr(current);
> >> kinfo.si_uid = from_kuid_munged(current_user_ns(), current_uid());
>
> How about doing it this way? If info is NULL, act like kill(2);
> otherwise, act like rt_sigqueueinfo(2).
>
> (Not actual working or compiled code.)
>
> SYSCALL_DEFINE4(process_signal, int, fd, int, sig, siginfo_t __user *, info,
> int, flags)
> {
> struct fd f = { 0 };
> kernel_siginfo_t kinfo;
> int ret;
>
> /* Make API extension possible. */
> ret = -EINVAL;
> if (flags)
> goto out;
>
> ret = -EBADF;
> f = fdget(fd);
> if (!f.file)
> goto out;
>
> ret = mumble_mumble_check_real_proc_file(f.file);
> if (ret)
> goto out;
>
> /* Act like kill(2) or rt_sigqueueinfo(2) depending on whether
> * the user gave us a siginfo structure.
> */
> if (info) {
> ret = __copy_siginfo_from_user(sig, &kinfo, info);
> if (ret)
> goto out;
> /* Combine this logic with rt_sigqueueinfo(2) */
> ret = -EPERM;
> if ((info->si_code >= 0 || info->si_code == SI_TKILL) &&
> (task_pid_vnr(current) != pid))
> goto out;
>
> } else {
> /* Combine this logic with kill(2) */
> clear_siginfo(&kinfo);
> kinfo.si_signo = sig;
> kinfo.si_errno = 0;
> kinfo.si_code = SI_USER;
> kinfo.si_pid = task_tgid_vnr(current);
> kinfo.si_uid = from_kuid_munged(current_user_ns(),
> current_uid());
> }
>
> ret = kill_pid_info(sig, &kinfo, proc_pid(file_inode(f.file)));
>
> out:
> if (f.file)
> fput(f);
> return ret;
> }
Right, allowing to ass NULL might make sense. I had:
/**
* sys_process_signal - send a signal to a process trough a process file descriptor
* @fd: the file descriptor of the process
* @sig: signal to be sent
* @info: the signal info
* @flags: future flags to be passed
*/
SYSCALL_DEFINE4(procfd_kill, int, fd, int, sig, siginfo_t __user *, info, int, flags)
{
int ret;
struct pid *pid;
kernel_siginfo_t kinfo;
struct fd f;
/* Do not allow users to pass garbage. */
if (flags)
return -EINVAL;
ret = __copy_siginfo_from_user(sig, &kinfo, info);
if (unlikely(ret))
return ret;
/* For now, enforce that caller's creds are used. */
kinfo.si_pid = task_tgid_vnr(current);
kinfo.si_uid = from_kuid_munged(current_user_ns(), current_uid());
f = fdget_raw(fd);
if (!f.file)
return -EBADF;
ret = -EINVAL;
/* Is this a process file descriptor? */
if (!proc_is_procfd(f.file) || !d_is_dir(f.file->f_path.dentry))
goto err;
pid = f.file->private_data;
if (!pid)
goto err;
ret = -EPERM;
/*
* Not even root can pretend to send signals from the kernel.
* Nor can they impersonate a kill()/tgkill(), which adds source info.
*/
if ((kinfo.si_code >= 0 || kinfo.si_code == SI_TKILL) &&
(task_pid(current) != pid))
goto err;
ret = kill_pid_info(sig, &kinfo, pid);
err:
fdput(f);
return ret;
}
^ permalink raw reply
* Re: [PATCH] proc: allow killing processes via file descriptors
From: Daniel Colascione @ 2018-11-19 0:53 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Andy Lutomirski, Randy Dunlap, Christian Brauner,
Eric W. Biederman, LKML, Serge E. Hallyn, Jann Horn,
Andrew Morton, Oleg Nesterov, Al Viro, Linux FS Devel, Linux API,
Tim Murray, Kees Cook, Jan Engelhardt
In-Reply-To: <20181119000928.h2wp2rn2pnlfysp7@yavin>
On Sun, Nov 18, 2018 at 4:09 PM, Aleksa Sarai <cyphar@cyphar.com> wrote:
> On 2018-11-18, Daniel Colascione <dancol@google.com> wrote:
>> On Sun, Nov 18, 2018 at 11:05 AM, Aleksa Sarai <cyphar@cyphar.com> wrote:
>> > On 2018-11-18, Daniel Colascione <dancol@google.com> wrote:
>> >> > Here's my point: if we're really going to make a new API to manipulate
>> >> > processes by their fd, I think we should have at least a decent idea
>> >> > of how that API will get extended in the future. Right now, we have
>> >> > an extremely awkward situation where opening an fd in /proc requires
>> >> > certain capabilities or uids, and using those fds often also checks
>> >> > current's capabilities, and the target process may have changed its
>> >> > own security context, including gaining privilege via SUID, SGID, or
>> >> > LSM transition rules in the mean time. This has been a huge source of
>> >> > security bugs. It would be nice to have a model for future APIs that
>> >> > avoids these problems.
>> >> >
>> >> > And I didn't say in my proposal that a process's identity should
>> >> > fundamentally change when it calls execve(). I'm suggesting that
>> >> > certain operations that could cause a process to gain privilege or
>> >> > otherwise require greater permission to introspect (mainly execve)
>> >> > could be handled by invalidating the new process management fds.
>> >> > Sure, if init re-execs itself, it's still PID 1, but that doesn't
>> >> > necessarily mean that:
>> >> >
>> >> > fd = process_open_management_fd(1);
>> >> > [init reexecs]
>> >> > process_do_something(fd);
>> >> >
>> >> > needs to work.
>> >>
>> >> PID 1 is a bad example here, because it doesn't get recycled. Other
>> >> PIDs do. The snippet you gave *does* need to work, in general, because
>> >> if exec invalidates the handle, and you need to reopen by PID to
>> >> re-establish your right to do something with the process, that process
>> >> may in fact have died between the invalidation and your reopen, and
>> >> your reopened FD may refer to some other random process.
>> >
>> > I imagine the error would be -EPERM rather than -ESRCH in this case,
>> > which would be incredibly trivial for userspace to differentiate
>> > between.
>>
>> Why would userspace necessarily see EPERM? The PID might get recycled
>> into a different random process that the caller has the ability to
>> affect.
>
> I'm not sure what you're talking about. execve() doesn't change the PID
> of a process, and in the case we are talking about:
>
> pidX_handle = open_pid_handle(pidX);
> [ pidX execs a setuid binary ]
> do_something(pidX_handle);
>
> pidX still has the same PID (so PID recycling is irrelevant in this
> case). The key point is whether do_something() should give you an error
> in such a state transition, and in that case I would say you'd get
> -EPERM which would indicate (obviously) insufficient privileges.
EPERM is the wrong error. All that's happened here is that the process
has execed itself; you may still have permission to operate on the
post-execve process. ESTALE is the right error here.
But yes, there is a PID trace. What do you do after getting ESTALE?
You reopen the handle and retry your operation. How do you open a new
handle? Unless you're using some awful /proc/self/fd/... hack, you
reopen by PID. And at that point, you've introduced a PID race again.
That's why, in my sketch below, I imagined creating the capability
handle from the process-identity handle and not, as in the snippet
above, directly from the PID.
>> Anyway: what other API requires, for correct operation, occasional
>> reopening through /proc/self/fd? It's cumbersome, and it doesn't add
>> anything. If we invalidate process handles on execve, and processes
>> are legally allowed to re-exec themselves for arbitrary reasons at any
>> time, that's tantamount to saying that handles might become invalid at
>> any time and that all callers must be prepared to go through the
>> reopen-and-retry path before any operation.
>
> O_PATH. In container runtimes this is necessary for several reasons to
> protect against malicious container root filesystems as well as avoiding
> exposing a dirfd to the container.
>
> In LXC, O_PATH re-opening is used for /dev/ptmx as well as some other
> operations. In runc we use it for FIFO re-opening so that we can signal
> pid1 in a container to execve() into user code.
>
> So this isn't a new thing.
Yuck. I'd still argue that 1) the reopen trick isn't really intended
as the mainline path for that kernel functionality, and 2) there ought
to be a way to do what you're describing in a cleaner way. I'd
classify this approach as a hack. It's one thing to require a hack in
specialized container initialization code, but it's another to bake it
into a hopefully-common API for something as fundamental as process
management, especially when there's a perfectly good alternative that
doesn't require this hack.
>> Why are we making them do that? So that a process can have an open FD
>> that represents a process-operation capability? Which capability does
>> the open FD represent?
>
> The re-opening part was just an argument to show that there isn't a
> condition where you wouldn't be able to get access to the 'struct pid'.
> I doubt that anyone would actually need to use this -- since you'd need
> to pass "/proc/pid/fd/..." to a more privileged process in order to use
> the re-opening.
>
> But this also means that we don't need to have a concept of a pidfd that
> isn't actually associated with a PID but is instead associated with
> current->mm (which is what you appear to be proposing with the whole
> "identity fd" concept).
Not current->mm; that can be shared with clone. struct signal is the
right long-term identity. It's usually easier to keep the struct pid
around though, which is exactly what a procfs FD is today: just a
lightweight handle to a struct pid.
>> I think when you and Andy must be talking about is an API that looks like this:
>>
>> int open_process_operation_handle(int procfs_dirfd, int capability_bitmask)
>>
>> capability_bitmask would have bits like
>>
>> PROCESS_CAPABILITY_KILL --- send a signal
>> PROCESS_CAPABILITY_PTRACE --- attach to a process
>> PROCESS_CAPABILITY_READ_EXIT_STATUS --- what it says on the tin
>> PROCESS_CAPABILITY_READ_CMDLINE --- etc.
>>
>> Then you'd have system calls like
>>
>> int process_kill(int process_capability_fd, int signo, const union sigval data)
>> int process_ptrace_attach(int process_capability_fd)
>> int process_wait_for_exit(int process_capability_fd, siginfo_t* exit_info)
>>
>> that worked on these capability bits. If a process execs or does
>> something else to change its security capabilities, operations on
>> these capability FDs would fail with ESTALE or something and callers
>> would have to re-acquire their capabilities.
>>
>> This approach works fine. It has some nice theoretical properties, and
>> could allow for things like nicer privilege separation for debuggers.
>> I wouldn't mind something like this getting into the kernel.
>
> Andy might be arguing for this (and as you said, I can see the benefit
> of doing it this way).
>
> I'm not convinced that doing permission checks on-open is necessary here
> -- I get Andy's point about write(2) semantics but I think a new set of
> proc_* syscalls wouldn't need to follow those semantics. I might be
> wrong though.
For now, it's fine to just expose system calls that operate directly
on the procfs dfd.
>> I just don't think this model is necessary right now. I want a small
>> change from what we have today, one likely to actually make it into
>> the tree. And bypassing the capability FDs and just allowing callers
>> to operate directly on process *identity* FDs, using privileges in
>> effect at the time of all, is at least no worse than what we have now.
>>
>> That is, I'm proposing an API that looks like this:
>>
>> int process_kill(int procfs_dfd, int signo, const union sigval value)
>>
>> If, later, process_kill were to *also* accept process-capability FDs,
>> nothing would break.
>
> Again, I think we should agree on whether it's necessary to have both
> types of fds before we commit to maintaining both APIs forever...
I don't think noting that an API *could* be extended in a certain way
in the future creates any obligation to decide, immediately, whether
that extension will ever be needed. Right now, I don't see a reason to
supply the capability FD API I described. I'm just saying that it
could be added in a low-friction way if necessary one day.
>> >> > Similarly, it seems like
>> >> > it's probably safe to be able to open an fd that lets you watch the
>> >> > exit status of a process, have the process call setresuid(), and still
>> >> > see the exit status.
>> >>
>> >> Is it? That's an open question.
>> >
>> > Well, if we consider wait4(2) it seems that this is already the case.
>> > If you fork+exec a setuid binary you can definitely see its exit code.
>>
>> Only if you're the parent. Otherwise, you can't see the process exit
>> status unless you pass a ptrace access check and consult
>> /proc/pid/stat after the process dies, but before the zombie
>> disappears. Random unrelated and unprivileged processes can't see exit
>> statuses from distant parts of the system.
>
> Sure, I'd propose that ptrace_may_access() is what we should use for
> operation permission checks.
The tricky part is that ptrace_may_access takes a struct task. We want
logic that's *like* ptrace_may_access, but that works posthumously.
It's especially tricky because there's an LSM hook that lets
__ptrace_may_access do arbitrary things. And we can't just run that
hook upon process death, since *after* a process dies, a process
holding an exithand FD (or whatever we call it) may pass that FD to
another process, and *that* process can read(2) from it.
Another option is doing the exithand access check at open time. I
think that's probably fine, and it would make things a lot simpler.
But if we use this option, we should understand what we're doing, and
get some security-conscious people to think through the implications.
^ permalink raw reply
* Re: [PATCH] proc: allow killing processes via file descriptors
From: Daniel Colascione @ 2018-11-19 1:14 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Christian Brauner, Andy Lutomirski, Eric W. Biederman, LKML,
Serge E. Hallyn, Jann Horn, Andrew Morton, Oleg Nesterov, Al Viro,
Linux FS Devel, Linux API, Tim Murray, Kees Cook, David Howells
In-Reply-To: <20181119000857.rnkuqdpmcutrwtem@yavin>
On Sun, Nov 18, 2018 at 4:08 PM, Aleksa Sarai <cyphar@cyphar.com> wrote:
> On 2018-11-18, Daniel Colascione <dancol@google.com> wrote:
>> > The gist is to have file descriptors for processes which is obviously not a new
>> > idea. This has been done before in other OSes and it has been tried before in
>> > Linux [2], [3] (Thanks to Kees for pointing out these patches.). So I want to
>> > make it very clear that I'm not laying claim to this being my or even a novel
>> > idea in any way. However, I want to diverge from previous approaches with my
>> > suggestion. (Though I can't be sure that there's not something similar in other
>> > OSes already.)
>>
>> Windows works basically as you describe. You can create a process is
>> suspended state, configure it however you want, then let it run.
>> CreateProcess (and even moreso, NtCreateProcess) also provide a rich
>> (and *extensible*) interface for pre-creation process configuration.
>>
>> >> One of the main motivations for having procfds is to have a race-free way of
>> > configuring, starting, polling, and killing a process. Basically, a process
>> > lifecycle api if you want to think about it that way. The api should also be
>> > easily extendable in the future to avoid running into the limitations we
>> > currently see with the clone*() syscall(s) again.
>> >
>> > One of the crucial points of the api is to *separate the configuration
>> > of a process through a procfd from actually creating the process*.
>> > This is a crucial property expressed in the open*() system calls. First, get a
>> > stable handle on an object then allow for ways to configure it. As such the
>> > procfd api shares the same insight with Al's and David's new mount api.
>> > (Fwiw, Andy also pointed out similarities with posix_spawn().)
>> > What I envisioned was to have the following syscalls (multiple name suggestions):
>> >
>> > 1. int process_open / proc_open / procopen
>> > 2. int process_config / proc_config / procconfig or ioctl()-based
>> > 3. int process_info / proc_info / procinfo or ioctl()-based
>> > 4. int process_manage / proc_manage / procmanage or ioctl()-based
>>
>> The API you've proposed seems fine to me, although I'd either 1)
>> consolidate operations further into one system call, or 2) separate
>> the different management operations into more and different system
>> calls that can be audited independently. The grouping you've proposed
>> seems to have the worst aspects of API splitting and API multiplexing.
>> But I have no objection to it in spirit.
>
> I think combining it all into one API is going to be a soft re-invention
> of ioctls, but specifically for procfds. This would be an improvement
> over just ioctls (since the current ioctl namespacing is based on
> well-behaved drivers and hoping we never have more than 256 ioctl
> drivers), but I'm not sure it would help make the API nicer than having
> separate syscalls (we'd have to do something similar to bpf(2) which I'm
> not a huge fan of).
Right. Multiplexers are nothing new, and I'm not a huge fan of them.
>From an API design perspective, having a bunch of different system
calls is probably best.
(I do wonder what happens to system call cache behavior once the
top-level system call table becomes huge though.)
>> That said, while I do want to fix process configuration and startup
>> generally, I want to fix specific holes in the existing API surface
>> first. The two patches I've already sent do that, and this work
>> shouldn't wait on an ideal larger process-API overhaul that may or may
>> not arrive. Based on previous history, I suspect that an API of the
>> scope you're proposing would take years to overcome all LKML
>> objections and land. I don't want to wait that long when we can make
>> smaller fixes that would not conflict with the general architecture.
>
> I believe this is precisely what Christian is trying to do with this
> patch (and you say as much later in your mail).
>
> I think that adding all of {sighand,sighand_exitcode,kill,...} would not
> help the path of landing a much larger API change. We should instead
> think about the API we want at the end of the day, and then land smaller
> changes which are long-term compatible (and won't just become deprecated
> APIs -- there's far too many of them, let's not add more needlessly).
I don't think we need to reach consensus on some long-term design to
address specific problems that we know today. The changes we're
talking about here *are* long-term compatible with a bigger process
API overhaul. They may or may not be *part* of that solution, but I
don't see them making that solution harder either. And the proposals
so far all seem to go in the right direction.
>> Next, I want to merge my exithand proposal, or something like it. It's
>> likewise a simple change that, in a minimal way, addresses a
>> longstanding API deficiency. I'm very strongly against the
>> POLLERR-on-directory variant of the idea.
>
> I agree with you on this need. I will admit I do somewhat like the EOF
> solution (mainly because it seamlessly deals with the multi-reader case)
> but I'm still not sure I like /proc/$pid/exit_sighand. As mentioned in
> the other discussion, ideally we would be only ever operating with the
This sentence got cut off.
> An ugly strawman of an alternative would be an interface that gave you
> an fd that you could similarly wait-until-EOF on, but that's probably
> not a major API improvement unless we also make said API provide exit
> status information in a way that works with the
> multiple-readers-with-different-creds usecase.
What about something like this then?
#define PROCESS_EXIT_HANDLE_CLOEXEC (1<<0)
#define PROCESS_EXIT_HANDLE_NONBLOCK (1<<1)
#define PROCESS_EXIT_HANDLE_WANT_STATUS (1<<2)
/* Open an "status handle" for the process identified by PROCFS_DFD,
* which must be an open descriptor to a /proc/pid directory. FLAGS is
* a combination of zero or more of the
* PROCESS_EXIT_HANDLE_* constants.
*
* Return -1 on error. On success, return a descriptor for a process
* status handle. Before the process identified by PROCFS_DFD exits,
* reads from the status handle block. After exit, reads from the
* status handle yield either EOF (if PROCESS_EXIT_HANDLE_WANT_STATUS
* is not specified) or a siginfo_t describing how the process exited
* (if PROCESS_EXIT_HANDLE_WANT_STATUS is specified), as from
* waitid(2).
*
* The returned file descriptor also supports poll(2) and other
* notification APIs.
*
* Any process may call process_get_statusfd from any PROCFS_DFD
* without PROCESS_EXIT_HANDLE_WANT_STATUS.
* If PROCESS_EXIT_HANDLE_WANT_STATUS is specified, PROCFS_DFD must
* refer either to the calling process (or one of its threads), a
* child of the current process, or a process for which the current
* process would be able to successfully call ptrace(2).
*
* The PROCESS_EXIT_HANDLE_WANT_STATUS permission check happens only
* at open time, not at read time, and the process handle can be
transferred like any other FD.
*/
int process_get_statusfd(int procfs_dfd, int flags);
> One other thing I think we should eventually consider is to provide an
> API which pings a listener whenever a process does an execve() (and
> possibly fork()).
I thought about providing this facility too, but it's not immediately
apparent to me who would use it. ISTM that most callers that would
want this would be happy grabbing the process with ptrace or passively
monitoring it with ftrace or the process connector.
^ permalink raw reply
* Re: [PATCH] proc: allow killing processes via file descriptors
From: Daniel Colascione @ 2018-11-19 1:16 UTC (permalink / raw)
To: Aleksa Sarai
Cc: Andy Lutomirski, Randy Dunlap, Christian Brauner,
Eric W. Biederman, LKML, Serge E. Hallyn, Jann Horn,
Andrew Morton, Oleg Nesterov, Al Viro, Linux FS Devel, Linux API,
Tim Murray, Kees Cook, Jan Engelhardt
In-Reply-To: <CAKOZueve_r5h9_B2YV5RzJYjTf-yS5uZfAbz+Ftqy5jFSk6Xdw@mail.gmail.com>
On Sun, Nov 18, 2018 at 4:53 PM, Daniel Colascione <dancol@google.com> wrote:
>> Sure, I'd propose that ptrace_may_access() is what we should use for
>> operation permission checks.
>
> The tricky part is that ptrace_may_access takes a struct task. We want
> logic that's *like* ptrace_may_access, but that works posthumously.
> It's especially tricky because there's an LSM hook that lets
> __ptrace_may_access do arbitrary things. And we can't just run that
> hook upon process death, since *after* a process dies, a process
> holding an exithand FD (or whatever we call it) may pass that FD to
> another process, and *that* process can read(2) from it.
>
> Another option is doing the exithand access check at open time. I
> think that's probably fine, and it would make things a lot simpler.
> But if we use this option, we should understand what we're doing, and
> get some security-conscious people to think through the implications.
A ptrace check is also probably too strict. Yama's ptrace_scope
feature will block ptrace between unrelated processes within a single
user context, but applying this restriction to exit code monitoring
seems too severe to me.
^ permalink raw reply
* Re: [PATCH] proc: allow killing processes via file descriptors
From: Andy Lutomirski @ 2018-11-19 1:43 UTC (permalink / raw)
To: Daniel Colascione
Cc: Aleksa Sarai, Andrew Lutomirski, Randy Dunlap, Christian Brauner,
Eric W. Biederman, LKML, Serge E. Hallyn, Jann Horn,
Andrew Morton, Oleg Nesterov, Al Viro, Linux FS Devel, Linux API,
Tim Murray, Kees Cook, Jan Engelhardt
In-Reply-To: <CAKOZuet4uzYjvNznfUvid2RH8kAuxteWWc26vLhJHKSfS6MjAA@mail.gmail.com>
On Sun, Nov 18, 2018 at 12:32 PM Daniel Colascione <dancol@google.com> wrote:
>
> On Sun, Nov 18, 2018 at 12:28 PM, Andy Lutomirski <luto@amacapital.net> wrote:
> >> That is, I'm proposing an API that looks like this:
> >>
> >> int process_kill(int procfs_dfd, int signo, const union sigval value)
> >>
> >> If, later, process_kill were to *also* accept process-capability FDs,
> >> nothing would break.
> >
> > Except that this makes it ambiguous to the caller as to whether their current creds are considered. So it would need to be a different syscall or at least a flag. Otherwise a lot of those nice theoretical properties go away.
>
> Sure. A flag might make for better ergonomics.
>
> >> Yes, that's what I have in mind. A siginfo_t is small enough that we
> >> could just store it as a blob allocated off the procfs inode or
> >> something like that without bothering with a shmfs file. You'd be able
> >> to read(2) the exit status as many times as you wanted.
> >
> > I think that, if the syscall in question is read(2), then it should work *once* per struct file. Otherwise running cat on the file would behave very oddly.
>
> Why? The file pointer would work normally.
Can you explain the exact semantics? If I have an fd where read(2)
returns the same 4-byte value every time read(2) is called, then cat
will just return an infinite sequence of the same value. This is not
a complete disaster, but it's not really a good thing.
>
> > Read and poll have the same problem as write: we can’t check caps in read or poll either.
>
> Why not? Reading /proc/pid/stat does an access check today and
> conditionally replaces the exit status with zero.
And that's probably a bug. It's at least a giant kludge that we shouldn't copy.
Here is the general rule: the basic operations that are expected to
treat file descriptors as capabilities *must* treat file descriptors
as capabilities, at least for new APIs. This includes read(2),
write(2), and poll(2). We should have an exceedingly good reason to
check current's creds, mm, or anything else about current in those
syscalls.
There is a good reason for this: consider what happens if you type:
sudo >/proc/PID/whatever
or
sudo </proc/PID/whatever
^ permalink raw reply
* Re: [PATCH] proc: allow killing processes via file descriptors
From: Al Viro @ 2018-11-19 2:47 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Daniel Colascione, Randy Dunlap, Christian Brauner,
Eric W. Biederman, LKML, Serge E. Hallyn, Jann Horn,
Andrew Morton, Oleg Nesterov, Aleksa Sarai, Linux FS Devel,
Linux API, Tim Murray, Kees Cook, Jan Engelhardt
In-Reply-To: <CALCETrUeNZPfrSYa9vH5Ukrk1Y+Kb9GkZOh6LkqG6Z9NpK5P0w@mail.gmail.com>
On Sun, Nov 18, 2018 at 09:42:35AM -0800, Andy Lutomirski wrote:
> Now here's the kicker: if the "running program" calls execve(), it
> goes away. The fd gets some sort of notification that this happened
Type error, parser failed.
Define "fd", please. If it's a "file descriptor", thank you do playing,
you've lost. That's not going to work. If it's "opened file" (aka
"file description" in horrible POSIXese), who's going to get notifications
and what kind of exclusion are you going to use?
^ permalink raw reply
* Re: [PATCH] proc: allow killing processes via file descriptors
From: Andy Lutomirski @ 2018-11-19 3:01 UTC (permalink / raw)
To: Al Viro
Cc: Andrew Lutomirski, Daniel Colascione, Randy Dunlap,
Christian Brauner, Eric W. Biederman, LKML, Serge E. Hallyn,
Jann Horn, Andrew Morton, Oleg Nesterov, Aleksa Sarai,
Linux FS Devel, Linux API, Tim Murray, Kees Cook, Jan Engelhardt
In-Reply-To: <20181119024704.GK32577@ZenIV.linux.org.uk>
On Sun, Nov 18, 2018 at 6:47 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> On Sun, Nov 18, 2018 at 09:42:35AM -0800, Andy Lutomirski wrote:
>
> > Now here's the kicker: if the "running program" calls execve(), it
> > goes away. The fd gets some sort of notification that this happened
>
> Type error, parser failed.
>
> Define "fd", please. If it's a "file descriptor", thank you do playing,
> you've lost. That's not going to work. If it's "opened file" (aka
> "file description" in horrible POSIXese), who's going to get notifications
> and what kind of exclusion are you going to use?
What I meant was: a program that has one of these fds would be able to
find out that an execve() happened and the program needs to refresh
its access to the target task. This could be as simple as POLLHUP
and, if needed, some syscall indicating exactly why we got POLLHUP
(e.g. execve vs exit).
There would be some sort of indication that a program that holds an fd
pointing at an "opened file" could get -- probably poll() would return
some status indicating that execve() happened and our capability is
gone, and, if needed
^ permalink raw reply
* Re: RFC: userspace exception fixups
From: Jethro Beekman @ 2018-11-19 5:17 UTC (permalink / raw)
To: Jarkko Sakkinen, Andy Lutomirski
Cc: Dave Hansen, Christopherson, Sean J, Florian Weimer, Linux API,
Jann Horn, Linus Torvalds, X86 ML, linux-arch, LKML,
Peter Zijlstra, Rich Felker, nhorman@redhat.com,
npmccallum@redhat.com, Ayoun, Serge, shay.katz-zamir@intel.com,
linux-sgx@vger.kernel.org, Andy Shevchenko, Thomas Gleixner
In-Reply-To: <20181118130203.GA18934@linux.intel.com>
[-- Attachment #1: Type: text/plain, Size: 2346 bytes --]
On 2018-11-18 18:32, Jarkko Sakkinen wrote:
> On Sun, Nov 18, 2018 at 09:15:48AM +0200, Jarkko Sakkinen wrote:
>> On Thu, Nov 01, 2018 at 10:53:40AM -0700, Andy Lutomirski wrote:
>>> Hi all-
>>>
>>> The people working on SGX enablement are grappling with a somewhat
>>> annoying issue: the x86 EENTER instruction is used from user code and
>>> can, as part of its normal-ish operation, raise an exception. It is
>>> also highly likely to be used from a library, and signal handling in
>>> libraries is unpleasant at best.
>>>
>>> There's been some discussion of adding a vDSO entry point to wrap
>>> EENTER and do something sensible with the exceptions, but I'm
>>> wondering if a more general mechanism would be helpful.
>>
>> I haven't really followed all of this discussion because I've been busy
>> working on the patch set but for me all of these approaches look awfully
>> complicated.
>>
>> I'll throw my own suggestion and apologize if this has been already
>> suggested and discarded: return-to-AEP.
>>
>> My idea is to do just a small extension to SGX AEX handling. At the
>> moment hardware will RAX, RBX and RCX with ERESUME parameters. We can
>> fill extend this by filling other three spare registers with exception
>> information.
>>
>> AEP handler can then do whatever it wants to do with this information
>> or just do ERESUME.
>
> A correction here. In practice this will add a requirement to have a bit
> more complicated AEP code (check the regs for exceptions) than before
> and not just bytes for ENCLU.
>
> e.g. AEP handler should be along the lines
>
> 1. #PF (or #UD or) happens. Kernel fills the registers when it cannot
> handle the exception and returns back to user space i.e. to the
> AEP handler.
> 2. Check the registers containing exception information. If they have
> been filled, take whatever actions user space wants to take.
> 3. Otherwise, just ERESUME.
>
> From my point of view this is making the AEP parameter useful. Its
> standard use is just weird (always point to a place just containing
> ENCLU bytes, why the heck it even exists).
I like this solution. Keeps things simple. One question: when an
exception occurs, how does the kernel know whether to set special
registers or send a signal?
--
Jethro Beekman | Fortanix
[-- Attachment #2: S/MIME Cryptographic Signature --]
[-- Type: application/pkcs7-signature, Size: 3990 bytes --]
^ permalink raw reply
* [PATCH v1 0/2] proc: allow signaling processes via file descriptors
From: Christian Brauner @ 2018-11-19 10:32 UTC (permalink / raw)
To: ebiederm, linux-kernel
Cc: serge, jannh, luto, akpm, oleg, cyphar, viro, linux-fsdevel,
linux-api, dancol, timmurray, linux-man, Christian Brauner
Hey,
This little series introduces the ability to signal processes via file
descriptors to eliminate race-conditions caused by pid recycling.
With this patch an open() call on /proc/<pid> will give userspace a
handle to struct pid of the process associated with /proc/<pid>. This
allows to maintain a stable handle on a process.
Discussion has shown that a dedicated syscall is prefered over an ioctl().
Thus, the new syscall procfd_signal() is introduced to solve this
problem. It operates on a process file descriptor. More details are
found in the individual commit messages.
With this series a process can be killed via:
#define _GNU_SOURCE
#include <errno.h>
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <signal.h>
#include <sys/stat.h>
#include <sys/syscall.h>
#include <sys/types.h>
#include <unistd.h>
int main(int argc, char *argv[])
{
int ret;
char buf[1000];
if (argc < 2)
exit(EXIT_FAILURE);
ret = snprintf(buf, sizeof(buf), "/proc/%s", argv[1]);
if (ret < 0)
exit(EXIT_FAILURE);
int fd = open(buf, O_DIRECTORY | O_CLOEXEC);
if (fd < 0) {
printf("%s - Failed to open \"%s\"\n", strerror(errno), buf);
exit(EXIT_FAILURE);
}
ret = syscall(__NR_procfd_signal, fd, SIGKILL, NULL, 0);
if (ret < 0) {
printf("Failed to send SIGKILL \"%s\"\n", strerror(errno));
close(fd);
exit(EXIT_FAILURE);
}
close(fd);
exit(EXIT_SUCCESS);
}
Thanks!
Christian
Christian Brauner (2):
proc: get process file descriptor from /proc/<pid>
signal: add procfd_signal() syscall
procfd_signal.2: document procfd_signal syscall
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
fs/proc/base.c | 23 ++++++++
include/linux/proc_fs.h | 1 +
include/linux/syscalls.h | 2 +
kernel/signal.c | 76 ++++++++++++++++++++++++--
6 files changed, 98 insertions(+), 6 deletions(-)
--
2.19.1
^ permalink raw reply
* [PATCH v1 1/2] proc: get process file descriptor from /proc/<pid>
From: Christian Brauner @ 2018-11-19 10:32 UTC (permalink / raw)
To: ebiederm, linux-kernel
Cc: serge, jannh, luto, akpm, oleg, cyphar, viro, linux-fsdevel,
linux-api, dancol, timmurray, linux-man, Christian Brauner,
Kees Cook
In-Reply-To: <20181119103241.5229-1-christian@brauner.io>
With this patch an open() call on /proc/<pid> will give userspace a handle
to struct pid of the process associated with /proc/<pid>. This allows to
maintain a stable handle on a process.
I have been discussing various approaches extensively during technical
conferences this year culminating in a long argument with Eric at Linux
Plumbers. The general consensus was that having a handle on a process
should be something that is very simple and easy to maintain with the
option of being extensible via a more advanced api if the need arises. I
believe that this patch is the most simple, dumb, and therefore
maintainable solution.
[1]: https://lkml.org/lkml/2018/10/30/118
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Jann Horn <jannh@google.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Andy Lutomirsky <luto@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Aleksa Sarai <cyphar@cyphar.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christian Brauner <christian@brauner.io>
---
Changelog:
v1:
- remove ioctl() to signal processes and replace with a dedicated syscall
in the next patch
---
fs/proc/base.c | 17 +++++++++++++++++
1 file changed, 17 insertions(+)
diff --git a/fs/proc/base.c b/fs/proc/base.c
index ce3465479447..6365a4fea314 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -3032,10 +3032,27 @@ static int proc_tgid_base_readdir(struct file *file, struct dir_context *ctx)
tgid_base_stuff, ARRAY_SIZE(tgid_base_stuff));
}
+static int proc_tgid_open(struct inode *inode, struct file *file)
+{
+ /* grab reference to struct pid and stash the pointer away */
+ file->private_data = get_pid(proc_pid(inode));
+ return 0;
+}
+
+static int proc_tgid_release(struct inode *inode, struct file *file)
+{
+ struct pid *pid = file->private_data;
+ /* drop reference to struct pid */
+ put_pid(pid);
+ return 0;
+}
+
static const struct file_operations proc_tgid_base_operations = {
+ .open = proc_tgid_open,
.read = generic_read_dir,
.iterate_shared = proc_tgid_base_readdir,
.llseek = generic_file_llseek,
+ .release = proc_tgid_release,
};
static struct dentry *proc_tgid_base_lookup(struct inode *dir, struct dentry *dentry, unsigned int flags)
--
2.19.1
^ permalink raw reply related
* [PATCH v1 2/2] signal: add procfd_signal() syscall
From: Christian Brauner @ 2018-11-19 10:32 UTC (permalink / raw)
To: ebiederm, linux-kernel
Cc: serge, jannh, luto, akpm, oleg, cyphar, viro, linux-fsdevel,
linux-api, dancol, timmurray, linux-man, Christian Brauner,
Kees Cook
In-Reply-To: <20181119103241.5229-1-christian@brauner.io>
The kill() syscall operates on process identifiers. After a process has
exited its pid can be reused by another process. If a caller sends a signal
to a reused pid it will end up signaling the wrong process. This issue has
often surfaced and there has been a push [1] to address this problem.
A prior patch has introduced the ability to get a file descriptor
referencing struct pid by opening /proc/<pid>. This guarantees a stable
handle on a process which can be used to send signals to the referenced
process. Discussion has shown that a dedicated syscall is preferable over
ioctl()s. Thus, the new syscall procfd_signal() is introduced to solve
this problem. It operates on a process file descriptor.
The syscall takes an additional siginfo_t and flags argument. If siginfo_t
is NULL then procfd_signal() behaves like kill() if it is not NULL it
behaves like rt_sigqueueinfo.
The flags argument is added to allow for future extensions of this syscall.
It currently needs to be passed as 0.
With this patch a process can be killed via:
#define _GNU_SOURCE
#include <errno.h>
#include <fcntl.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <signal.h>
#include <sys/stat.h>
#include <sys/syscall.h>
#include <sys/types.h>
#include <unistd.h>
int main(int argc, char *argv[])
{
int ret;
char buf[1000];
if (argc < 2)
exit(EXIT_FAILURE);
ret = snprintf(buf, sizeof(buf), "/proc/%s", argv[1]);
if (ret < 0)
exit(EXIT_FAILURE);
int fd = open(buf, O_DIRECTORY | O_CLOEXEC);
if (fd < 0) {
printf("%s - Failed to open \"%s\"\n", strerror(errno), buf);
exit(EXIT_FAILURE);
}
ret = syscall(__NR_procfd_signal, fd, SIGKILL, NULL, 0);
if (ret < 0) {
printf("Failed to send SIGKILL \"%s\"\n", strerror(errno));
close(fd);
exit(EXIT_FAILURE);
}
close(fd);
exit(EXIT_SUCCESS);
}
[1]: https://lkml.org/lkml/2018/11/18/130
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Serge Hallyn <serge@hallyn.com>
Cc: Jann Horn <jannh@google.com>
Cc: Kees Cook <keescook@chromium.org>
Cc: Andy Lutomirsky <luto@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Aleksa Sarai <cyphar@cyphar.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Christian Brauner <christian@brauner.io>
---
Changelog:
v1:
- patch introduced
---
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
fs/proc/base.c | 6 ++
include/linux/proc_fs.h | 1 +
include/linux/syscalls.h | 2 +
kernel/signal.c | 76 ++++++++++++++++++++++++--
6 files changed, 81 insertions(+), 6 deletions(-)
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index 3cf7b533b3d1..e637eab883e9 100644
--- a/arch/x86/entry/syscalls/syscall_32.tbl
+++ b/arch/x86/entry/syscalls/syscall_32.tbl
@@ -398,3 +398,4 @@
384 i386 arch_prctl sys_arch_prctl __ia32_compat_sys_arch_prctl
385 i386 io_pgetevents sys_io_pgetevents __ia32_compat_sys_io_pgetevents
386 i386 rseq sys_rseq __ia32_sys_rseq
+387 i386 procfd_signal sys_procfd_signal __ia32_sys_procfd_signal
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index f0b1709a5ffb..e95f6741ab42 100644
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -343,6 +343,7 @@
332 common statx __x64_sys_statx
333 common io_pgetevents __x64_sys_io_pgetevents
334 common rseq __x64_sys_rseq
+335 common procfd_signal __x64_sys_procfd_signal
#
# x32-specific system call numbers start at 512 to avoid cache impact
diff --git a/fs/proc/base.c b/fs/proc/base.c
index 6365a4fea314..a12c9de92bd0 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -3055,6 +3055,12 @@ static const struct file_operations proc_tgid_base_operations = {
.release = proc_tgid_release,
};
+bool proc_is_procfd(const struct file *file)
+{
+ return d_is_dir(file->f_path.dentry) &&
+ (file->f_op == &proc_tgid_base_operations);
+}
+
static struct dentry *proc_tgid_base_lookup(struct inode *dir, struct dentry *dentry, unsigned int flags)
{
return proc_pident_lookup(dir, dentry,
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index d0e1f1522a78..2d53a47fba34 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -73,6 +73,7 @@ struct proc_dir_entry *proc_create_net_single_write(const char *name, umode_t mo
int (*show)(struct seq_file *, void *),
proc_write_t write,
void *data);
+extern bool proc_is_procfd(const struct file *file);
#else /* CONFIG_PROC_FS */
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 2ac3d13a915b..a5ca8cb84566 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -907,6 +907,8 @@ asmlinkage long sys_statx(int dfd, const char __user *path, unsigned flags,
unsigned mask, struct statx __user *buffer);
asmlinkage long sys_rseq(struct rseq __user *rseq, uint32_t rseq_len,
int flags, uint32_t sig);
+asmlinkage long sys_procfd_signal(int fd, int sig, siginfo_t __user *info,
+ int flags);
/*
* Architecture-specific system calls
diff --git a/kernel/signal.c b/kernel/signal.c
index 9a32bc2088c9..e8a8929de710 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -19,7 +19,9 @@
#include <linux/sched/task.h>
#include <linux/sched/task_stack.h>
#include <linux/sched/cputime.h>
+#include <linux/file.h>
#include <linux/fs.h>
+#include <linux/proc_fs.h>
#include <linux/tty.h>
#include <linux/binfmts.h>
#include <linux/coredump.h>
@@ -3286,6 +3288,16 @@ COMPAT_SYSCALL_DEFINE4(rt_sigtimedwait, compat_sigset_t __user *, uthese,
}
#endif
+static inline void prepare_kill_siginfo(int sig, struct kernel_siginfo *info)
+{
+ clear_siginfo(info);
+ info->si_signo = sig;
+ info->si_errno = 0;
+ info->si_code = SI_USER;
+ info->si_pid = task_tgid_vnr(current);
+ info->si_uid = from_kuid_munged(current_user_ns(), current_uid());
+}
+
/**
* sys_kill - send a signal to a process
* @pid: the PID of the process
@@ -3295,16 +3307,68 @@ SYSCALL_DEFINE2(kill, pid_t, pid, int, sig)
{
struct kernel_siginfo info;
- clear_siginfo(&info);
- info.si_signo = sig;
- info.si_errno = 0;
- info.si_code = SI_USER;
- info.si_pid = task_tgid_vnr(current);
- info.si_uid = from_kuid_munged(current_user_ns(), current_uid());
+ prepare_kill_siginfo(sig, &info);
return kill_something_info(sig, &info, pid);
}
+/**
+ * sys_procfd_signal - send a signal to a process through a process file
+ * descriptor
+ * @fd: the file descriptor of the process
+ * @sig: signal to be sent
+ * @info: the signal info
+ * @flags: future flags to be passed
+ */
+SYSCALL_DEFINE4(procfd_signal, int, fd, int, sig, siginfo_t __user *, info,
+ int, flags)
+{
+ int ret;
+ struct pid *pid;
+ kernel_siginfo_t kinfo;
+ struct fd f;
+
+ /* Enforce flags be set to 0 until we add an extension. */
+ if (flags)
+ return -EINVAL;
+
+ f = fdget_raw(fd);
+ if (!f.file)
+ return -EBADF;
+
+ ret = -EINVAL;
+ /* Is this a process file descriptor? */
+ if (!proc_is_procfd(f.file))
+ goto err;
+
+ pid = f.file->private_data;
+ if (!pid)
+ goto err;
+
+ if (info) {
+ ret = __copy_siginfo_from_user(sig, &kinfo, info);
+ if (unlikely(ret))
+ goto err;
+ /*
+ * Not even root can pretend to send signals from the kernel.
+ * Nor can they impersonate a kill()/tgkill(), which adds
+ * source info.
+ */
+ ret = -EPERM;
+ if ((kinfo.si_code >= 0 || kinfo.si_code == SI_TKILL) &&
+ (task_pid(current) != pid))
+ goto err;
+ } else {
+ prepare_kill_siginfo(sig, &kinfo);
+ }
+
+ ret = kill_pid_info(sig, &kinfo, pid);
+
+err:
+ fdput(f);
+ return ret;
+}
+
static int
do_send_specific(pid_t tgid, pid_t pid, int sig, struct kernel_siginfo *info)
{
--
2.19.1
^ permalink raw reply related
* [PATCH] procfd_signal.2: document procfd_signal syscall
From: Christian Brauner @ 2018-11-19 10:32 UTC (permalink / raw)
To: ebiederm, linux-kernel
Cc: serge, jannh, luto, akpm, oleg, cyphar, viro, linux-fsdevel,
linux-api, dancol, timmurray, linux-man, Christian Brauner
In-Reply-To: <20181119103241.5229-1-christian@brauner.io>
Signed-off-by: Christian Brauner <christian@brauner.io>
---
Changelog:
v1:
- patch introduced
---
man2/procfd_signal.2 | 147 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 147 insertions(+)
create mode 100644 man2/procfd_signal.2
diff --git a/man2/procfd_signal.2 b/man2/procfd_signal.2
new file mode 100644
index 000000000..6af0b74f4
--- /dev/null
+++ b/man2/procfd_signal.2
@@ -0,0 +1,147 @@
+.\" Copyright (C) 2018 Christian Brauner <christian@brauner.io>
+.\"
+.\" %%%LICENSE_START(VERBATIM)
+.\" Permission is granted to make and distribute verbatim copies of this
+.\" manual provided the copyright notice and this permission notice are
+.\" preserved on all copies.
+.\"
+.\" Permission is granted to copy and distribute modified versions of this
+.\" manual under the conditions for verbatim copying, provided that the
+.\" entire resulting derived work is distributed under the terms of a
+.\" permission notice identical to this one.
+.\"
+.\" Since the Linux kernel and libraries are constantly changing, this
+.\" manual page may be incorrect or out-of-date. The author(s) assume no
+.\" responsibility for errors or omissions, or for damages resulting from
+.\" the use of the information contained herein. The author(s) may not
+.\" have taken the same level of care in the production of this manual,
+.\" which is licensed free of charge, as they might when working
+.\" professionally.
+.\"
+.\" Formatted or processed versions of this manual, if unaccompanied by
+.\" the source, must acknowledge the copyright and authors of this work.
+.\" %%%LICENSE_END
+.\"
+.TH PROCFD_SIGNAL 2 2017-09-15 "Linux" "Linux Programmer's Manual"
+.SH NAME
+procfd_signal \- send signal to a process through a process descriptor
+.SH SYNOPSIS
+.nf
+.B #include <sys/types.h>
+.B #include <signal.h>
+.PP
+.BI "int procfd_signal(int " fd ", int " sig ", siginfo_t *" info ", int " flags );
+.fi
+.SH DESCRIPTION
+.BR procfd_signal ()
+sends the signal specified in
+.I sig
+to the process identified by the file descriptor
+.IR fd .
+The permissions required to send a signal are the same as for
+.BR kill (2).
+As with
+.BR kill (2),
+the null signal (0) can be used to check if a process with a given
+PID exists.
+.PP
+The optional
+.I info
+argument specifies the data to accompany the signal.
+This argument is a pointer to a structure of type
+.IR siginfo_t ,
+described in
+.BR sigaction (2)
+(and defined by including
+.IR <sigaction.h> ).
+The caller should set the following fields in this structure:
+.TP
+.I si_code
+This must be one of the
+.B SI_*
+codes in the Linux kernel source file
+.IR include/asm-generic/siginfo.h ,
+with the restriction that the code must be negative
+(i.e., cannot be
+.BR SI_USER ,
+which is used by the kernel to indicate a signal sent by
+.BR kill (2))
+and cannot (since Linux 2.6.39) be
+.BR SI_TKILL
+(which is used by the kernel to indicate a signal sent using
+.\" tkill(2) or
+.BR tgkill (2)).
+.TP
+.I si_pid
+This should be set to a process ID,
+typically the process ID of the sender.
+.TP
+.I si_uid
+This should be set to a user ID,
+typically the real user ID of the sender.
+.TP
+.I si_value
+This field contains the user data to accompany the signal.
+For more information, see the description of the last
+.RI ( "union sigval" )
+argument of
+.BR sigqueue (3).
+.PP
+Internally, the kernel sets the
+.I si_signo
+field to the value specified in
+.IR sig ,
+so that the receiver of the signal can also obtain
+the signal number via that field.
+.PP
+The
+.I flags
+argument is reserved for future extension and must be set to 0.
+.PP
+.SH RETURN VALUE
+On success, this system call returns 0.
+On error, it returns \-1 and
+.I errno
+is set to indicate the error.
+.SH ERRORS
+.TP
+.B EBADF
+.I fd
+is not a valid file descriptor.
+.TP
+.B EINVAL
+An invalid signal was specified.
+.TP
+.B EINVAL
+.I fd
+does not refer to a process.
+.TP
+.B EINVAL
+The flags argument was not 0.
+.TP
+.B EPERM
+The caller does not have permission to send the signal to the target.
+For the required permissions, see
+.BR kill (2).
+Or:
+.I uinfo->si_code
+is invalid.
+.TP
+.B ESRCH
+The process or process group does not exist.
+Note that an existing process might be a zombie,
+a process that has terminated execution, but
+has not yet been
+.BR wait (2)ed
+for.
+.SH CONFORMING TO
+This system call is Linux-specific.
+.SH SEE ALSO
+.BR kill (2),
+.BR sigaction (2),
+.BR sigprocmask (2),
+.BR tgkill (2),
+.BR pthread_sigqueue (3),
+.BR rt_sigqueueinfo (2),
+.BR sigqueue (3),
+.BR signal (7)
--
2.19.1
^ permalink raw reply related
* Re: [PATCH] proc: allow killing processes via file descriptors
From: kbuild test robot @ 2018-11-19 10:56 UTC (permalink / raw)
Cc: kbuild-all, ebiederm, linux-kernel, serge, jannh, luto, akpm,
oleg, cyphar, viro, linux-fsdevel, linux-api, dancol, timmurray,
Christian Brauner, Kees Cook
In-Reply-To: <20181118111751.6142-1-christian@brauner.io>
[-- Attachment #1: Type: text/plain, Size: 1151 bytes --]
Hi Christian,
Thank you for the patch! Perhaps something to improve:
[auto build test WARNING on linus/master]
[also build test WARNING on v4.20-rc3 next-20181116]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Christian-Brauner/proc-allow-killing-processes-via-file-descriptors/20181119-170316
config: riscv-tinyconfig (attached as .config)
compiler: riscv64-linux-gcc (GCC) 8.1.0
reproduce:
wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
GCC_VERSION=8.1.0 make.cross ARCH=riscv
All warnings (new ones prefixed by >>):
./usr/include/linux/v4l2-controls.h:1105: found __[us]{8,16,32,64} type without #include <linux/types.h>
>> ./usr/include/linux/procfd.h:8: found __[us]{8,16,32,64} type without #include <linux/types.h>
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 4416 bytes --]
^ permalink raw reply
* Re: RFC: userspace exception fixups
From: Jarkko Sakkinen @ 2018-11-19 14:05 UTC (permalink / raw)
To: Jethro Beekman
Cc: Andy Lutomirski, Dave Hansen, Christopherson, Sean J,
Florian Weimer, Linux API, Jann Horn, Linus Torvalds, X86 ML,
linux-arch, LKML, Peter Zijlstra, Rich Felker, nhorman@redhat.com,
npmccallum@redhat.com, Ayoun, Serge, shay.katz-zamir@intel.com,
linux-sgx@vger.kernel.org, Andy Shevchenko
In-Reply-To: <ce304dba-0b02-02e2-4de8-212cd3bd1876@fortanix.com>
On Mon, Nov 19, 2018 at 05:17:26AM +0000, Jethro Beekman wrote:
> On 2018-11-18 18:32, Jarkko Sakkinen wrote:
> > On Sun, Nov 18, 2018 at 09:15:48AM +0200, Jarkko Sakkinen wrote:
> > > On Thu, Nov 01, 2018 at 10:53:40AM -0700, Andy Lutomirski wrote:
> > > > Hi all-
> > > >
> > > > The people working on SGX enablement are grappling with a somewhat
> > > > annoying issue: the x86 EENTER instruction is used from user code and
> > > > can, as part of its normal-ish operation, raise an exception. It is
> > > > also highly likely to be used from a library, and signal handling in
> > > > libraries is unpleasant at best.
> > > >
> > > > There's been some discussion of adding a vDSO entry point to wrap
> > > > EENTER and do something sensible with the exceptions, but I'm
> > > > wondering if a more general mechanism would be helpful.
> > >
> > > I haven't really followed all of this discussion because I've been busy
> > > working on the patch set but for me all of these approaches look awfully
> > > complicated.
> > >
> > > I'll throw my own suggestion and apologize if this has been already
> > > suggested and discarded: return-to-AEP.
> > >
> > > My idea is to do just a small extension to SGX AEX handling. At the
> > > moment hardware will RAX, RBX and RCX with ERESUME parameters. We can
> > > fill extend this by filling other three spare registers with exception
> > > information.
> > >
> > > AEP handler can then do whatever it wants to do with this information
> > > or just do ERESUME.
> >
> > A correction here. In practice this will add a requirement to have a bit
> > more complicated AEP code (check the regs for exceptions) than before
> > and not just bytes for ENCLU.
> >
> > e.g. AEP handler should be along the lines
> >
> > 1. #PF (or #UD or) happens. Kernel fills the registers when it cannot
> > handle the exception and returns back to user space i.e. to the
> > AEP handler.
> > 2. Check the registers containing exception information. If they have
> > been filled, take whatever actions user space wants to take.
> > 3. Otherwise, just ERESUME.
> >
> > From my point of view this is making the AEP parameter useful. Its
> > standard use is just weird (always point to a place just containing
> > ENCLU bytes, why the heck it even exists).
>
> I like this solution. Keeps things simple. One question: when an exception
> occurs, how does the kernel know whether to set special registers or send a
> signal?
Yes, and AFAIK people do in many cases people want to do something else
than just direct ERESUME in AEP handler so would neither be a major
bummer for user space. If I remember correctly you have such?
You can check the cases that we have for SIGSEGV (namely EPCM conflict)
from Sean's patch 08/23.
I'm open for expanding the scope. It is the easy part after there is
consensus for the handling mechanism :-)
/Jarkko
^ permalink raw reply
* RE: [PATCH] proc: allow killing processes via file descriptors
From: David Laight @ 2018-11-19 14:15 UTC (permalink / raw)
To: 'Christian Brauner', ebiederm@xmission.com,
linux-kernel@vger.kernel.org
Cc: serge@hallyn.com, jannh@google.com, luto@kernel.org,
akpm@linux-foundation.org, oleg@redhat.com, cyphar@cyphar.com,
viro@zeniv.linux.org.uk, linux-fsdevel@vger.kernel.org,
linux-api@vger.kernel.org, dancol@google.com,
timmurray@google.com, Kees Cook
In-Reply-To: <20181118111751.6142-1-christian@brauner.io>
From: > Christian Brauner
> Sent: 18 November 2018 11:18
>
> With this patch an open() call on /proc/<pid> will give userspace a handle
> to struct pid of the process associated with /proc/<pid>. This allows to
> maintain a stable handle on a process.
My 3c...
You need to add a version of fork() that returns an open fd to
/proc/pid to the parent.
Is it possible to overload fcntl() rather than ioctl() ?
More interestingly what about a 'unique pid' (eg the pid extended to
(say) 128 bits with a use count) that can be safely put into a /var/run/pid
file for a daemon and used later in a 'kill' that will only ever
reference the correct process.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
^ permalink raw reply
* Re: [PATCH 0/3] Fix unsafe BPF_PROG_TEST_RUN interface
From: Lorenz Bauer @ 2018-11-19 14:30 UTC (permalink / raw)
To: ys114321; +Cc: Alexei Starovoitov, Daniel Borkmann, netdev, linux-api
In-Reply-To: <CAH3MdRW1=iUm0qQdNrv9r5Orn3rZ2XBDn-qym5i_cpi7wo6PxA@mail.gmail.com>
On Sun, 18 Nov 2018 at 06:13, Y Song <ys114321@gmail.com> wrote:
>
> There is a slight change of user space behavior for this patch.
> Without this patch, the value bpf_attr.test.data_size_out is output only.
> For example,
> output buffer : out_buf (user allocated size 10)
> data_size_out is a random value (e.g., 1),
>
> The actual data to copy is 5.
>
> In today's implementation, the kernel will copy 5 and set data_size_out is 5.
>
> With this patch, the kernel will copy 1 and set data_size_out is 5.
>
> I am not 100% sure at this time whether we CAN overload data_size_out
> since it MAY break existing applications.
Yes, that's correct. I think that the likelihood of this is low. It would
affect code that uses bpf_attr without zeroing it first. I had a look around,
and I could only find code that would keep working:
* kernel libbpf and descendants (e.g katran)
* github.com/iovisor/gobpf
* github.com/newtools/ebpf
That doesn't really guarantee anything of course.
--
Lorenz Bauer | Systems Engineer
25 Lavington St., London SE1 0NZ
www.cloudflare.com
^ permalink raw reply
* Re: RFC: userspace exception fixups
From: Jarkko Sakkinen @ 2018-11-19 14:59 UTC (permalink / raw)
To: Jethro Beekman
Cc: Andy Lutomirski, Dave Hansen, Christopherson, Sean J,
Florian Weimer, Linux API, Jann Horn, Linus Torvalds, X86 ML,
linux-arch, LKML, Peter Zijlstra, Rich Felker, nhorman@redhat.com,
npmccallum@redhat.com, Ayoun, Serge, shay.katz-zamir@intel.com,
linux-sgx@vger.kernel.org, Andy Shevchenko
In-Reply-To: <20181119140543.GF8755@linux.intel.com>
On Mon, Nov 19, 2018 at 04:05:43PM +0200, Jarkko Sakkinen wrote:
> On Mon, Nov 19, 2018 at 05:17:26AM +0000, Jethro Beekman wrote:
> > On 2018-11-18 18:32, Jarkko Sakkinen wrote:
> > > On Sun, Nov 18, 2018 at 09:15:48AM +0200, Jarkko Sakkinen wrote:
> > > > On Thu, Nov 01, 2018 at 10:53:40AM -0700, Andy Lutomirski wrote:
> > > > > Hi all-
> > > > >
> > > > > The people working on SGX enablement are grappling with a somewhat
> > > > > annoying issue: the x86 EENTER instruction is used from user code and
> > > > > can, as part of its normal-ish operation, raise an exception. It is
> > > > > also highly likely to be used from a library, and signal handling in
> > > > > libraries is unpleasant at best.
> > > > >
> > > > > There's been some discussion of adding a vDSO entry point to wrap
> > > > > EENTER and do something sensible with the exceptions, but I'm
> > > > > wondering if a more general mechanism would be helpful.
> > > >
> > > > I haven't really followed all of this discussion because I've been busy
> > > > working on the patch set but for me all of these approaches look awfully
> > > > complicated.
> > > >
> > > > I'll throw my own suggestion and apologize if this has been already
> > > > suggested and discarded: return-to-AEP.
> > > >
> > > > My idea is to do just a small extension to SGX AEX handling. At the
> > > > moment hardware will RAX, RBX and RCX with ERESUME parameters. We can
> > > > fill extend this by filling other three spare registers with exception
> > > > information.
> > > >
> > > > AEP handler can then do whatever it wants to do with this information
> > > > or just do ERESUME.
> > >
> > > A correction here. In practice this will add a requirement to have a bit
> > > more complicated AEP code (check the regs for exceptions) than before
> > > and not just bytes for ENCLU.
> > >
> > > e.g. AEP handler should be along the lines
> > >
> > > 1. #PF (or #UD or) happens. Kernel fills the registers when it cannot
> > > handle the exception and returns back to user space i.e. to the
> > > AEP handler.
> > > 2. Check the registers containing exception information. If they have
> > > been filled, take whatever actions user space wants to take.
> > > 3. Otherwise, just ERESUME.
> > >
> > > From my point of view this is making the AEP parameter useful. Its
> > > standard use is just weird (always point to a place just containing
> > > ENCLU bytes, why the heck it even exists).
> >
> > I like this solution. Keeps things simple. One question: when an exception
> > occurs, how does the kernel know whether to set special registers or send a
> > signal?
>
> Yes, and AFAIK people do in many cases people want to do something else
> than just direct ERESUME in AEP handler so would neither be a major
> bummer for user space. If I remember correctly you have such?
>
> You can check the cases that we have for SIGSEGV (namely EPCM conflict)
> from Sean's patch 08/23.
>
> I'm open for expanding the scope. It is the easy part after there is
> consensus for the handling mechanism :-)
Not sure if it a good idea or not but maybe even have new ioctl in
addition to the enclave construction ioctls that you use to specify per
enclave what you want to get. SIGSEGV could be the fallback behavior if
you do not "register" to any exceptions.
/Jarkko
^ permalink raw reply
* Re: RFC: userspace exception fixups
From: Andy Lutomirski @ 2018-11-19 15:29 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Andrew Lutomirski, Dave Hansen, Christopherson, Sean J,
Jethro Beekman, Florian Weimer, Linux API, Jann Horn,
Linus Torvalds, X86 ML, linux-arch, LKML, Peter Zijlstra,
Rich Felker, nhorman, npmccallum, Ayoun, Serge, shay.katz-zamir,
linux-sgx, Andy Shevchenko, Thomas Gleixner, Ingo Molnar
In-Reply-To: <20181118071548.GA4795@linux.intel.com>
On Sat, Nov 17, 2018 at 11:16 PM Jarkko Sakkinen
<jarkko.sakkinen@linux.intel.com> wrote:
>
> On Thu, Nov 01, 2018 at 10:53:40AM -0700, Andy Lutomirski wrote:
> > Hi all-
> >
> > The people working on SGX enablement are grappling with a somewhat
> > annoying issue: the x86 EENTER instruction is used from user code and
> > can, as part of its normal-ish operation, raise an exception. It is
> > also highly likely to be used from a library, and signal handling in
> > libraries is unpleasant at best.
> >
> > There's been some discussion of adding a vDSO entry point to wrap
> > EENTER and do something sensible with the exceptions, but I'm
> > wondering if a more general mechanism would be helpful.
>
> I haven't really followed all of this discussion because I've been busy
> working on the patch set but for me all of these approaches look awfully
> complicated.
>
> I'll throw my own suggestion and apologize if this has been already
> suggested and discarded: return-to-AEP.
>
> My idea is to do just a small extension to SGX AEX handling. At the
> moment hardware will RAX, RBX and RCX with ERESUME parameters. We can
> fill extend this by filling other three spare registers with exception
> information.
I have two issues with this approach:
1. The kernel needs some way to know *when* to apply this fixup.
Decoding the instruction stream and doing it to all exceptions that
hit an ENCLU instruction seems like a poor design.
2. It starts exposing what looks like a more generic exception
handling mechanism to userspace, except that it's nonsensical for
anything other than ENCLU.
^ permalink raw reply
* Re: [PATCH v1 1/2] proc: get process file descriptor from /proc/<pid>
From: Andy Lutomirski @ 2018-11-19 15:32 UTC (permalink / raw)
To: Christian Brauner
Cc: Eric W. Biederman, LKML, Serge E. Hallyn, Jann Horn,
Andrew Lutomirski, Andrew Morton, Oleg Nesterov, Aleksa Sarai,
Al Viro, Linux FS Devel, Linux API, Daniel Colascione, Tim Murray,
linux-man, Kees Cook
In-Reply-To: <20181119103241.5229-2-christian@brauner.io>
On Mon, Nov 19, 2018 at 2:33 AM Christian Brauner <christian@brauner.io> wrote:
>
> With this patch an open() call on /proc/<pid> will give userspace a handle
> to struct pid of the process associated with /proc/<pid>. This allows to
> maintain a stable handle on a process.
> I have been discussing various approaches extensively during technical
> conferences this year culminating in a long argument with Eric at Linux
> Plumbers. The general consensus was that having a handle on a process
> should be something that is very simple and easy to maintain with the
> option of being extensible via a more advanced api if the need arises. I
> believe that this patch is the most simple, dumb, and therefore
> maintainable solution.
How does the mechanism you're adding here differ from proc_pid()?
^ permalink raw reply
* Re: [PATCH v1 2/2] signal: add procfd_signal() syscall
From: Andy Lutomirski @ 2018-11-19 15:45 UTC (permalink / raw)
To: Christian Brauner
Cc: Eric W. Biederman, LKML, Serge E. Hallyn, Jann Horn,
Andrew Lutomirski, Andrew Morton, Oleg Nesterov, Aleksa Sarai,
Al Viro, Linux FS Devel, Linux API, Daniel Colascione, Tim Murray,
linux-man, Kees Cook
In-Reply-To: <20181119103241.5229-3-christian@brauner.io>
On Mon, Nov 19, 2018 at 2:33 AM Christian Brauner <christian@brauner.io> wrote:
>
> The kill() syscall operates on process identifiers. After a process has
> exited its pid can be reused by another process. If a caller sends a signal
> to a reused pid it will end up signaling the wrong process. This issue has
> often surfaced and there has been a push [1] to address this problem.
>
> A prior patch has introduced the ability to get a file descriptor
> referencing struct pid by opening /proc/<pid>. This guarantees a stable
> handle on a process which can be used to send signals to the referenced
> process. Discussion has shown that a dedicated syscall is preferable over
> ioctl()s. Thus, the new syscall procfd_signal() is introduced to solve
> this problem. It operates on a process file descriptor.
> The syscall takes an additional siginfo_t and flags argument. If siginfo_t
> is NULL then procfd_signal() behaves like kill() if it is not NULL it
> behaves like rt_sigqueueinfo.
> The flags argument is added to allow for future extensions of this syscall.
> It currently needs to be passed as 0.
A few questions. First: you've made this work on /proc/PID, but
should it also work on /proc/PID/task/TID to send signals to a
specific thread?
> +bool proc_is_procfd(const struct file *file)
> +{
> + return d_is_dir(file->f_path.dentry) &&
> + (file->f_op == &proc_tgid_base_operations);
> +}
Maybe rename to proc_is_tgid_procfd() to leave room for proc_is_tid_procfd()?
> + if (info) {
> + ret = __copy_siginfo_from_user(sig, &kinfo, info);
> + if (unlikely(ret))
> + goto err;
> + /*
> + * Not even root can pretend to send signals from the kernel.
> + * Nor can they impersonate a kill()/tgkill(), which adds
> + * source info.
> + */
> + ret = -EPERM;
> + if ((kinfo.si_code >= 0 || kinfo.si_code == SI_TKILL) &&
> + (task_pid(current) != pid))
> + goto err;
Is the exception for signaling yourself actually useful here?
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox