* Re: [PATCH v4] signal: add taskfd_send_signal() syscall
From: Serge E. Hallyn @ 2018-12-07 0:59 UTC (permalink / raw)
To: Daniel Colascione
Cc: Serge E. Hallyn, Christian Brauner, Eric W. Biederman,
linux-kernel, Linux API, Andy Lutomirski, Arnd Bergmann,
Jann Horn, Andrew Morton, Oleg Nesterov, Aleksa Sarai, Al Viro,
Linux FS Devel, Tim Murray, linux-man, Kees Cook, Florian Weimer,
tglx, x86
In-Reply-To: <CAKOZuevgF7ObKcVvu-iuNK3xLMX2Ze_S+adBEGNZCdfXRibh1A@mail.gmail.com>
On Thu, Dec 06, 2018 at 04:34:54PM -0800, Daniel Colascione wrote:
> On Thu, Dec 6, 2018 at 4:31 PM Serge E. Hallyn <serge@hallyn.com> wrote:
> >
> > On Fri, Dec 07, 2018 at 12:17:45AM +0100, Christian Brauner wrote:
> > > On Thu, Dec 06, 2018 at 11:39:48PM +0100, Christian Brauner wrote:
> > > > On Thu, Dec 06, 2018 at 03:46:53PM -0600, Eric W. Biederman wrote:
> > > > > Christian Brauner <christian@brauner.io> writes:
> > > > >
> > > > > >> Your intention is to add the thread case to support pthreads once the
> > > > > >> process case is sorted out. So this is something that needs to be made
> > > > > >> clear. Did I miss how you plan to handle threads?
> > > > > >
> > > > > > Yeah, maybe you missed it in the commit message [2] which is based on a
> > > > > > discussion with Andy [3] and Arnd [4]:
> > > > >
> > > > > Looking at your references I haven't missed it. You are not deciding
> > > > > anything as of yet to keep it simple. Except you are returning
> > > > > EOPNOTSUPP. You are very much intending to do something.
> > > >
> > > > That was clear all along and was pointed at every occassion in the
> > > > threads. I even went through the hazzle to give you all of the
> > > > references when there's lore.kernel.org.
> > > >
> > > > >
> > > > > Decide. Do you use the flags parameter or is the width of the
> > > > > target depending on the flags.
> > >
> > > Ok, let's try to be constructive. I understand the general concern for
> > > the future so let's put a contract into the commit message stating that
> > > the width of the target aka *what is signaled* will be based on a flag
> > > parameter if we ever extend it:
> > >
> > > taskfd_send_signal(fd, SIGSTOP, NULL, TASKFD_PGID);
> > > taskfd_send_signal(fd, SIGSTOP, NULL, TASKFD_TID);
> > >
> > > with the current default being
> > >
> > > taskfd_send_signal(fd, SIGSTOP, NULL, TASKFD_PID);
> > >
> > > This seems to me the cleanest solution as we only use one type of file
> > > descriptor. Can everyone be on board with this? If so I'm going to send
> > > out a new version of the patch.
> > >
> > > Christian
> >
> > I'm on board with this, but I think you need to also clarify what exactly
> > the fd stands for. I think that (a) userspace should not have to care
> > about the struct pid implementation, and so (b) the procfd should stand
> > for all the pids. So when taskfd_send_signal(fd, SIGSTOP, NULL, TASKFD_PGID)
> > becomes implemented, then open(/proc/5) will pin all three pids, as will
> > open(/proc/5/task/6).
>
> This change doesn't "pin" any PID, and it makes no sense to make a
> process FD stand for all its threads. What does that even mean?
Currently the patch relies on the procfd inode saving a copy to the PIDTYPE_PID
pid. I'm not sure offhand, can it go to the PIDTYPE_PGID from that after the
task has died, or not? I didn't think so. If it can then great.
The point is (a) these are details which should not have to bother userspace,
and (b) how to decide who we're sending the signal to (tid/pid/pgid) should
be specified in precisely one way. So either a flag, or comign from the type
of fd that was opened.
-serge
^ permalink raw reply
* Re: [PATCH v4] signal: add taskfd_send_signal() syscall
From: Daniel Colascione @ 2018-12-07 1:39 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: Christian Brauner, Eric W. Biederman, linux-kernel, Linux API,
Andy Lutomirski, Arnd Bergmann, Jann Horn, Andrew Morton,
Oleg Nesterov, Aleksa Sarai, Al Viro, Linux FS Devel, Tim Murray,
linux-man, Kees Cook, Florian Weimer, tglx, x86
In-Reply-To: <20181207005917.GA11302@mail.hallyn.com>
On Thu, Dec 6, 2018 at 4:59 PM Serge E. Hallyn <serge@hallyn.com> wrote:
>
> On Thu, Dec 06, 2018 at 04:34:54PM -0800, Daniel Colascione wrote:
> > On Thu, Dec 6, 2018 at 4:31 PM Serge E. Hallyn <serge@hallyn.com> wrote:
> > >
> > > On Fri, Dec 07, 2018 at 12:17:45AM +0100, Christian Brauner wrote:
> > > > On Thu, Dec 06, 2018 at 11:39:48PM +0100, Christian Brauner wrote:
> > > > > On Thu, Dec 06, 2018 at 03:46:53PM -0600, Eric W. Biederman wrote:
> > > > > > Christian Brauner <christian@brauner.io> writes:
> > > > > >
> > > > > > >> Your intention is to add the thread case to support pthreads once the
> > > > > > >> process case is sorted out. So this is something that needs to be made
> > > > > > >> clear. Did I miss how you plan to handle threads?
> > > > > > >
> > > > > > > Yeah, maybe you missed it in the commit message [2] which is based on a
> > > > > > > discussion with Andy [3] and Arnd [4]:
> > > > > >
> > > > > > Looking at your references I haven't missed it. You are not deciding
> > > > > > anything as of yet to keep it simple. Except you are returning
> > > > > > EOPNOTSUPP. You are very much intending to do something.
> > > > >
> > > > > That was clear all along and was pointed at every occassion in the
> > > > > threads. I even went through the hazzle to give you all of the
> > > > > references when there's lore.kernel.org.
> > > > >
> > > > > >
> > > > > > Decide. Do you use the flags parameter or is the width of the
> > > > > > target depending on the flags.
> > > >
> > > > Ok, let's try to be constructive. I understand the general concern for
> > > > the future so let's put a contract into the commit message stating that
> > > > the width of the target aka *what is signaled* will be based on a flag
> > > > parameter if we ever extend it:
> > > >
> > > > taskfd_send_signal(fd, SIGSTOP, NULL, TASKFD_PGID);
> > > > taskfd_send_signal(fd, SIGSTOP, NULL, TASKFD_TID);
> > > >
> > > > with the current default being
> > > >
> > > > taskfd_send_signal(fd, SIGSTOP, NULL, TASKFD_PID);
> > > >
> > > > This seems to me the cleanest solution as we only use one type of file
> > > > descriptor. Can everyone be on board with this? If so I'm going to send
> > > > out a new version of the patch.
> > > >
> > > > Christian
> > >
> > > I'm on board with this, but I think you need to also clarify what exactly
> > > the fd stands for. I think that (a) userspace should not have to care
> > > about the struct pid implementation, and so (b) the procfd should stand
> > > for all the pids. So when taskfd_send_signal(fd, SIGSTOP, NULL, TASKFD_PGID)
> > > becomes implemented, then open(/proc/5) will pin all three pids, as will
> > > open(/proc/5/task/6).
> >
> > This change doesn't "pin" any PID, and it makes no sense to make a
> > process FD stand for all its threads. What does that even mean?
>
> Currently the patch relies on the procfd inode saving a copy to the PIDTYPE_PID
> pid.
struct pid doesn't have a type field. The interpretation depends on
the caller's use of the struct pid, and in the current path, that's
PIDTYPE_PID. What, specifically, is wrong with the current approach?
> I'm not sure offhand, can it go to the PIDTYPE_PGID from that after the
> task has died, or not? I didn't think so. If it can then great.
You're arguing that something that does, in fact, work, is somehow
broken in some unspecified way. The kill_pid_info lookup works fine.
What, specifically, is wrong with the semantics as implemented?
> The point is (a) these are details which should not have to bother userspace,
These details *don't* bother userspace.
You're raising concerns that are either imaginary or long-since
addressed. Does the patch cause some kind of maintenance burden? No,
it doesn't, not moreso than any other piece of code. Does the
interface have unclear semantics? No, it clearly sends a signal to a
process, just like kill. Does the patch expose kernel implementation
details? No, it doesn't, because the interface is simply not defined
in terms of these details. Do we need to change how signal delivery
works? No, because if it's fine for kill, it's fine for this facility,
and if some future signal cleanup separates the cases more, that
cleanup can change this code as well.
The change is well-documented, simple, extensible, and addresses an
actual problem. Every legitimate technical criticism has now been
addressed. I don't understand where this opposition is coming from,
since the objections refer to nothing that's actually in the patch or
exposed to the user.
> and (b) how to decide who we're sending the signal to (tid/pid/pgid) should
> be specified in precisely one way. So either a flag, or comign from the type
> of fd that was opened.
You can't send signals to a thread with the current patch. There's no
ambiguity in providing zero ways to do something.
^ permalink raw reply
* Re: [PATCH v4] signal: add taskfd_send_signal() syscall
From: Christian Brauner @ 2018-12-07 1:54 UTC (permalink / raw)
To: Daniel Colascione
Cc: Serge E. Hallyn, Eric W. Biederman, linux-kernel, Linux API,
Andy Lutomirski, Arnd Bergmann, Jann Horn, Andrew Morton,
Oleg Nesterov, Aleksa Sarai, Al Viro, Linux FS Devel, Tim Murray,
linux-man, Kees Cook, Florian Weimer, tglx, x86
In-Reply-To: <CAKOZuesqmPvYoibx3MvVj86EMWRRW4My=7n6iiiBJa+MXo-GJA@mail.gmail.com>
On Thu, Dec 06, 2018 at 05:39:18PM -0800, Daniel Colascione wrote:
> On Thu, Dec 6, 2018 at 4:59 PM Serge E. Hallyn <serge@hallyn.com> wrote:
> >
> > On Thu, Dec 06, 2018 at 04:34:54PM -0800, Daniel Colascione wrote:
> > > On Thu, Dec 6, 2018 at 4:31 PM Serge E. Hallyn <serge@hallyn.com> wrote:
> > > >
> > > > On Fri, Dec 07, 2018 at 12:17:45AM +0100, Christian Brauner wrote:
> > > > > On Thu, Dec 06, 2018 at 11:39:48PM +0100, Christian Brauner wrote:
> > > > > > On Thu, Dec 06, 2018 at 03:46:53PM -0600, Eric W. Biederman wrote:
> > > > > > > Christian Brauner <christian@brauner.io> writes:
> > > > > > >
> > > > > > > >> Your intention is to add the thread case to support pthreads once the
> > > > > > > >> process case is sorted out. So this is something that needs to be made
> > > > > > > >> clear. Did I miss how you plan to handle threads?
> > > > > > > >
> > > > > > > > Yeah, maybe you missed it in the commit message [2] which is based on a
> > > > > > > > discussion with Andy [3] and Arnd [4]:
> > > > > > >
> > > > > > > Looking at your references I haven't missed it. You are not deciding
> > > > > > > anything as of yet to keep it simple. Except you are returning
> > > > > > > EOPNOTSUPP. You are very much intending to do something.
> > > > > >
> > > > > > That was clear all along and was pointed at every occassion in the
> > > > > > threads. I even went through the hazzle to give you all of the
> > > > > > references when there's lore.kernel.org.
> > > > > >
> > > > > > >
> > > > > > > Decide. Do you use the flags parameter or is the width of the
> > > > > > > target depending on the flags.
> > > > >
> > > > > Ok, let's try to be constructive. I understand the general concern for
> > > > > the future so let's put a contract into the commit message stating that
> > > > > the width of the target aka *what is signaled* will be based on a flag
> > > > > parameter if we ever extend it:
> > > > >
> > > > > taskfd_send_signal(fd, SIGSTOP, NULL, TASKFD_PGID);
> > > > > taskfd_send_signal(fd, SIGSTOP, NULL, TASKFD_TID);
> > > > >
> > > > > with the current default being
> > > > >
> > > > > taskfd_send_signal(fd, SIGSTOP, NULL, TASKFD_PID);
> > > > >
> > > > > This seems to me the cleanest solution as we only use one type of file
> > > > > descriptor. Can everyone be on board with this? If so I'm going to send
> > > > > out a new version of the patch.
> > > > >
> > > > > Christian
> > > >
> > > > I'm on board with this, but I think you need to also clarify what exactly
> > > > the fd stands for. I think that (a) userspace should not have to care
> > > > about the struct pid implementation, and so (b) the procfd should stand
> > > > for all the pids. So when taskfd_send_signal(fd, SIGSTOP, NULL, TASKFD_PGID)
> > > > becomes implemented, then open(/proc/5) will pin all three pids, as will
> > > > open(/proc/5/task/6).
> > >
> > > This change doesn't "pin" any PID, and it makes no sense to make a
> > > process FD stand for all its threads. What does that even mean?
> >
> > Currently the patch relies on the procfd inode saving a copy to the PIDTYPE_PID
> > pid.
>
> struct pid doesn't have a type field. The interpretation depends on
> the caller's use of the struct pid, and in the current path, that's
> PIDTYPE_PID. What, specifically, is wrong with the current approach?
>
> > I'm not sure offhand, can it go to the PIDTYPE_PGID from that after the
> > task has died, or not? I didn't think so. If it can then great.
>
> You're arguing that something that does, in fact, work, is somehow
> broken in some unspecified way. The kill_pid_info lookup works fine.
> What, specifically, is wrong with the semantics as implemented?
>
> > The point is (a) these are details which should not have to bother userspace,
>
> These details *don't* bother userspace.
>
> You're raising concerns that are either imaginary or long-since
> addressed. Does the patch cause some kind of maintenance burden? No,
> it doesn't, not moreso than any other piece of code. Does the
> interface have unclear semantics? No, it clearly sends a signal to a
> process, just like kill. Does the patch expose kernel implementation
> details? No, it doesn't, because the interface is simply not defined
> in terms of these details. Do we need to change how signal delivery
> works? No, because if it's fine for kill, it's fine for this facility,
> and if some future signal cleanup separates the cases more, that
> cleanup can change this code as well.
>
> The change is well-documented, simple, extensible, and addresses an
> actual problem. Every legitimate technical criticism has now been
> addressed. I don't understand where this opposition is coming from,
> since the objections refer to nothing that's actually in the patch or
> exposed to the user.
>
> > and (b) how to decide who we're sending the signal to (tid/pid/pgid) should
> > be specified in precisely one way. So either a flag, or comign from the type
> > of fd that was opened.
>
> You can't send signals to a thread with the current patch. There's no
> ambiguity in providing zero ways to do something.
So Serge's point is not about changing the current patch. What he's
basically saying is: If we are expected to state how we were to extend
this syscall in the future which Serge and I figured is currently Eric's
only remaining objection then:
- flags are a good way to go (I agree)
- there's a concrete way how to do this by stashing the relevent struct
pids for PIDTYPE_PID, PIDTYPE_TGID, PIDTYPE_PGID in file->private_data
which can then be retrieved in taskfd_send_signal()
There is not intent nor requirement to do this right now. What we have
right now is fine for a start, I agree! But here's how we go forward if
we ever need to! :)
Christian
^ permalink raw reply
* Re: [RFC PATCH 0/3] THP eligibility reporting via proc
From: Michal Hocko @ 2018-12-07 10:55 UTC (permalink / raw)
To: linux-api
Cc: Andrew Morton, Alexey Dobriyan, linux-mm, LKML, Dan Williams,
David Rientjes, Jan Kara
In-Reply-To: <20181120103515.25280-1-mhocko@kernel.org>
On Tue 20-11-18 11:35:12, Michal Hocko wrote:
> Hi,
> this series of three patches aims at making THP eligibility reporting
> much more robust and long term sustainable. The trigger for the change
> is a regression report [1] and the long follow up discussion. In short
> the specific application didn't have good API to query whether a particular
> mapping can be backed by THP so it has used VMA flags to workaround that.
> These flags represent a deep internal state of VMAs and as such they should
> be used by userspace with a great deal of caution.
>
> A similar has happened for [2] when users complained that VM_MIXEDMAP is
> no longer set on DAX mappings. Again a lack of a proper API led to an
> abuse.
>
> The first patch in the series tries to emphasise that that the semantic
> of flags might change and any application consuming those should be really
> careful.
>
> The remaining two patches provide a more suitable interface to address [1]
> and provide a consistent API to query the THP status both for each VMA
> and process wide as well.
Are there any other comments on these? I haven't heard any pushback so
far so I will re-send with RFC dropped early next week.
>
> [1] http://lkml.kernel.org/r/http://lkml.kernel.org/r/alpine.DEB.2.21.1809241054050.224429@chino.kir.corp.google.com
> [2] http://lkml.kernel.org/r/20181002100531.GC4135@quack2.suse.cz
>
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [PATCH v4] signal: add taskfd_send_signal() syscall
From: Serge E. Hallyn @ 2018-12-07 16:47 UTC (permalink / raw)
To: Christian Brauner
Cc: linux-kernel, linux-api, luto, arnd, ebiederm, serge, jannh, akpm,
oleg, cyphar, viro, linux-fsdevel, dancol, timmurray, linux-man,
keescook, fweimer, tglx, x86
In-Reply-To: <20181206121858.12215-1-christian@brauner.io>
On Thu, Dec 06, 2018 at 01:18:58PM +0100, Christian Brauner wrote:
> The kill() syscall operates on process identifiers (pid). 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.
>
> This patch uses file descriptors (fd) from proc/<pid> as stable handles on
> struct pid. Even if a pid is recycled the handle will not change. The fd
> can be used to send signals to the process it refers to.
> Thus, the new syscall taskfd_send_signal() is introduced to solve this
> problem. Instead of pids it operates on process fds (taskfd).
>
> /* prototype and argument /*
> long taskfd_send_signal(int taskfd, int sig, siginfo_t *info, unsigned int flags);
>
> In addition to the taskfd and signal argument it takes an additional
> siginfo_t and flags argument. If the siginfo_t argument is NULL then
> taskfd_send_signal() behaves like kill(). If it is not NULL
> taskfd_send_signal() 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. Failing to do so will cause EINVAL.
>
> /* taskfd_send_signal() replaces multiple pid-based syscalls */
> The taskfd_send_signal() syscall currently takes on the job of the
> following syscalls that operate on pids:
> - kill(2)
> - rt_sigqueueinfo(2)
> The syscall is defined in such a way that it can also operate on thread fds
> instead of process fds. In a future patchset I will extend it to operate on
> taskfds from /proc/<pid>/task/<tid> at which point it will additionally
> take on the job of:
> - tgkill(2)
> - rt_tgsigqueueinfo(2)
> Right now this is intentionally left out to keep this patchset as simple as
> possible (cf. [4]). If a taskfd of /proc/<pid>/task/<tid> is passed
> EOPNOTSUPP will be returned to give userspace a way to detect when I add
> support for such taskfds (cf. [10]).
>
> /* naming */
> The original prefix of the syscall was "procfd_". However, it has been
> pointed out that since this syscall will eventually operate on both
> processes and threads the name should reflect this (cf. [12]). The best
> possible candidate even from a userspace perspective seems to be "task".
> Although "task" is used internally we are alreday deviating from POSIX by
> using file descriptors to processes in the first place so it seems fine to
> use the "taskfd_" prefix.
>
> The name taskfd_send_signal() was also chosen to reflect the fact that it
> takes on the job of multiple syscalls. It is intentional that the name is
> not reminiscent of neither kill(2) nor rt_sigqueueinfo(2). Not the fomer
> because it might imply that taskfd_send_signal() is only a replacement for
> kill(2) and not the latter because it is a hazzle to remember the correct
> spelling (especially for non-native speakers) and because it is not
> descriptive enough of what the syscall actually does. The name
> "taskfd_send_signal" makes it very clear that its job is to send signals.
>
> /* O_PATH file descriptors */
> taskfds opened as O_PATH fds cannot be used to send signals to a process
> (cf. [2]). Signaling processes through taskfds is the equivalent of writing
> to a file. Thus, this is not an operation that operates "purely at the
> file descriptor level" as required by the open(2) manpage.
>
> /* zombies */
> Zombies can be signaled just as any other process. No special error will be
> reported since a zombie state is an unreliable state (cf. [3]).
>
> /* cross-namespace signals */
> The patch currently enforces that the signaler and signalee either are in
> the same pid namespace or that the signaler's pid namespace is an ancestor
> of the signalee's pid namespace. This is done for the sake of simplicity
> and because it is unclear to what values certain members of struct
> siginfo_t would need to be set to (cf. [5], [6]).
>
> /* compat syscalls */
> It became clear that we would like to avoid adding compat syscalls (cf.
> [7]). The compat syscall handling is now done in kernel/signal.c itself by
> adding __copy_siginfo_from_user_generic() which lets us avoid compat
> syscalls (cf. [8]). It should be noted that the addition of
> __copy_siginfo_from_user_any() is caused by a bug in the original
> implementation of rt_sigqueueinfo(2) (cf. 12).
> With upcoming rework for syscall handling things might improve
> significantly (cf. [11]) and __copy_siginfo_from_user_any() will not gain
> any additional callers.
>
> /* testing */
> This patch was tested on x64 and x86.
>
> /* userspace usage */
> An asciinema recording for the basic functionality can be found under [9].
> With this patch a process can be killed via:
>
> #define _GNU_SOURCE
> #include <errno.h>
> #include <fcntl.h>
> #include <signal.h>
> #include <stdio.h>
> #include <stdlib.h>
> #include <string.h>
> #include <sys/stat.h>
> #include <sys/syscall.h>
> #include <sys/types.h>
> #include <unistd.h>
>
> static inline int do_taskfd_send_signal(int taskfd, int sig, siginfo_t *info,
> unsigned int flags)
> {
> #ifdef __NR_taskfd_send_signal
> return syscall(__NR_taskfd_send_signal, taskfd, sig, info, flags);
> #else
> return -ENOSYS;
> #endif
> }
>
> int main(int argc, char *argv[])
> {
> int fd, ret, saved_errno, sig;
>
> if (argc < 3)
> exit(EXIT_FAILURE);
>
> fd = open(argv[1], O_DIRECTORY | O_CLOEXEC);
> if (fd < 0) {
> printf("%s - Failed to open \"%s\"\n", strerror(errno), argv[1]);
> exit(EXIT_FAILURE);
> }
>
> sig = atoi(argv[2]);
>
> printf("Sending signal %d to process %s\n", sig, argv[1]);
> ret = do_taskfd_send_signal(fd, sig, NULL, 0);
>
> saved_errno = errno;
> close(fd);
> errno = saved_errno;
>
> if (ret < 0) {
> printf("%s - Failed to send signal %d to process %s\n",
> strerror(errno), sig, argv[1]);
> exit(EXIT_FAILURE);
> }
>
> exit(EXIT_SUCCESS);
> }
>
> [1]: https://lore.kernel.org/lkml/20181029221037.87724-1-dancol@google.com/
> [2]: https://lore.kernel.org/lkml/874lbtjvtd.fsf@oldenburg2.str.redhat.com/
> [3]: https://lore.kernel.org/lkml/20181204132604.aspfupwjgjx6fhva@brauner.io/
> [4]: https://lore.kernel.org/lkml/20181203180224.fkvw4kajtbvru2ku@brauner.io/
> [5]: https://lore.kernel.org/lkml/20181121213946.GA10795@mail.hallyn.com/
> [6]: https://lore.kernel.org/lkml/20181120103111.etlqp7zop34v6nv4@brauner.io/
> [7]: https://lore.kernel.org/lkml/36323361-90BD-41AF-AB5B-EE0D7BA02C21@amacapital.net/
> [8]: https://lore.kernel.org/lkml/87tvjxp8pc.fsf@xmission.com/
> [9]: https://asciinema.org/a/IQjuCHew6bnq1cr78yuMv16cy
> [10]: https://lore.kernel.org/lkml/20181203180224.fkvw4kajtbvru2ku@brauner.io/
> [11]: https://lore.kernel.org/lkml/F53D6D38-3521-4C20-9034-5AF447DF62FF@amacapital.net/
> [12]: https://lore.kernel.org/lkml/87zhtjn8ck.fsf@xmission.com/
>
> Cc: Arnd Bergmann <arnd@arndb.de>
> Cc: "Eric W. Biederman" <ebiederm@xmission.com>
> Cc: Serge Hallyn <serge@hallyn.com>
> Cc: Jann Horn <jannh@google.com>
> 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>
> Cc: Florian Weimer <fweimer@redhat.com>
> Signed-off-by: Christian Brauner <christian@brauner.io>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> ---
> Changelog:
> v4:
> - updated asciinema to use "taskfd_" prefix
> - s/procfd_send_signal/taskfd_send_signal/g
> - s/proc_is_tgid_procfd/tgid_taskfd_to_pid/b
> - s/proc_is_tid_procfd/tid_taskfd_to_pid/b
> - s/__copy_siginfo_from_user_generic/__copy_siginfo_from_user_any/g
> - make it clear that __copy_siginfo_from_user_any() is a workaround caused
> by a bug in the original implementation of rt_sigqueueinfo()
> - when spoofing signals turn them into regular kill signals if si_code is
> set to SI_USER
> - make proc_is_t{g}id_procfd() return struct pid to allow proc_pid() to
> stay private to fs/proc/
> v3:
> - add __copy_siginfo_from_user_generic() to avoid adding compat syscalls
> - s/procfd_signal/procfd_send_signal/g
> - change type of flags argument from int to unsigned int
> - add comment about what happens to zombies
> - add proc_is_tid_procfd()
> - return EOPNOTSUPP when /proc/<pid>/task/<tid> fd is passed so userspace
> has a way of knowing that tidfds are not supported currently.
> v2:
> - define __NR_procfd_signal in unistd.h
> - wire up compat syscall
> - s/proc_is_procfd/proc_is_tgid_procfd/g
> - provide stubs when CONFIG_PROC_FS=n
> - move proc_pid() to linux/proc_fs.h header
> - use proc_pid() to grab struct pid from /proc/<pid> fd
> v1:
> - patch introduced
> ---
> arch/x86/entry/syscalls/syscall_32.tbl | 1 +
> arch/x86/entry/syscalls/syscall_64.tbl | 1 +
> fs/proc/base.c | 20 +++-
> include/linux/proc_fs.h | 12 +++
> include/linux/syscalls.h | 3 +
> include/uapi/asm-generic/unistd.h | 4 +-
> kernel/signal.c | 132 +++++++++++++++++++++++--
> 7 files changed, 164 insertions(+), 9 deletions(-)
>
> diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
> index 3cf7b533b3d1..7efb63fd0617 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 taskfd_send_signal sys_taskfd_send_signal __ia32_sys_taskfd_send_signal
> diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
> index f0b1709a5ffb..be894f4a84e9 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 taskfd_send_signal __x64_sys_taskfd_send_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 ce3465479447..b8b88bfee455 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -716,8 +716,6 @@ static int proc_pid_permission(struct inode *inode, int mask)
> return generic_permission(inode, mask);
> }
>
> -
> -
> static const struct inode_operations proc_def_inode_operations = {
> .setattr = proc_setattr,
> };
> @@ -3038,6 +3036,15 @@ static const struct file_operations proc_tgid_base_operations = {
> .llseek = generic_file_llseek,
> };
>
> +struct pid *tgid_taskfd_to_pid(const struct file *file)
> +{
> + if (!d_is_dir(file->f_path.dentry) ||
> + (file->f_op != &proc_tgid_base_operations))
> + return ERR_PTR(-EBADF);
> +
> + return proc_pid(file_inode(file));
> +}
> +
> static struct dentry *proc_tgid_base_lookup(struct inode *dir, struct dentry *dentry, unsigned int flags)
> {
> return proc_pident_lookup(dir, dentry,
> @@ -3422,6 +3429,15 @@ static const struct file_operations proc_tid_base_operations = {
> .llseek = generic_file_llseek,
> };
>
> +struct pid *tid_taskfd_to_pid(const struct file *file)
> +{
> + if (!d_is_dir(file->f_path.dentry) ||
> + (file->f_op != &proc_tid_base_operations))
> + return ERR_PTR(-EBADF);
> +
> + return proc_pid(file_inode(file));
> +}
> +
> static const struct inode_operations proc_tid_base_inode_operations = {
> .lookup = proc_tid_base_lookup,
> .getattr = pid_getattr,
> diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
> index d0e1f1522a78..96817415c420 100644
> --- a/include/linux/proc_fs.h
> +++ b/include/linux/proc_fs.h
> @@ -73,6 +73,8 @@ 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 struct pid *tgid_taskfd_to_pid(const struct file *file);
> +extern struct pid *tid_taskfd_to_pid(const struct file *file);
>
> #else /* CONFIG_PROC_FS */
>
> @@ -114,6 +116,16 @@ static inline int remove_proc_subtree(const char *name, struct proc_dir_entry *p
> #define proc_create_net(name, mode, parent, state_size, ops) ({NULL;})
> #define proc_create_net_single(name, mode, parent, show, data) ({NULL;})
>
> +static inline struct pid *tgid_taskfd_to_pid(const struct file *file)
> +{
> + return ERR_PTR(-EBADF);
> +}
> +
> +static inline struct pid *tid_taskfd_to_pid(const struct file *file)
> +{
> + return ERR_PTR(-EBADF);
> +}
> +
> #endif /* CONFIG_PROC_FS */
>
> struct net;
> diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
> index 2ac3d13a915b..5ffe194ef29b 100644
> --- a/include/linux/syscalls.h
> +++ b/include/linux/syscalls.h
> @@ -907,6 +907,9 @@ 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_taskfd_send_signal(int taskfd, int sig,
> + siginfo_t __user *info,
> + unsigned int flags);
>
> /*
> * Architecture-specific system calls
> diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
> index 538546edbfbd..9343dca63fd9 100644
> --- a/include/uapi/asm-generic/unistd.h
> +++ b/include/uapi/asm-generic/unistd.h
> @@ -738,9 +738,11 @@ __SYSCALL(__NR_statx, sys_statx)
> __SC_COMP(__NR_io_pgetevents, sys_io_pgetevents, compat_sys_io_pgetevents)
> #define __NR_rseq 293
> __SYSCALL(__NR_rseq, sys_rseq)
> +#define __NR_taskfd_send_signal 294
> +__SYSCALL(__NR_taskfd_send_signal, sys_taskfd_send_signal)
>
> #undef __NR_syscalls
> -#define __NR_syscalls 294
> +#define __NR_syscalls 295
>
> /*
> * 32 bit systems traditionally used different
> diff --git a/kernel/signal.c b/kernel/signal.c
> index 9a32bc2088c9..a00a4bcb7605 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,124 @@ 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);
> }
>
> +/*
> + * Verify that the signaler and signalee either are in the same pid namespace
> + * or that the signaler's pid namespace is an ancestor of the signalee's pid
> + * namespace.
> + */
> +static bool may_signal_taskfd(struct pid *pid)
may_signal_taskfd seems like a misnomer here. It leads the reader to think it
is about permission, when it's really about accessibility.
> +{
> + struct pid_namespace *active = task_active_pid_ns(current);
> + struct pid_namespace *p = ns_of_pid(pid);
> +
> + for (;;) {
> + if (!p)
> + return false;
> + if (p == active)
> + break;
> + p = p->parent;
> + }
> +
> + return true;
> +}
> +
> +static int copy_siginfo_from_user_any(kernel_siginfo_t *kinfo, siginfo_t *info)
> +{
> +#ifdef CONFIG_COMPAT
> + /*
> + * Avoid hooking up compat syscalls and instead handle necessary
> + * conversions here. Note, this is a stop-gap measure and should not be
> + * considered a generic solution.
> + */
> + if (in_compat_syscall())
> + return copy_siginfo_from_user32(
> + kinfo, (struct compat_siginfo __user *)info);
> +#endif
> + return copy_siginfo_from_user(kinfo, info);
> +}
> +
> +/**
> + * sys_taskfd_send_signal - send a signal to a process through a task file
> + * descriptor
> + * @taskfd: the file descriptor of the process
> + * @sig: signal to be sent
> + * @info: the signal info
> + * @flags: future flags to be passed
> + *
> + * Return: 0 on success, negative errno on failure
> + */
> +SYSCALL_DEFINE4(taskfd_send_signal, int, taskfd, int, sig,
> + siginfo_t __user *, info, unsigned int, flags)
> +{
> + int ret;
> + struct fd f;
> + struct pid *pid;
> + kernel_siginfo_t kinfo;
> +
> + /* Enforce flags be set to 0 until we add an extension. */
> + if (flags)
> + return -EINVAL;
> +
> + f = fdget_raw(taskfd);
> + if (!f.file)
> + return -EBADF;
> +
> + pid = tid_taskfd_to_pid(f.file);
> + if (!IS_ERR(pid)) {
> + /*
> + * Give userspace a way to detect /proc/<pid>/task/<tid>
> + * support when we add it.
> + */
> + ret = -EOPNOTSUPP;
> + goto err;
> + }
> +
> + /* Is this a procfd? */
> + pid = tgid_taskfd_to_pid(f.file);
> + if (IS_ERR(pid)) {
> + ret = PTR_ERR(pid);
> + goto err;
> + }
> +
> + ret = -EINVAL;
> + if (!may_signal_taskfd(pid))
> + goto err;
> +
> + if (info) {
> + ret = copy_siginfo_from_user_any(&kinfo, info);
> + if (unlikely(ret))
> + goto err;
> +
> + ret = -EINVAL;
> + if (unlikely(sig != kinfo.si_signo))
> + goto err;
> +
> + if ((task_pid(current) != pid) &&
> + (kinfo.si_code >= 0 || kinfo.si_code == SI_TKILL)) {
> + /* Only allow sending arbitrary signals to yourself. */
> + ret = -EPERM;
> + if (kinfo.si_code != SI_USER)
> + goto err;
> +
> + /* Turn this into a regular kill signal. */
> + prepare_kill_siginfo(sig, &kinfo);
> + }
> + } 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
* Re: [PATCH v4] signal: add taskfd_send_signal() syscall
From: Serge E. Hallyn @ 2018-12-07 16:49 UTC (permalink / raw)
To: Christian Brauner
Cc: Daniel Colascione, Serge E. Hallyn, Eric W. Biederman,
linux-kernel, Linux API, Andy Lutomirski, Arnd Bergmann,
Jann Horn, Andrew Morton, Oleg Nesterov, Aleksa Sarai, Al Viro,
Linux FS Devel, Tim Murray, linux-man, Kees Cook, Florian Weimer,
tglx, x86
In-Reply-To: <20181207015423.4miorx43l3qhppfz@brauner.io>
On Fri, Dec 07, 2018 at 02:54:25AM +0100, Christian Brauner wrote:
> On Thu, Dec 06, 2018 at 05:39:18PM -0800, Daniel Colascione wrote:
> > On Thu, Dec 6, 2018 at 4:59 PM Serge E. Hallyn <serge@hallyn.com> wrote:
> > >
> > > On Thu, Dec 06, 2018 at 04:34:54PM -0800, Daniel Colascione wrote:
> > > > On Thu, Dec 6, 2018 at 4:31 PM Serge E. Hallyn <serge@hallyn.com> wrote:
> > > > >
> > > > > On Fri, Dec 07, 2018 at 12:17:45AM +0100, Christian Brauner wrote:
> > > > > > On Thu, Dec 06, 2018 at 11:39:48PM +0100, Christian Brauner wrote:
> > > > > > > On Thu, Dec 06, 2018 at 03:46:53PM -0600, Eric W. Biederman wrote:
> > > > > > > > Christian Brauner <christian@brauner.io> writes:
> > > > > > > >
> > > > > > > > >> Your intention is to add the thread case to support pthreads once the
> > > > > > > > >> process case is sorted out. So this is something that needs to be made
> > > > > > > > >> clear. Did I miss how you plan to handle threads?
> > > > > > > > >
> > > > > > > > > Yeah, maybe you missed it in the commit message [2] which is based on a
> > > > > > > > > discussion with Andy [3] and Arnd [4]:
> > > > > > > >
> > > > > > > > Looking at your references I haven't missed it. You are not deciding
> > > > > > > > anything as of yet to keep it simple. Except you are returning
> > > > > > > > EOPNOTSUPP. You are very much intending to do something.
> > > > > > >
> > > > > > > That was clear all along and was pointed at every occassion in the
> > > > > > > threads. I even went through the hazzle to give you all of the
> > > > > > > references when there's lore.kernel.org.
> > > > > > >
> > > > > > > >
> > > > > > > > Decide. Do you use the flags parameter or is the width of the
> > > > > > > > target depending on the flags.
> > > > > >
> > > > > > Ok, let's try to be constructive. I understand the general concern for
> > > > > > the future so let's put a contract into the commit message stating that
> > > > > > the width of the target aka *what is signaled* will be based on a flag
> > > > > > parameter if we ever extend it:
> > > > > >
> > > > > > taskfd_send_signal(fd, SIGSTOP, NULL, TASKFD_PGID);
> > > > > > taskfd_send_signal(fd, SIGSTOP, NULL, TASKFD_TID);
> > > > > >
> > > > > > with the current default being
> > > > > >
> > > > > > taskfd_send_signal(fd, SIGSTOP, NULL, TASKFD_PID);
> > > > > >
> > > > > > This seems to me the cleanest solution as we only use one type of file
> > > > > > descriptor. Can everyone be on board with this? If so I'm going to send
> > > > > > out a new version of the patch.
> > > > > >
> > > > > > Christian
> > > > >
> > > > > I'm on board with this, but I think you need to also clarify what exactly
> > > > > the fd stands for. I think that (a) userspace should not have to care
> > > > > about the struct pid implementation, and so (b) the procfd should stand
> > > > > for all the pids. So when taskfd_send_signal(fd, SIGSTOP, NULL, TASKFD_PGID)
> > > > > becomes implemented, then open(/proc/5) will pin all three pids, as will
> > > > > open(/proc/5/task/6).
> > > >
> > > > This change doesn't "pin" any PID, and it makes no sense to make a
> > > > process FD stand for all its threads. What does that even mean?
> > >
> > > Currently the patch relies on the procfd inode saving a copy to the PIDTYPE_PID
> > > pid.
> >
> > struct pid doesn't have a type field. The interpretation depends on
> > the caller's use of the struct pid, and in the current path, that's
> > PIDTYPE_PID. What, specifically, is wrong with the current approach?
> >
> > > I'm not sure offhand, can it go to the PIDTYPE_PGID from that after the
> > > task has died, or not? I didn't think so. If it can then great.
> >
> > You're arguing that something that does, in fact, work, is somehow
> > broken in some unspecified way. The kill_pid_info lookup works fine.
> > What, specifically, is wrong with the semantics as implemented?
> >
> > > The point is (a) these are details which should not have to bother userspace,
> >
> > These details *don't* bother userspace.
> >
> > You're raising concerns that are either imaginary or long-since
> > addressed. Does the patch cause some kind of maintenance burden? No,
> > it doesn't, not moreso than any other piece of code. Does the
> > interface have unclear semantics? No, it clearly sends a signal to a
> > process, just like kill. Does the patch expose kernel implementation
> > details? No, it doesn't, because the interface is simply not defined
> > in terms of these details. Do we need to change how signal delivery
> > works? No, because if it's fine for kill, it's fine for this facility,
> > and if some future signal cleanup separates the cases more, that
> > cleanup can change this code as well.
> >
> > The change is well-documented, simple, extensible, and addresses an
> > actual problem. Every legitimate technical criticism has now been
> > addressed. I don't understand where this opposition is coming from,
> > since the objections refer to nothing that's actually in the patch or
> > exposed to the user.
> >
> > > and (b) how to decide who we're sending the signal to (tid/pid/pgid) should
> > > be specified in precisely one way. So either a flag, or comign from the type
> > > of fd that was opened.
> >
> > You can't send signals to a thread with the current patch. There's no
> > ambiguity in providing zero ways to do something.
>
> So Serge's point is not about changing the current patch. What he's
Right, I'm an ack on the patch. As is no changes are needed.
> basically saying is: If we are expected to state how we were to extend
> this syscall in the future which Serge and I figured is currently Eric's
> only remaining objection then:
> - flags are a good way to go (I agree)
> - there's a concrete way how to do this by stashing the relevent struct
> pids for PIDTYPE_PID, PIDTYPE_TGID, PIDTYPE_PGID in file->private_data
> which can then be retrieved in taskfd_send_signal()
> There is not intent nor requirement to do this right now. What we have
> right now is fine for a start, I agree! But here's how we go forward if
> we ever need to! :)
>
> Christian
^ permalink raw reply
* [PATCH 0/5] y2038: further system call conversion
From: Arnd Bergmann @ 2018-12-07 21:18 UTC (permalink / raw)
To: y2038
Cc: linux-kernel, Deepa Dinamani, John Stultz, Thomas Gleixner,
linux-api, Arnd Bergmann
This is another set of system call changes that were intended to get
merged much earlier. I've rebased the patches on top of the latest kernel
and Deepa's poll/select/io_pgetevents series now, and applied it to the
same y2038 branch for inclusion in 4.21.
This is just a continuation of the earlier system call changes, so I
expect no surprises here, unless I made a mistake in the rebase.
After this series, the only remaining system calls that need to be
modified are:
- setsockopt/getsockopt, they do not need a new entry in the system
call table, only new numbers assigned for their socket options.
Deepa is finalizing her series to passing 64-bit timestamps in cmsg
correctly for SO_TIMESTAMP{,NS,ING}.
Further, we need a new SO_RCVTIMEO/SO_SNDTIMEO implementation
here, which has not been implemented yet.
- waitid/getrusage, this needs a rewrite now from earlier versions
based on recent feedback. The plan now is to incorporate nanosecond
timestamps in a new revision of the 'rusage' structure. Possibly also
include BSD wait6() semantics of passing back the child resource
usage in waitid(), and/or a method to pass a signal mask as in
io_pgetevents(). waitid/getrusage() are not required for correct
behavior with 64-bit time_t and can be safely emulated on top of
the existing syscalls by the C library beyond 2038. This needs more
discussion.
- clock_adjtime, this is also optional and can be implemented
by the C library if necessary. The plan is to do the same as x32
and use the 64-bit structure natively on both 32-bit and 64-bit
architectures. The timex structure contains a 'timeval' at the
moment, but also supports nanoseconds passed in it.
- getitimer/setitimer, these again can be trivially emulated
by the C library.
Arnd
Arnd Bergmann (5):
y2038: futex: Move compat implementation into futex.c
y2038: futex: Add support for __kernel_timespec
y2038: socket: Add compat_sys_recvmmsg_time64
y2038: signal: Add sys_rt_sigtimedwait_time32
y2038: signal: Add compat_sys_rt_sigtimedwait_time64
include/linux/compat.h | 8 +-
include/linux/futex.h | 8 --
include/linux/socket.h | 9 +-
include/linux/syscalls.h | 9 +-
kernel/Makefile | 3 -
kernel/futex.c | 207 +++++++++++++++++++++++++++++++++++++--
kernel/futex_compat.c | 202 --------------------------------------
kernel/signal.c | 65 ++++++++++++
kernel/sys_ni.c | 2 +
net/compat.c | 34 +++----
net/socket.c | 62 +++++++++---
11 files changed, 345 insertions(+), 264 deletions(-)
delete mode 100644 kernel/futex_compat.c
--
2.18.0
^ permalink raw reply
* [PATCH 1/5] y2038: futex: Move compat implementation into futex.c
From: Arnd Bergmann @ 2018-12-07 21:18 UTC (permalink / raw)
To: y2038
Cc: linux-kernel, Deepa Dinamani, John Stultz, Thomas Gleixner,
linux-api, Arnd Bergmann
In-Reply-To: <20181207211808.328719-1-arnd@arndb.de>
We are going to share the compat_sys_futex() handler between 64-bit
architectures and 32-bit architectures that need to deal with both 32-bit
and 64-bit time_t, and this is easier if both entry points are in the
same file.
In fact, most other system call handlers do the same thing these days, so
let's follow the trend here and merge all of futex_compat.c into futex.c.
In the process, a few minor changes have to be done to make sure everything
still makes sense: handle_futex_death() and futex_cmpxchg_enabled() become
local symbol, and the compat version of the fetch_robust_entry() function
gets renamed to compat_fetch_robust_entry() to avoid a symbol clash.
This is intended as a purely cosmetic patch, no behavior should
change.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
include/linux/futex.h | 8 --
kernel/Makefile | 3 -
kernel/futex.c | 195 +++++++++++++++++++++++++++++++++++++++-
kernel/futex_compat.c | 202 ------------------------------------------
4 files changed, 192 insertions(+), 216 deletions(-)
delete mode 100644 kernel/futex_compat.c
diff --git a/include/linux/futex.h b/include/linux/futex.h
index 821ae502d3d8..ccaef0097785 100644
--- a/include/linux/futex.h
+++ b/include/linux/futex.h
@@ -9,9 +9,6 @@ struct inode;
struct mm_struct;
struct task_struct;
-extern int
-handle_futex_death(u32 __user *uaddr, struct task_struct *curr, int pi);
-
/*
* Futexes are matched on equal values of this key.
* The key type depends on whether it's a shared or private mapping.
@@ -55,11 +52,6 @@ extern void exit_robust_list(struct task_struct *curr);
long do_futex(u32 __user *uaddr, int op, u32 val, ktime_t *timeout,
u32 __user *uaddr2, u32 val2, u32 val3);
-#ifdef CONFIG_HAVE_FUTEX_CMPXCHG
-#define futex_cmpxchg_enabled 1
-#else
-extern int futex_cmpxchg_enabled;
-#endif
#else
static inline void exit_robust_list(struct task_struct *curr)
{
diff --git a/kernel/Makefile b/kernel/Makefile
index 7343b3a9bff0..8e40a6742d23 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -49,9 +49,6 @@ obj-$(CONFIG_PROFILING) += profile.o
obj-$(CONFIG_STACKTRACE) += stacktrace.o
obj-y += time/
obj-$(CONFIG_FUTEX) += futex.o
-ifeq ($(CONFIG_COMPAT),y)
-obj-$(CONFIG_FUTEX) += futex_compat.o
-endif
obj-$(CONFIG_GENERIC_ISA_DMA) += dma.o
obj-$(CONFIG_SMP) += smp.o
ifneq ($(CONFIG_SMP),y)
diff --git a/kernel/futex.c b/kernel/futex.c
index f423f9b6577e..5cc7c3b098e9 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -44,6 +44,7 @@
* along with this program; if not, write to the Free Software
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*/
+#include <linux/compat.h>
#include <linux/slab.h>
#include <linux/poll.h>
#include <linux/fs.h>
@@ -173,8 +174,10 @@
* double_lock_hb() and double_unlock_hb(), respectively.
*/
-#ifndef CONFIG_HAVE_FUTEX_CMPXCHG
-int __read_mostly futex_cmpxchg_enabled;
+#ifdef CONFIG_HAVE_FUTEX_CMPXCHG
+#define futex_cmpxchg_enabled 1
+#else
+static int __read_mostly futex_cmpxchg_enabled;
#endif
/*
@@ -3360,7 +3363,7 @@ SYSCALL_DEFINE3(get_robust_list, int, pid,
* Process a futex-list entry, check whether it's owned by the
* dying task, and do notification if so:
*/
-int handle_futex_death(u32 __user *uaddr, struct task_struct *curr, int pi)
+static int handle_futex_death(u32 __user *uaddr, struct task_struct *curr, int pi)
{
u32 uval, uninitialized_var(nval), mval;
@@ -3589,6 +3592,192 @@ SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val,
return do_futex(uaddr, op, val, tp, uaddr2, val2, val3);
}
+#ifdef CONFIG_COMPAT
+/*
+ * Fetch a robust-list pointer. Bit 0 signals PI futexes:
+ */
+static inline int
+compat_fetch_robust_entry(compat_uptr_t *uentry, struct robust_list __user **entry,
+ compat_uptr_t __user *head, unsigned int *pi)
+{
+ if (get_user(*uentry, head))
+ return -EFAULT;
+
+ *entry = compat_ptr((*uentry) & ~1);
+ *pi = (unsigned int)(*uentry) & 1;
+
+ return 0;
+}
+
+static void __user *futex_uaddr(struct robust_list __user *entry,
+ compat_long_t futex_offset)
+{
+ compat_uptr_t base = ptr_to_compat(entry);
+ void __user *uaddr = compat_ptr(base + futex_offset);
+
+ return uaddr;
+}
+
+/*
+ * Walk curr->robust_list (very carefully, it's a userspace list!)
+ * and mark any locks found there dead, and notify any waiters.
+ *
+ * We silently return on any sign of list-walking problem.
+ */
+void compat_exit_robust_list(struct task_struct *curr)
+{
+ struct compat_robust_list_head __user *head = curr->compat_robust_list;
+ struct robust_list __user *entry, *next_entry, *pending;
+ unsigned int limit = ROBUST_LIST_LIMIT, pi, pip;
+ unsigned int uninitialized_var(next_pi);
+ compat_uptr_t uentry, next_uentry, upending;
+ compat_long_t futex_offset;
+ int rc;
+
+ if (!futex_cmpxchg_enabled)
+ return;
+
+ /*
+ * Fetch the list head (which was registered earlier, via
+ * sys_set_robust_list()):
+ */
+ if (compat_fetch_robust_entry(&uentry, &entry, &head->list.next, &pi))
+ return;
+ /*
+ * Fetch the relative futex offset:
+ */
+ if (get_user(futex_offset, &head->futex_offset))
+ return;
+ /*
+ * Fetch any possibly pending lock-add first, and handle it
+ * if it exists:
+ */
+ if (compat_fetch_robust_entry(&upending, &pending,
+ &head->list_op_pending, &pip))
+ return;
+
+ next_entry = NULL; /* avoid warning with gcc */
+ while (entry != (struct robust_list __user *) &head->list) {
+ /*
+ * Fetch the next entry in the list before calling
+ * handle_futex_death:
+ */
+ rc = compat_fetch_robust_entry(&next_uentry, &next_entry,
+ (compat_uptr_t __user *)&entry->next, &next_pi);
+ /*
+ * A pending lock might already be on the list, so
+ * dont process it twice:
+ */
+ if (entry != pending) {
+ void __user *uaddr = futex_uaddr(entry, futex_offset);
+
+ if (handle_futex_death(uaddr, curr, pi))
+ return;
+ }
+ if (rc)
+ return;
+ uentry = next_uentry;
+ entry = next_entry;
+ pi = next_pi;
+ /*
+ * Avoid excessively long or circular lists:
+ */
+ if (!--limit)
+ break;
+
+ cond_resched();
+ }
+ if (pending) {
+ void __user *uaddr = futex_uaddr(pending, futex_offset);
+
+ handle_futex_death(uaddr, curr, pip);
+ }
+}
+
+COMPAT_SYSCALL_DEFINE2(set_robust_list,
+ struct compat_robust_list_head __user *, head,
+ compat_size_t, len)
+{
+ if (!futex_cmpxchg_enabled)
+ return -ENOSYS;
+
+ if (unlikely(len != sizeof(*head)))
+ return -EINVAL;
+
+ current->compat_robust_list = head;
+
+ return 0;
+}
+
+COMPAT_SYSCALL_DEFINE3(get_robust_list, int, pid,
+ compat_uptr_t __user *, head_ptr,
+ compat_size_t __user *, len_ptr)
+{
+ struct compat_robust_list_head __user *head;
+ unsigned long ret;
+ struct task_struct *p;
+
+ if (!futex_cmpxchg_enabled)
+ return -ENOSYS;
+
+ rcu_read_lock();
+
+ ret = -ESRCH;
+ if (!pid)
+ p = current;
+ else {
+ p = find_task_by_vpid(pid);
+ if (!p)
+ goto err_unlock;
+ }
+
+ ret = -EPERM;
+ if (!ptrace_may_access(p, PTRACE_MODE_READ_REALCREDS))
+ goto err_unlock;
+
+ head = p->compat_robust_list;
+ rcu_read_unlock();
+
+ if (put_user(sizeof(*head), len_ptr))
+ return -EFAULT;
+ return put_user(ptr_to_compat(head), head_ptr);
+
+err_unlock:
+ rcu_read_unlock();
+
+ return ret;
+}
+
+COMPAT_SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val,
+ struct old_timespec32 __user *, utime, u32 __user *, uaddr2,
+ u32, val3)
+{
+ struct timespec ts;
+ ktime_t t, *tp = NULL;
+ int val2 = 0;
+ int cmd = op & FUTEX_CMD_MASK;
+
+ if (utime && (cmd == FUTEX_WAIT || cmd == FUTEX_LOCK_PI ||
+ cmd == FUTEX_WAIT_BITSET ||
+ cmd == FUTEX_WAIT_REQUEUE_PI)) {
+ if (compat_get_timespec(&ts, utime))
+ return -EFAULT;
+ if (!timespec_valid(&ts))
+ return -EINVAL;
+
+ t = timespec_to_ktime(ts);
+ if (cmd == FUTEX_WAIT)
+ t = ktime_add_safe(ktime_get(), t);
+ tp = &t;
+ }
+ if (cmd == FUTEX_REQUEUE || cmd == FUTEX_CMP_REQUEUE ||
+ cmd == FUTEX_CMP_REQUEUE_PI || cmd == FUTEX_WAKE_OP)
+ val2 = (int) (unsigned long) utime;
+
+ return do_futex(uaddr, op, val, tp, uaddr2, val2, val3);
+}
+#endif /* CONFIG_COMPAT */
+
static void __init futex_detect_cmpxchg(void)
{
#ifndef CONFIG_HAVE_FUTEX_CMPXCHG
diff --git a/kernel/futex_compat.c b/kernel/futex_compat.c
deleted file mode 100644
index 410a77a8f6e2..000000000000
--- a/kernel/futex_compat.c
+++ /dev/null
@@ -1,202 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * linux/kernel/futex_compat.c
- *
- * Futex compatibililty routines.
- *
- * Copyright 2006, Red Hat, Inc., Ingo Molnar
- */
-
-#include <linux/linkage.h>
-#include <linux/compat.h>
-#include <linux/nsproxy.h>
-#include <linux/futex.h>
-#include <linux/ptrace.h>
-#include <linux/syscalls.h>
-
-#include <linux/uaccess.h>
-
-
-/*
- * Fetch a robust-list pointer. Bit 0 signals PI futexes:
- */
-static inline int
-fetch_robust_entry(compat_uptr_t *uentry, struct robust_list __user **entry,
- compat_uptr_t __user *head, unsigned int *pi)
-{
- if (get_user(*uentry, head))
- return -EFAULT;
-
- *entry = compat_ptr((*uentry) & ~1);
- *pi = (unsigned int)(*uentry) & 1;
-
- return 0;
-}
-
-static void __user *futex_uaddr(struct robust_list __user *entry,
- compat_long_t futex_offset)
-{
- compat_uptr_t base = ptr_to_compat(entry);
- void __user *uaddr = compat_ptr(base + futex_offset);
-
- return uaddr;
-}
-
-/*
- * Walk curr->robust_list (very carefully, it's a userspace list!)
- * and mark any locks found there dead, and notify any waiters.
- *
- * We silently return on any sign of list-walking problem.
- */
-void compat_exit_robust_list(struct task_struct *curr)
-{
- struct compat_robust_list_head __user *head = curr->compat_robust_list;
- struct robust_list __user *entry, *next_entry, *pending;
- unsigned int limit = ROBUST_LIST_LIMIT, pi, pip;
- unsigned int uninitialized_var(next_pi);
- compat_uptr_t uentry, next_uentry, upending;
- compat_long_t futex_offset;
- int rc;
-
- if (!futex_cmpxchg_enabled)
- return;
-
- /*
- * Fetch the list head (which was registered earlier, via
- * sys_set_robust_list()):
- */
- if (fetch_robust_entry(&uentry, &entry, &head->list.next, &pi))
- return;
- /*
- * Fetch the relative futex offset:
- */
- if (get_user(futex_offset, &head->futex_offset))
- return;
- /*
- * Fetch any possibly pending lock-add first, and handle it
- * if it exists:
- */
- if (fetch_robust_entry(&upending, &pending,
- &head->list_op_pending, &pip))
- return;
-
- next_entry = NULL; /* avoid warning with gcc */
- while (entry != (struct robust_list __user *) &head->list) {
- /*
- * Fetch the next entry in the list before calling
- * handle_futex_death:
- */
- rc = fetch_robust_entry(&next_uentry, &next_entry,
- (compat_uptr_t __user *)&entry->next, &next_pi);
- /*
- * A pending lock might already be on the list, so
- * dont process it twice:
- */
- if (entry != pending) {
- void __user *uaddr = futex_uaddr(entry, futex_offset);
-
- if (handle_futex_death(uaddr, curr, pi))
- return;
- }
- if (rc)
- return;
- uentry = next_uentry;
- entry = next_entry;
- pi = next_pi;
- /*
- * Avoid excessively long or circular lists:
- */
- if (!--limit)
- break;
-
- cond_resched();
- }
- if (pending) {
- void __user *uaddr = futex_uaddr(pending, futex_offset);
-
- handle_futex_death(uaddr, curr, pip);
- }
-}
-
-COMPAT_SYSCALL_DEFINE2(set_robust_list,
- struct compat_robust_list_head __user *, head,
- compat_size_t, len)
-{
- if (!futex_cmpxchg_enabled)
- return -ENOSYS;
-
- if (unlikely(len != sizeof(*head)))
- return -EINVAL;
-
- current->compat_robust_list = head;
-
- return 0;
-}
-
-COMPAT_SYSCALL_DEFINE3(get_robust_list, int, pid,
- compat_uptr_t __user *, head_ptr,
- compat_size_t __user *, len_ptr)
-{
- struct compat_robust_list_head __user *head;
- unsigned long ret;
- struct task_struct *p;
-
- if (!futex_cmpxchg_enabled)
- return -ENOSYS;
-
- rcu_read_lock();
-
- ret = -ESRCH;
- if (!pid)
- p = current;
- else {
- p = find_task_by_vpid(pid);
- if (!p)
- goto err_unlock;
- }
-
- ret = -EPERM;
- if (!ptrace_may_access(p, PTRACE_MODE_READ_REALCREDS))
- goto err_unlock;
-
- head = p->compat_robust_list;
- rcu_read_unlock();
-
- if (put_user(sizeof(*head), len_ptr))
- return -EFAULT;
- return put_user(ptr_to_compat(head), head_ptr);
-
-err_unlock:
- rcu_read_unlock();
-
- return ret;
-}
-
-COMPAT_SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val,
- struct old_timespec32 __user *, utime, u32 __user *, uaddr2,
- u32, val3)
-{
- struct timespec ts;
- ktime_t t, *tp = NULL;
- int val2 = 0;
- int cmd = op & FUTEX_CMD_MASK;
-
- if (utime && (cmd == FUTEX_WAIT || cmd == FUTEX_LOCK_PI ||
- cmd == FUTEX_WAIT_BITSET ||
- cmd == FUTEX_WAIT_REQUEUE_PI)) {
- if (compat_get_timespec(&ts, utime))
- return -EFAULT;
- if (!timespec_valid(&ts))
- return -EINVAL;
-
- t = timespec_to_ktime(ts);
- if (cmd == FUTEX_WAIT)
- t = ktime_add_safe(ktime_get(), t);
- tp = &t;
- }
- if (cmd == FUTEX_REQUEUE || cmd == FUTEX_CMP_REQUEUE ||
- cmd == FUTEX_CMP_REQUEUE_PI || cmd == FUTEX_WAKE_OP)
- val2 = (int) (unsigned long) utime;
-
- return do_futex(uaddr, op, val, tp, uaddr2, val2, val3);
-}
--
2.18.0
^ permalink raw reply related
* [PATCH 2/5] y2038: futex: Add support for __kernel_timespec
From: Arnd Bergmann @ 2018-12-07 21:18 UTC (permalink / raw)
To: y2038
Cc: linux-kernel, Deepa Dinamani, John Stultz, Thomas Gleixner,
linux-api, Arnd Bergmann
In-Reply-To: <20181207211808.328719-1-arnd@arndb.de>
This prepares sys_futex for y2038 safe calling: the native
syscall is changed to receive a __kernel_timespec argument, which
will be switched to 64-bit time_t in the future. All the internal
time handling gets changed to timespec64, and the compat_sys_futex
entry point is moved under the CONFIG_COMPAT_32BIT_TIME check
to provide compatibility for existing 32-bit architectures.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
include/linux/syscalls.h | 2 +-
kernel/futex.c | 22 ++++++++++++----------
2 files changed, 13 insertions(+), 11 deletions(-)
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index a27cf407de92..247ad9eca955 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -553,7 +553,7 @@ asmlinkage long sys_unshare(unsigned long unshare_flags);
/* kernel/futex.c */
asmlinkage long sys_futex(u32 __user *uaddr, int op, u32 val,
- struct timespec __user *utime, u32 __user *uaddr2,
+ struct __kernel_timespec __user *utime, u32 __user *uaddr2,
u32 val3);
asmlinkage long sys_get_robust_list(int pid,
struct robust_list_head __user * __user *head_ptr,
diff --git a/kernel/futex.c b/kernel/futex.c
index 5cc7c3b098e9..b305beaab739 100644
--- a/kernel/futex.c
+++ b/kernel/futex.c
@@ -3558,10 +3558,10 @@ long do_futex(u32 __user *uaddr, int op, u32 val, ktime_t *timeout,
SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val,
- struct timespec __user *, utime, u32 __user *, uaddr2,
+ struct __kernel_timespec __user *, utime, u32 __user *, uaddr2,
u32, val3)
{
- struct timespec ts;
+ struct timespec64 ts;
ktime_t t, *tp = NULL;
u32 val2 = 0;
int cmd = op & FUTEX_CMD_MASK;
@@ -3571,12 +3571,12 @@ SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val,
cmd == FUTEX_WAIT_REQUEUE_PI)) {
if (unlikely(should_fail_futex(!(op & FUTEX_PRIVATE_FLAG))))
return -EFAULT;
- if (copy_from_user(&ts, utime, sizeof(ts)) != 0)
+ if (get_timespec64(&ts, utime))
return -EFAULT;
- if (!timespec_valid(&ts))
+ if (!timespec64_valid(&ts))
return -EINVAL;
- t = timespec_to_ktime(ts);
+ t = timespec64_to_ktime(ts);
if (cmd == FUTEX_WAIT)
t = ktime_add_safe(ktime_get(), t);
tp = &t;
@@ -3747,12 +3747,14 @@ COMPAT_SYSCALL_DEFINE3(get_robust_list, int, pid,
return ret;
}
+#endif /* CONFIG_COMPAT */
+#ifdef CONFIG_COMPAT_32BIT_TIME
COMPAT_SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val,
struct old_timespec32 __user *, utime, u32 __user *, uaddr2,
u32, val3)
{
- struct timespec ts;
+ struct timespec64 ts;
ktime_t t, *tp = NULL;
int val2 = 0;
int cmd = op & FUTEX_CMD_MASK;
@@ -3760,12 +3762,12 @@ COMPAT_SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val,
if (utime && (cmd == FUTEX_WAIT || cmd == FUTEX_LOCK_PI ||
cmd == FUTEX_WAIT_BITSET ||
cmd == FUTEX_WAIT_REQUEUE_PI)) {
- if (compat_get_timespec(&ts, utime))
+ if (get_old_timespec32(&ts, utime))
return -EFAULT;
- if (!timespec_valid(&ts))
+ if (!timespec64_valid(&ts))
return -EINVAL;
- t = timespec_to_ktime(ts);
+ t = timespec64_to_ktime(ts);
if (cmd == FUTEX_WAIT)
t = ktime_add_safe(ktime_get(), t);
tp = &t;
@@ -3776,7 +3778,7 @@ COMPAT_SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val,
return do_futex(uaddr, op, val, tp, uaddr2, val2, val3);
}
-#endif /* CONFIG_COMPAT */
+#endif /* CONFIG_COMPAT_32BIT_TIME */
static void __init futex_detect_cmpxchg(void)
{
--
2.18.0
^ permalink raw reply related
* [PATCH 3/5] y2038: socket: Add compat_sys_recvmmsg_time64
From: Arnd Bergmann @ 2018-12-07 21:18 UTC (permalink / raw)
To: y2038
Cc: linux-kernel, Deepa Dinamani, John Stultz, Thomas Gleixner,
linux-api, Arnd Bergmann
In-Reply-To: <20181207211808.328719-1-arnd@arndb.de>
recvmmsg() takes two arguments to pointers of structures that differ
between 32-bit and 64-bit architectures: mmsghdr and timespec.
For y2038 compatbility, we are changing the native system call from
timespec to __kernel_timespec with a 64-bit time_t (in another patch),
and use the existing compat system call on both 32-bit and 64-bit
architectures for compatibility with traditional 32-bit user space.
As we now have two variants of recvmmsg() for 32-bit tasks that are both
different from the variant that we use on 64-bit tasks, this means we
also require two compat system calls!
The solution I picked is to flip things around: The existing
compat_sys_recvmmsg() call gets moved from net/compat.c into net/socket.c
and now handles the case for old user space on all architectures that
have set CONFIG_COMPAT_32BIT_TIME. A new compat_sys_recvmmsg_time64()
call gets added in the old place for 64-bit architectures only, this
one handles the case of a compat mmsghdr structure combined with
__kernel_timespec.
In the indirect sys_socketcall(), we now need to call either
do_sys_recvmmsg() or __compat_sys_recvmmsg(), depending on what kind of
architecture we are on. For compat_sys_socketcall(), no such change is
needed, we always call __compat_sys_recvmmsg().
I decided to not add a new SYS_RECVMMSG_TIME64 socketcall: Any libc
implementation for 64-bit time_t will need significant changes including
an updated asm/unistd.h, and it seems better to consistently use the
separate syscalls that configuration, leaving the socketcall only for
backward compatibility with 32-bit time_t based libc.
The naming is asymmetric for the moment, so both existing syscalls
entry points keep their names, while the new ones are recvmmsg_time32
and compat_recvmmsg_time64 respectively. I expect that we will rename
the compat syscalls later as we start using generated syscall tables
everywhere and add these entry points.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
include/linux/compat.h | 5 +++-
include/linux/socket.h | 9 ++++--
include/linux/syscalls.h | 3 ++
kernel/sys_ni.c | 2 ++
net/compat.c | 34 ++++++++--------------
net/socket.c | 62 +++++++++++++++++++++++++++++-----------
6 files changed, 73 insertions(+), 42 deletions(-)
diff --git a/include/linux/compat.h b/include/linux/compat.h
index 8be8daa38c9a..72969d303434 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -893,7 +893,10 @@ asmlinkage long compat_sys_move_pages(pid_t pid, compat_ulong_t nr_pages,
asmlinkage long compat_sys_rt_tgsigqueueinfo(compat_pid_t tgid,
compat_pid_t pid, int sig,
struct compat_siginfo __user *uinfo);
-asmlinkage long compat_sys_recvmmsg(int fd, struct compat_mmsghdr __user *mmsg,
+asmlinkage long compat_sys_recvmmsg_time64(int fd, struct compat_mmsghdr __user *mmsg,
+ unsigned vlen, unsigned int flags,
+ struct __kernel_timespec __user *timeout);
+asmlinkage long compat_sys_recvmmsg(int fd, struct mmsghdr __user *mmsg,
unsigned vlen, unsigned int flags,
struct old_timespec32 __user *timeout);
asmlinkage long compat_sys_wait4(compat_pid_t pid,
diff --git a/include/linux/socket.h b/include/linux/socket.h
index 8b571e9b9f76..333b5df8a1b2 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -348,7 +348,8 @@ struct ucred {
extern int move_addr_to_kernel(void __user *uaddr, int ulen, struct sockaddr_storage *kaddr);
extern int put_cmsg(struct msghdr*, int level, int type, int len, void *data);
-struct timespec64;
+struct __kernel_timespec;
+struct old_timespec32;
/* The __sys_...msg variants allow MSG_CMSG_COMPAT iff
* forbid_cmsg_compat==false
@@ -357,8 +358,10 @@ extern long __sys_recvmsg(int fd, struct user_msghdr __user *msg,
unsigned int flags, bool forbid_cmsg_compat);
extern long __sys_sendmsg(int fd, struct user_msghdr __user *msg,
unsigned int flags, bool forbid_cmsg_compat);
-extern int __sys_recvmmsg(int fd, struct mmsghdr __user *mmsg, unsigned int vlen,
- unsigned int flags, struct timespec64 *timeout);
+extern int __sys_recvmmsg(int fd, struct mmsghdr __user *mmsg,
+ unsigned int vlen, unsigned int flags,
+ struct __kernel_timespec __user *timeout,
+ struct old_timespec32 __user *timeout32);
extern int __sys_sendmmsg(int fd, struct mmsghdr __user *mmsg,
unsigned int vlen, unsigned int flags,
bool forbid_cmsg_compat);
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 247ad9eca955..03cda6793be3 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -843,6 +843,9 @@ asmlinkage long sys_accept4(int, struct sockaddr __user *, int __user *, int);
asmlinkage long sys_recvmmsg(int fd, struct mmsghdr __user *msg,
unsigned int vlen, unsigned flags,
struct __kernel_timespec __user *timeout);
+asmlinkage long sys_recvmmsg_time32(int fd, struct mmsghdr __user *msg,
+ unsigned int vlen, unsigned flags,
+ struct old_timespec32 __user *timeout);
asmlinkage long sys_wait4(pid_t pid, int __user *stat_addr,
int options, struct rusage __user *ru);
diff --git a/kernel/sys_ni.c b/kernel/sys_ni.c
index df556175be50..ab9d0e3c6d50 100644
--- a/kernel/sys_ni.c
+++ b/kernel/sys_ni.c
@@ -284,7 +284,9 @@ COND_SYSCALL_COMPAT(move_pages);
COND_SYSCALL(perf_event_open);
COND_SYSCALL(accept4);
COND_SYSCALL(recvmmsg);
+COND_SYSCALL(recvmmsg_time32);
COND_SYSCALL_COMPAT(recvmmsg);
+COND_SYSCALL_COMPAT(recvmmsg_time64);
/*
* Architecture specific syscalls: see further below
diff --git a/net/compat.c b/net/compat.c
index 47a614b370cd..f7084780a8f8 100644
--- a/net/compat.c
+++ b/net/compat.c
@@ -810,34 +810,23 @@ COMPAT_SYSCALL_DEFINE6(recvfrom, int, fd, void __user *, buf, compat_size_t, len
return __compat_sys_recvfrom(fd, buf, len, flags, addr, addrlen);
}
-static int __compat_sys_recvmmsg(int fd, struct compat_mmsghdr __user *mmsg,
- unsigned int vlen, unsigned int flags,
- struct old_timespec32 __user *timeout)
+COMPAT_SYSCALL_DEFINE5(recvmmsg_time64, int, fd, struct compat_mmsghdr __user *, mmsg,
+ unsigned int, vlen, unsigned int, flags,
+ struct __kernel_timespec __user *, timeout)
{
- int datagrams;
- struct timespec64 ktspec;
-
- if (timeout == NULL)
- return __sys_recvmmsg(fd, (struct mmsghdr __user *)mmsg, vlen,
- flags | MSG_CMSG_COMPAT, NULL);
-
- if (compat_get_timespec64(&ktspec, timeout))
- return -EFAULT;
-
- datagrams = __sys_recvmmsg(fd, (struct mmsghdr __user *)mmsg, vlen,
- flags | MSG_CMSG_COMPAT, &ktspec);
- if (datagrams > 0 && compat_put_timespec64(&ktspec, timeout))
- datagrams = -EFAULT;
-
- return datagrams;
+ return __sys_recvmmsg(fd, (struct mmsghdr __user *)mmsg, vlen,
+ flags | MSG_CMSG_COMPAT, timeout, NULL);
}
+#ifdef CONFIG_COMPAT_32BIT_TIME
COMPAT_SYSCALL_DEFINE5(recvmmsg, int, fd, struct compat_mmsghdr __user *, mmsg,
unsigned int, vlen, unsigned int, flags,
struct old_timespec32 __user *, timeout)
{
- return __compat_sys_recvmmsg(fd, mmsg, vlen, flags, timeout);
+ return __sys_recvmmsg(fd, (struct mmsghdr __user *)mmsg, vlen,
+ flags | MSG_CMSG_COMPAT, NULL, timeout);
}
+#endif
COMPAT_SYSCALL_DEFINE2(socketcall, int, call, u32 __user *, args)
{
@@ -925,8 +914,9 @@ COMPAT_SYSCALL_DEFINE2(socketcall, int, call, u32 __user *, args)
ret = __compat_sys_recvmsg(a0, compat_ptr(a1), a[2]);
break;
case SYS_RECVMMSG:
- ret = __compat_sys_recvmmsg(a0, compat_ptr(a1), a[2], a[3],
- compat_ptr(a[4]));
+ ret = __sys_recvmmsg(a0, compat_ptr(a1), a[2],
+ a[3] | MSG_CMSG_COMPAT, NULL,
+ compat_ptr(a[4]));
break;
case SYS_ACCEPT4:
ret = __sys_accept4(a0, compat_ptr(a1), compat_ptr(a[2]), a[3]);
diff --git a/net/socket.c b/net/socket.c
index 593826e11a53..f6a2d3a7b611 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -2341,8 +2341,9 @@ SYSCALL_DEFINE3(recvmsg, int, fd, struct user_msghdr __user *, msg,
* Linux recvmmsg interface
*/
-int __sys_recvmmsg(int fd, struct mmsghdr __user *mmsg, unsigned int vlen,
- unsigned int flags, struct timespec64 *timeout)
+static int do_recvmmsg(int fd, struct mmsghdr __user *mmsg,
+ unsigned int vlen, unsigned int flags,
+ struct timespec64 *timeout)
{
int fput_needed, err, datagrams;
struct socket *sock;
@@ -2451,25 +2452,32 @@ int __sys_recvmmsg(int fd, struct mmsghdr __user *mmsg, unsigned int vlen,
return datagrams;
}
-static int do_sys_recvmmsg(int fd, struct mmsghdr __user *mmsg,
- unsigned int vlen, unsigned int flags,
- struct __kernel_timespec __user *timeout)
+int __sys_recvmmsg(int fd, struct mmsghdr __user *mmsg,
+ unsigned int vlen, unsigned int flags,
+ struct __kernel_timespec __user *timeout,
+ struct old_timespec32 __user *timeout32)
{
int datagrams;
struct timespec64 timeout_sys;
- if (flags & MSG_CMSG_COMPAT)
- return -EINVAL;
-
- if (!timeout)
- return __sys_recvmmsg(fd, mmsg, vlen, flags, NULL);
+ if (timeout && get_timespec64(&timeout_sys, timeout))
+ return -EFAULT;
- if (get_timespec64(&timeout_sys, timeout))
+ if (timeout32 && get_old_timespec32(&timeout_sys, timeout32))
return -EFAULT;
- datagrams = __sys_recvmmsg(fd, mmsg, vlen, flags, &timeout_sys);
+ if (!timeout && !timeout32)
+ do_recvmmsg(fd, mmsg, vlen, flags, NULL);
+
+ datagrams = do_recvmmsg(fd, mmsg, vlen, flags, &timeout_sys);
- if (datagrams > 0 && put_timespec64(&timeout_sys, timeout))
+ if (!datagrams)
+ return 0;
+
+ if (timeout && put_timespec64(&timeout_sys, timeout))
+ datagrams = -EFAULT;
+
+ if (timeout32 && put_old_timespec32(&timeout_sys, timeout32))
datagrams = -EFAULT;
return datagrams;
@@ -2479,8 +2487,23 @@ SYSCALL_DEFINE5(recvmmsg, int, fd, struct mmsghdr __user *, mmsg,
unsigned int, vlen, unsigned int, flags,
struct __kernel_timespec __user *, timeout)
{
- return do_sys_recvmmsg(fd, mmsg, vlen, flags, timeout);
+ if (flags & MSG_CMSG_COMPAT)
+ return -EINVAL;
+
+ return __sys_recvmmsg(fd, mmsg, vlen, flags, timeout, NULL);
+}
+
+#ifdef CONFIG_COMPAT_32BIT_TIME
+SYSCALL_DEFINE5(recvmmsg_time32, int, fd, struct mmsghdr __user *, mmsg,
+ unsigned int, vlen, unsigned int, flags,
+ struct old_timespec32 __user *, timeout)
+{
+ if (flags & MSG_CMSG_COMPAT)
+ return -EINVAL;
+
+ return __sys_recvmmsg(fd, mmsg, vlen, flags, NULL, timeout);
}
+#endif
#ifdef __ARCH_WANT_SYS_SOCKETCALL
/* Argument list sizes for sys_socketcall */
@@ -2600,8 +2623,15 @@ SYSCALL_DEFINE2(socketcall, int, call, unsigned long __user *, args)
a[2], true);
break;
case SYS_RECVMMSG:
- err = do_sys_recvmmsg(a0, (struct mmsghdr __user *)a1, a[2],
- a[3], (struct __kernel_timespec __user *)a[4]);
+ if (IS_ENABLED(CONFIG_64BIT) || !IS_ENABLED(CONFIG_64BIT_TIME))
+ err = __sys_recvmmsg(a0, (struct mmsghdr __user *)a1,
+ a[2], a[3],
+ (struct __kernel_timespec __user *)a[4],
+ NULL);
+ else
+ err = __sys_recvmmsg(a0, (struct mmsghdr __user *)a1,
+ a[2], a[3], NULL,
+ (struct old_timespec32 __user *)a[4]);
break;
case SYS_ACCEPT4:
err = __sys_accept4(a0, (struct sockaddr __user *)a1,
--
2.18.0
^ permalink raw reply related
* [PATCH 4/5] y2038: signal: Add sys_rt_sigtimedwait_time32
From: Arnd Bergmann @ 2018-12-07 21:18 UTC (permalink / raw)
To: y2038
Cc: linux-kernel, Deepa Dinamani, John Stultz, Thomas Gleixner,
linux-api, Arnd Bergmann
In-Reply-To: <20181207211808.328719-1-arnd@arndb.de>
Once sys_rt_sigtimedwait() gets changed to a 64-bit time_t, we have
to provide compatibility support for existing binaries.
An earlier version of this patch reused the compat_sys_rt_sigtimedwait
entry point to avoid code duplication, but this newer approach
duplicates the existing native entry point instead, which seems
a bit cleaner.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
include/linux/syscalls.h | 4 ++++
kernel/signal.c | 33 +++++++++++++++++++++++++++++++++
2 files changed, 37 insertions(+)
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 03cda6793be3..251979d2e709 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -649,6 +649,10 @@ asmlinkage long sys_rt_sigtimedwait(const sigset_t __user *uthese,
siginfo_t __user *uinfo,
const struct __kernel_timespec __user *uts,
size_t sigsetsize);
+asmlinkage long sys_rt_sigtimedwait_time32(const sigset_t __user *uthese,
+ siginfo_t __user *uinfo,
+ const struct old_timespec32 __user *uts,
+ size_t sigsetsize);
asmlinkage long sys_rt_sigqueueinfo(pid_t pid, int sig, siginfo_t __user *uinfo);
/* kernel/sys.c */
diff --git a/kernel/signal.c b/kernel/signal.c
index 3c8ea7a328e0..be6744cd0a11 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3332,6 +3332,39 @@ SYSCALL_DEFINE4(rt_sigtimedwait, const sigset_t __user *, uthese,
return ret;
}
+#ifdef CONFIG_COMPAT_32BIT_TIME
+SYSCALL_DEFINE4(rt_sigtimedwait_time32, const sigset_t __user *, uthese,
+ siginfo_t __user *, uinfo,
+ const struct old_timespec32 __user *, uts,
+ size_t, sigsetsize)
+{
+ sigset_t these;
+ struct timespec64 ts;
+ kernel_siginfo_t info;
+ int ret;
+
+ if (sigsetsize != sizeof(sigset_t))
+ return -EINVAL;
+
+ if (copy_from_user(&these, uthese, sizeof(these)))
+ return -EFAULT;
+
+ if (uts) {
+ if (get_old_timespec32(&ts, uts))
+ return -EFAULT;
+ }
+
+ ret = do_sigtimedwait(&these, &info, uts ? &ts : NULL);
+
+ if (ret > 0 && uinfo) {
+ if (copy_siginfo_to_user(uinfo, &info))
+ ret = -EFAULT;
+ }
+
+ return ret;
+}
+#endif
+
#ifdef CONFIG_COMPAT
COMPAT_SYSCALL_DEFINE4(rt_sigtimedwait, compat_sigset_t __user *, uthese,
struct compat_siginfo __user *, uinfo,
--
2.18.0
^ permalink raw reply related
* [PATCH 5/5] y2038: signal: Add compat_sys_rt_sigtimedwait_time64
From: Arnd Bergmann @ 2018-12-07 21:18 UTC (permalink / raw)
To: y2038
Cc: linux-kernel, Deepa Dinamani, John Stultz, Thomas Gleixner,
linux-api, Arnd Bergmann
In-Reply-To: <20181207211808.328719-1-arnd@arndb.de>
Now that 32-bit architectures have two variants of sys_rt_sigtimedwaid()
for 32-bit and 64-bit time_t, we also need to have a second compat system
call entry point on the corresponding 64-bit architectures.
The traditional system call keeps getting handled
by compat_sys_rt_sigtimedwait(), and this adds a new
compat_sys_rt_sigtimedwait_time64() that differs only in the timeout
argument type.
The naming remains a bit asymmetric for the moment. Ideally we would
want to have compat_sys_rt_sigtimedwait_time32() for the old version
and compat_sys_rt_sigtimedwait() for the new one to mirror the names
of the native entry points, but renaming the existing system call
tables causes unnecessary churn. I would suggest renaming all such
system calls together at a later point.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
include/linux/compat.h | 3 +++
kernel/signal.c | 32 ++++++++++++++++++++++++++++++++
2 files changed, 35 insertions(+)
diff --git a/include/linux/compat.h b/include/linux/compat.h
index 72969d303434..3bc5f218efd8 100644
--- a/include/linux/compat.h
+++ b/include/linux/compat.h
@@ -788,6 +788,9 @@ asmlinkage long compat_sys_rt_sigpending(compat_sigset_t __user *uset,
asmlinkage long compat_sys_rt_sigtimedwait(compat_sigset_t __user *uthese,
struct compat_siginfo __user *uinfo,
struct old_timespec32 __user *uts, compat_size_t sigsetsize);
+asmlinkage long compat_sys_rt_sigtimedwait_time64(compat_sigset_t __user *uthese,
+ struct compat_siginfo __user *uinfo,
+ struct __kernel_timespec __user *uts, compat_size_t sigsetsize);
asmlinkage long compat_sys_rt_sigqueueinfo(compat_pid_t pid, int sig,
struct compat_siginfo __user *uinfo);
/* No generic prototype for rt_sigreturn */
diff --git a/kernel/signal.c b/kernel/signal.c
index be6744cd0a11..53e07d97ffe0 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -3366,6 +3366,37 @@ SYSCALL_DEFINE4(rt_sigtimedwait_time32, const sigset_t __user *, uthese,
#endif
#ifdef CONFIG_COMPAT
+COMPAT_SYSCALL_DEFINE4(rt_sigtimedwait_time64, compat_sigset_t __user *, uthese,
+ struct compat_siginfo __user *, uinfo,
+ struct __kernel_timespec __user *, uts, compat_size_t, sigsetsize)
+{
+ sigset_t s;
+ struct timespec64 t;
+ kernel_siginfo_t info;
+ long ret;
+
+ if (sigsetsize != sizeof(sigset_t))
+ return -EINVAL;
+
+ if (get_compat_sigset(&s, uthese))
+ return -EFAULT;
+
+ if (uts) {
+ if (get_timespec64(&t, uts))
+ return -EFAULT;
+ }
+
+ ret = do_sigtimedwait(&s, &info, uts ? &t : NULL);
+
+ if (ret > 0 && uinfo) {
+ if (copy_siginfo_to_user32(uinfo, &info))
+ ret = -EFAULT;
+ }
+
+ return ret;
+}
+
+#ifdef CONFIG_COMPAT_32BIT_TIME
COMPAT_SYSCALL_DEFINE4(rt_sigtimedwait, compat_sigset_t __user *, uthese,
struct compat_siginfo __user *, uinfo,
struct old_timespec32 __user *, uts, compat_size_t, sigsetsize)
@@ -3396,6 +3427,7 @@ COMPAT_SYSCALL_DEFINE4(rt_sigtimedwait, compat_sigset_t __user *, uthese,
return ret;
}
#endif
+#endif
/**
* sys_kill - send a signal to a process
--
2.18.0
^ permalink raw reply related
* [PATCH v5 0/1] signaling processes through pidfds
From: Christian Brauner @ 2018-12-08 5:40 UTC (permalink / raw)
To: linux-kernel, linux-api, luto, arnd, ebiederm, serge, keescook
Cc: jannh, akpm, oleg, cyphar, viro, linux-fsdevel, dancol, timmurray,
fweimer, tglx, x86, Christian Brauner
Hey everyone,
This is v5 of this patchset.
v5 does not introduce any functional changes since none were requested
or required in the thread. Instead, it focusses on updated documentation
making it very clear what the intentions are how to extend this syscall.
Eric, I dragged Serge into this and we went through the mails and tried
to very thoroughly address your concerns about whether the width of the
target should depend on flags or file descriptor types. As far as we
understand from the threads this was your ultimate worry and also the
reason why you withheld your agreement to the name of the syscall.
We have outlined how the syscall is intended to be extended and decided
that flags (e.g. PIDFD_TYPE_TID, PIDFD_TYPE_PGID) are the way to go. In
line with this we decided to accept "pidfd_" as prefix for the new
syscall.
All concerns we could identify and understand we tried to address. I
hope this will be sufficient for you to get behind the patch.
The relevant section in the commit message is titled:
/* sending signals to threads (tid) and process groups (pgid) */
Thanks!
Christian
Christian Brauner (1):
signal: add pidfd_send_signal() syscall
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
fs/proc/base.c | 20 +++-
include/linux/proc_fs.h | 12 +++
include/linux/syscalls.h | 3 +
include/uapi/asm-generic/unistd.h | 4 +-
kernel/signal.c | 141 +++++++++++++++++++++++--
7 files changed, 173 insertions(+), 9 deletions(-)
--
2.19.1
^ permalink raw reply
* [PATCH v5 1/1] signal: add pidfd_send_signal() syscall
From: Christian Brauner @ 2018-12-08 5:40 UTC (permalink / raw)
To: linux-kernel, linux-api, luto, arnd, ebiederm, serge, keescook
Cc: jannh, akpm, oleg, cyphar, viro, linux-fsdevel, dancol, timmurray,
fweimer, tglx, x86, Christian Brauner
In-Reply-To: <20181208054059.19813-1-christian@brauner.io>
The kill() syscall operates on process identifiers (pid). 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 to address this problem [1].
This patch uses file descriptors (fd) from proc/<pid> as stable handles on
struct pid. Even if a pid is recycled the handle will not change. The fd
can be used to send signals to the process it refers to.
Thus, the new syscall pidfd_send_signal() is introduced to solve this
problem. Instead of pids it operates on process fds (pidfd).
/* prototype and argument /*
long pidfd_send_signal(int pidfd, int sig, siginfo_t *info, unsigned int flags);
In addition to the pidfd and signal argument it takes an additional
siginfo_t and flags argument. If the siginfo_t argument is NULL then
pidfd_send_signal() is equivalent to kill(<positive-pid>, <signal>). If it
is not NULL pidfd_send_signal() is equivalent to rt_sigqueueinfo().
The flags argument is added to allow for future extensions of this syscall.
It currently needs to be passed as 0. Failing to do so will cause EINVAL.
/* pidfd_send_signal() replaces multiple pid-based syscalls */
The pidfd_send_signal() syscall currently takes on the job of
rt_sigqueueinfo(2) and parts of the functionality of kill(2), Namely, when a
positive pid is passed to kill(2). It will however be possible to also
replace tgkill(2) and rt_tgsigqueueinfo(2) if this syscall is extended.
/* sending signals to threads (tid) and process groups (pgid) */
Specifically, the pidfd_send_signal() syscall does currently not operate on
process groups or threads. This is left for future extensions.
In order to extend the syscall to allow sending signal to threads and
process groups appropriately named flags (e.g. PIDFD_TYPE_PGID, and
PIDFD_TYPE_TID) should be added. This implies that the flags argument will
determine what is signaled and not the file descriptor itself. Put in other
words, grouping in this api is a property of the flags argument not a
property of the file descriptor (cf. [13]).
When appropriate extensions through the flags argument are added then
pidfd_send_signal() can additionally replace the part of kill(2) which
operates on process groups as well as the tgkill(2) and
rt_tgsigqueueinfo(2) syscalls.
How such an extension could be implemented has been very roughly sketched
in [14], [15], and [16]. However, this should not be taken as a commitment
to a particular implementation. There might be better ways to do it.
Right now this is intentionally left out to keep this patchset as simple as
possible (cf. [4]). For example, if a pidfd for a tid from
/proc/<pid>/task/<tid> is passed EOPNOTSUPP will be returned to give
userspace a way to detect when I add support for signaling to threads (cf. [10]).
/* naming */
The syscall had various names throughout iterations of this patchset:
- procfd_signal()
- procfd_send_signal()
- taskfd_send_signal()
In the last round of reviews it was pointed out that given that if the
flags argument decides the scope of the signal instead of different types
of fds it might make sense to either settle for "procfd_" or "pidfd_" as
prefix. The community was willing to accept either (cf. [17] and [18]).
Given that one developer expressed strong preference for the "pidfd_"
prefix (cf. [13] and with other developers less opinionated about the name
we should settle for "pidfd_" to avoid further bikeshedding.
The "_send_signal" suffix was chosen to reflect the fact that the syscall
takes on the job of multiple syscalls. It is therefore intentional that the
name is not reminiscent of neither kill(2) nor rt_sigqueueinfo(2). Not the
fomer because it might imply that pidfd_send_signal() is a replacement for
kill(2), and not the latter because it is a hassle to remember the correct
spelling - especially for non-native speakers - and because it is not
descriptive enough of what the syscall actually does. The name
"pidfd_send_signal" makes it very clear that its job is to send signals.
/* O_PATH file descriptors */
pidfds opened as O_PATH fds cannot be used to send signals to a process
(cf. [2]). Signaling processes through pidfds is the equivalent of writing
to a file. Thus, this is not an operation that operates "purely at the file
descriptor level" as required by the open(2) manpage.
/* zombies */
Zombies can be signaled just as any other process. No special error will be
reported since a zombie state is an unreliable state (cf. [3]). However,
this can be added as an extension through the @flags argument if the need
ever arises.
/* cross-namespace signals */
The patch currently enforces that the signaler and signalee either are in
the same pid namespace or that the signaler's pid namespace is an ancestor
of the signalee's pid namespace. This is done for the sake of simplicity
and because it is unclear to what values certain members of struct
siginfo_t would need to be set to (cf. [5], [6]).
/* compat syscalls */
It became clear that we would like to avoid adding compat syscalls
(cf. [7]). The compat syscall handling is now done in kernel/signal.c
itself by adding __copy_siginfo_from_user_generic() which lets us avoid
compat syscalls (cf. [8]). It should be noted that the addition of
__copy_siginfo_from_user_any() is caused by a bug in the original
implementation of rt_sigqueueinfo(2) (cf. 12).
With upcoming rework for syscall handling things might improve
significantly (cf. [11]) and __copy_siginfo_from_user_any() will not gain
any additional callers.
/* testing */
This patch was tested on x64 and x86.
/* userspace usage */
An asciinema recording for the basic functionality can be found under [9].
With this patch a process can be killed via:
#define _GNU_SOURCE
#include <errno.h>
#include <fcntl.h>
#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/stat.h>
#include <sys/syscall.h>
#include <sys/types.h>
#include <unistd.h>
static inline int do_pidfd_send_signal(int pidfd, int sig, siginfo_t *info,
unsigned int flags)
{
#ifdef __NR_pidfd_send_signal
return syscall(__NR_pidfd_send_signal, pidfd, sig, info, flags);
#else
return -ENOSYS;
#endif
}
int main(int argc, char *argv[])
{
int fd, ret, saved_errno, sig;
if (argc < 3)
exit(EXIT_FAILURE);
fd = open(argv[1], O_DIRECTORY | O_CLOEXEC);
if (fd < 0) {
printf("%s - Failed to open \"%s\"\n", strerror(errno), argv[1]);
exit(EXIT_FAILURE);
}
sig = atoi(argv[2]);
printf("Sending signal %d to process %s\n", sig, argv[1]);
ret = do_pidfd_send_signal(fd, sig, NULL, 0);
saved_errno = errno;
close(fd);
errno = saved_errno;
if (ret < 0) {
printf("%s - Failed to send signal %d to process %s\n",
strerror(errno), sig, argv[1]);
exit(EXIT_FAILURE);
}
exit(EXIT_SUCCESS);
}
[1]: https://lore.kernel.org/lkml/20181029221037.87724-1-dancol@google.com/
[2]: https://lore.kernel.org/lkml/874lbtjvtd.fsf@oldenburg2.str.redhat.com/
[3]: https://lore.kernel.org/lkml/20181204132604.aspfupwjgjx6fhva@brauner.io/
[4]: https://lore.kernel.org/lkml/20181203180224.fkvw4kajtbvru2ku@brauner.io/
[5]: https://lore.kernel.org/lkml/20181121213946.GA10795@mail.hallyn.com/
[6]: https://lore.kernel.org/lkml/20181120103111.etlqp7zop34v6nv4@brauner.io/
[7]: https://lore.kernel.org/lkml/36323361-90BD-41AF-AB5B-EE0D7BA02C21@amacapital.net/
[8]: https://lore.kernel.org/lkml/87tvjxp8pc.fsf@xmission.com/
[9]: https://asciinema.org/a/IQjuCHew6bnq1cr78yuMv16cy
[10]: https://lore.kernel.org/lkml/20181203180224.fkvw4kajtbvru2ku@brauner.io/
[11]: https://lore.kernel.org/lkml/F53D6D38-3521-4C20-9034-5AF447DF62FF@amacapital.net/
[12]: https://lore.kernel.org/lkml/87zhtjn8ck.fsf@xmission.com/
[13]: https://lore.kernel.org/lkml/871s6u9z6u.fsf@xmission.com/
[14]: https://lore.kernel.org/lkml/20181206231742.xxi4ghn24z4h2qki@brauner.io/
[15]: https://lore.kernel.org/lkml/20181207003124.GA11160@mail.hallyn.com/
[16]: https://lore.kernel.org/lkml/20181207015423.4miorx43l3qhppfz@brauner.io/
[17]: https://lore.kernel.org/lkml/CAGXu5jL8PciZAXvOvCeCU3wKUEB_dU-O3q0tDw4uB_ojMvDEew@mail.gmail.com/
[18]: https://lore.kernel.org/lkml/20181206222746.GB9224@mail.hallyn.com/
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Cc: Jann Horn <jannh@google.com>
Cc: Andy Lutomirsky <luto@kernel.org>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Oleg Nesterov <oleg@redhat.com>
Cc: Al Viro <viro@zeniv.linux.org.uk>
Cc: Florian Weimer <fweimer@redhat.com>
Signed-off-by: Christian Brauner <christian@brauner.io>
Reviewed-by: Kees Cook <keescook@chromium.org>
Acked-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Serge Hallyn <serge@hallyn.com>
Acked-by: Aleksa Sarai <cyphar@cyphar.com>
---
Changelog:
v5:
- s/may_signal_taskfd/access_taskfd_pidns/g
- make it clear that process grouping is a property of the @flags argument
Eric has argued that he would like to know when we add thread and process
group signal support whether grouping will be a property of the file
descriptor or the flag argument and he would oppose this until a
commitment has been made. It seems that the cleanest strategy is to make
grouping a property of the @flags argument.
He also argued that in this case the prefix of the syscall should be
"pidfd_" (cf. https://lore.kernel.org/lkml/871s6u9z6u.fsf@xmission.com/).
- use "pidfd_" as prefix for the syscall since grouping will be a property
of the @flags argument
- substantial rewrite of the commit message to reflect the discussion
v4:
- updated asciinema to use "taskfd_" prefix
- s/procfd_send_signal/taskfd_send_signal/g
- s/proc_is_tgid_procfd/tgid_taskfd_to_pid/b
- s/proc_is_tid_procfd/tid_taskfd_to_pid/b
- s/__copy_siginfo_from_user_generic/__copy_siginfo_from_user_any/g
- make it clear that __copy_siginfo_from_user_any() is a workaround caused
by a bug in the original implementation of rt_sigqueueinfo()
- when spoofing signals turn them into regular kill signals if si_code is
set to SI_USER
- make proc_is_t{g}id_procfd() return struct pid to allow proc_pid() to
stay private to fs/proc/
v3:
- add __copy_siginfo_from_user_generic() to avoid adding compat syscalls
- s/procfd_signal/procfd_send_signal/g
- change type of flags argument from int to unsigned int
- add comment about what happens to zombies
- add proc_is_tid_procfd()
- return EOPNOTSUPP when /proc/<pid>/task/<tid> fd is passed so userspace
has a way of knowing that tidfds are not supported currently.
v2:
- define __NR_procfd_signal in unistd.h
- wire up compat syscall
- s/proc_is_procfd/proc_is_tgid_procfd/g
- provide stubs when CONFIG_PROC_FS=n
- move proc_pid() to linux/proc_fs.h header
- use proc_pid() to grab struct pid from /proc/<pid> fd
v1:
- patch introduced
---
arch/x86/entry/syscalls/syscall_32.tbl | 1 +
arch/x86/entry/syscalls/syscall_64.tbl | 1 +
fs/proc/base.c | 20 +++-
include/linux/proc_fs.h | 12 +++
include/linux/syscalls.h | 3 +
include/uapi/asm-generic/unistd.h | 4 +-
kernel/signal.c | 141 +++++++++++++++++++++++--
7 files changed, 173 insertions(+), 9 deletions(-)
diff --git a/arch/x86/entry/syscalls/syscall_32.tbl b/arch/x86/entry/syscalls/syscall_32.tbl
index 3cf7b533b3d1..6804c1e84b36 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 pidfd_send_signal sys_pidfd_send_signal __ia32_sys_pidfd_send_signal
diff --git a/arch/x86/entry/syscalls/syscall_64.tbl b/arch/x86/entry/syscalls/syscall_64.tbl
index f0b1709a5ffb..aa4b858fa0f1 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 pidfd_send_signal __x64_sys_pidfd_send_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 ce3465479447..bf680b7b603a 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -716,8 +716,6 @@ static int proc_pid_permission(struct inode *inode, int mask)
return generic_permission(inode, mask);
}
-
-
static const struct inode_operations proc_def_inode_operations = {
.setattr = proc_setattr,
};
@@ -3038,6 +3036,15 @@ static const struct file_operations proc_tgid_base_operations = {
.llseek = generic_file_llseek,
};
+struct pid *tgid_pidfd_to_pid(const struct file *file)
+{
+ if (!d_is_dir(file->f_path.dentry) ||
+ (file->f_op != &proc_tgid_base_operations))
+ return ERR_PTR(-EBADF);
+
+ return proc_pid(file_inode(file));
+}
+
static struct dentry *proc_tgid_base_lookup(struct inode *dir, struct dentry *dentry, unsigned int flags)
{
return proc_pident_lookup(dir, dentry,
@@ -3422,6 +3429,15 @@ static const struct file_operations proc_tid_base_operations = {
.llseek = generic_file_llseek,
};
+struct pid *tid_pidfd_to_pid(const struct file *file)
+{
+ if (!d_is_dir(file->f_path.dentry) ||
+ (file->f_op != &proc_tid_base_operations))
+ return ERR_PTR(-EBADF);
+
+ return proc_pid(file_inode(file));
+}
+
static const struct inode_operations proc_tid_base_inode_operations = {
.lookup = proc_tid_base_lookup,
.getattr = pid_getattr,
diff --git a/include/linux/proc_fs.h b/include/linux/proc_fs.h
index d0e1f1522a78..eb150e5c0ab8 100644
--- a/include/linux/proc_fs.h
+++ b/include/linux/proc_fs.h
@@ -73,6 +73,8 @@ 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 struct pid *tgid_pidfd_to_pid(const struct file *file);
+extern struct pid *tid_pidfd_to_pid(const struct file *file);
#else /* CONFIG_PROC_FS */
@@ -114,6 +116,16 @@ static inline int remove_proc_subtree(const char *name, struct proc_dir_entry *p
#define proc_create_net(name, mode, parent, state_size, ops) ({NULL;})
#define proc_create_net_single(name, mode, parent, show, data) ({NULL;})
+static inline struct pid *tgid_pidfd_to_pid(const struct file *file)
+{
+ return ERR_PTR(-EBADF);
+}
+
+static inline struct pid *tid_pidfd_to_pid(const struct file *file)
+{
+ return ERR_PTR(-EBADF);
+}
+
#endif /* CONFIG_PROC_FS */
struct net;
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 2ac3d13a915b..fd85b9045a9f 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -907,6 +907,9 @@ 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_pidfd_send_signal(int pidfd, int sig,
+ siginfo_t __user *info,
+ unsigned int flags);
/*
* Architecture-specific system calls
diff --git a/include/uapi/asm-generic/unistd.h b/include/uapi/asm-generic/unistd.h
index 538546edbfbd..0822abc5927a 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -738,9 +738,11 @@ __SYSCALL(__NR_statx, sys_statx)
__SC_COMP(__NR_io_pgetevents, sys_io_pgetevents, compat_sys_io_pgetevents)
#define __NR_rseq 293
__SYSCALL(__NR_rseq, sys_rseq)
+#define __NR_pidfd_send_signal 294
+__SYSCALL(__NR_pidfd_send_signal, sys_pidfd_send_signal)
#undef __NR_syscalls
-#define __NR_syscalls 294
+#define __NR_syscalls 295
/*
* 32 bit systems traditionally used different
diff --git a/kernel/signal.c b/kernel/signal.c
index 9a32bc2088c9..3c83d3a5c7c5 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,133 @@ 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);
}
+/*
+ * Verify that the signaler and signalee either are in the same pid namespace
+ * or that the signaler's pid namespace is an ancestor of the signalee's pid
+ * namespace.
+ */
+static bool access_pidfd_pidns(struct pid *pid)
+{
+ struct pid_namespace *active = task_active_pid_ns(current);
+ struct pid_namespace *p = ns_of_pid(pid);
+
+ for (;;) {
+ if (!p)
+ return false;
+ if (p == active)
+ break;
+ p = p->parent;
+ }
+
+ return true;
+}
+
+static int copy_siginfo_from_user_any(kernel_siginfo_t *kinfo, siginfo_t *info)
+{
+#ifdef CONFIG_COMPAT
+ /*
+ * Avoid hooking up compat syscalls and instead handle necessary
+ * conversions here. Note, this is a stop-gap measure and should not be
+ * considered a generic solution.
+ */
+ if (in_compat_syscall())
+ return copy_siginfo_from_user32(
+ kinfo, (struct compat_siginfo __user *)info);
+#endif
+ return copy_siginfo_from_user(kinfo, info);
+}
+
+/**
+ * sys_pidfd_send_signal - send a signal to a process through a task file
+ * descriptor
+ * @pidfd: the file descriptor of the process
+ * @sig: signal to be sent
+ * @info: the signal info
+ * @flags: future flags to be passed
+ *
+ * The syscall currently only signals via PIDTYPE_PID which covers
+ * kill(<positive-pid>, <signal>. It does not signal threads or process
+ * groups.
+ * In order to extend the syscall to threads and process groups the @flags
+ * argument should be used. In essence, the @flags argument will determine
+ * what is signaled and not the file descriptor itself. Put in other words,
+ * grouping is a property of the flags argument not a property of the file
+ * descriptor.
+ *
+ * Return: 0 on success, negative errno on failure
+ */
+SYSCALL_DEFINE4(pidfd_send_signal, int, pidfd, int, sig,
+ siginfo_t __user *, info, unsigned int, flags)
+{
+ int ret;
+ struct fd f;
+ struct pid *pid;
+ kernel_siginfo_t kinfo;
+
+ /* Enforce flags be set to 0 until we add an extension. */
+ if (flags)
+ return -EINVAL;
+
+ f = fdget_raw(pidfd);
+ if (!f.file)
+ return -EBADF;
+
+ pid = tid_pidfd_to_pid(f.file);
+ if (!IS_ERR(pid)) {
+ /*
+ * Give userspace a way to detect /proc/<pid>/task/<tid>
+ * support when we add it.
+ */
+ ret = -EOPNOTSUPP;
+ goto err;
+ }
+
+ /* Is this a pidfd? */
+ pid = tgid_pidfd_to_pid(f.file);
+ if (IS_ERR(pid)) {
+ ret = PTR_ERR(pid);
+ goto err;
+ }
+
+ ret = -EINVAL;
+ if (!access_pidfd_pidns(pid))
+ goto err;
+
+ if (info) {
+ ret = copy_siginfo_from_user_any(&kinfo, info);
+ if (unlikely(ret))
+ goto err;
+
+ ret = -EINVAL;
+ if (unlikely(sig != kinfo.si_signo))
+ goto err;
+
+ if ((task_pid(current) != pid) &&
+ (kinfo.si_code >= 0 || kinfo.si_code == SI_TKILL)) {
+ /* Only allow sending arbitrary signals to yourself. */
+ ret = -EPERM;
+ if (kinfo.si_code != SI_USER)
+ goto err;
+
+ /* Turn this into a regular kill signal. */
+ prepare_kill_siginfo(sig, &kinfo);
+ }
+ } 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
* [GIT PULL] asm-generic: bugfix for asm/unistd.h
From: Arnd Bergmann @ 2018-12-08 16:01 UTC (permalink / raw)
To: Linus Torvalds
Cc: Guo Ren, Marcin Juszkiewicz, Linux Kernel Mailing List, Linux API
The following changes since commit 2e6e902d185027f8e3cb8b7305238f7e35d6a436:
Linux 4.20-rc4 (2018-11-25 14:19:31 -0800)
are available in the Git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/arnd/asm-generic.git
tags/asm-generic-4.20
for you to fetch changes up to b7d624ab431227af376787148cd7d474088c03aa:
asm-generic: unistd.h: fixup broken macro include. (2018-12-06 16:57:47 +0100)
----------------------------------------------------------------
asm-generic: bugfix for asm/unistd.h
Multiple people reported a bug I introduced in asm-generic/unistd.h
in 4.20, this is the obvious bugfix to get glibc and others to
correctly build again on new architectures that no longer provide
the old fstatat64() family of system calls.
----------------------------------------------------------------
Guo Ren (1):
asm-generic: unistd.h: fixup broken macro include.
include/uapi/asm-generic/unistd.h | 4 ++++
1 file changed, 4 insertions(+)
^ permalink raw reply
* Re: [GIT PULL] asm-generic: bugfix for asm/unistd.h
From: pr-tracker-bot @ 2018-12-08 19:50 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Linus Torvalds, Guo Ren, Marcin Juszkiewicz,
Linux Kernel Mailing List, Linux API
In-Reply-To: <CAK8P3a20S++xEHTYXXuipZ+YUZc0_EY2bj0JmaDwWdgWoLTROA@mail.gmail.com>
The pull request you sent on Sat, 8 Dec 2018 17:01:30 +0100:
> git://git.kernel.org/pub/scm/linux/kernel/git/arnd/asm-generic.git tags/asm-generic-4.20
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/8214bdf7d3e69050221aad19ec28826ed6cb4934
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
^ permalink raw reply
* Re: [PATCH v4] signal: add taskfd_send_signal() syscall
From: kbuild test robot @ 2018-12-08 21:46 UTC (permalink / raw)
Cc: kbuild-all, linux-kernel, linux-api, luto, arnd, ebiederm, serge,
jannh, akpm, oleg, cyphar, viro, linux-fsdevel, dancol, timmurray,
linux-man, keescook, fweimer, tglx, x86, Christian Brauner
In-Reply-To: <20181206121858.12215-1-christian@brauner.io>
[-- Attachment #1: Type: text/plain, Size: 1671 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-rc5]
[cannot apply to next-20181207]
[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/signal-add-taskfd_send_signal-syscall/20181209-044342
config: ia64-allnoconfig (attached as .config)
compiler: ia64-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=ia64
All warnings (new ones prefixed by >>):
<stdin>:1185:2: warning: #warning syscall perf_event_open not implemented [-Wcpp]
<stdin>:1239:2: warning: #warning syscall seccomp not implemented [-Wcpp]
<stdin>:1317:2: warning: #warning syscall pkey_mprotect not implemented [-Wcpp]
<stdin>:1320:2: warning: #warning syscall pkey_alloc not implemented [-Wcpp]
<stdin>:1323:2: warning: #warning syscall pkey_free not implemented [-Wcpp]
<stdin>:1326:2: warning: #warning syscall statx not implemented [-Wcpp]
<stdin>:1332:2: warning: #warning syscall io_pgetevents not implemented [-Wcpp]
<stdin>:1335:2: warning: #warning syscall rseq not implemented [-Wcpp]
>> <stdin>:1338:2: warning: #warning syscall taskfd_send_signal not implemented [-Wcpp]
---
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: 5655 bytes --]
^ permalink raw reply
* Re: Official Linux system wrapper library?
From: Randy Dunlap @ 2018-12-09 4:38 UTC (permalink / raw)
To: Jonathan Corbet, Greg KH
Cc: Carlos O'Donell, Daniel Colascione, linux-kernel,
Joel Fernandes, Linux API
In-Reply-To: <20181112090828.2573987d@lwn.net>
On 11/12/18 8:08 AM, Jonathan Corbet wrote:
> On Sun, 11 Nov 2018 18:36:30 -0800
> Greg KH <gregkh@linuxfoundation.org> wrote:
>
>> We should have a checklist. That's a great idea. Now to find someone
>> to write it... :)
>
> Do we think the LPC session might have the right people to create such a
> thing? If so, I can try to put together a coherent presentation of the
> result.
Hi,
Did anything ever happen with this syscall checklist suggestion?
thnx,
--
~Randy
^ permalink raw reply
* Re: [PATCH v5 1/1] signal: add pidfd_send_signal() syscall
From: kbuild test robot @ 2018-12-09 8:39 UTC (permalink / raw)
Cc: kbuild-all, linux-kernel, linux-api, luto, arnd, ebiederm, serge,
keescook, jannh, akpm, oleg, cyphar, viro, linux-fsdevel, dancol,
timmurray, fweimer, tglx, x86, Christian Brauner
In-Reply-To: <20181208054059.19813-2-christian@brauner.io>
[-- Attachment #1: Type: text/plain, Size: 1667 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-rc5]
[cannot apply to next-20181207]
[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/signaling-processes-through-pidfds/20181209-142857
config: ia64-allnoconfig (attached as .config)
compiler: ia64-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=ia64
All warnings (new ones prefixed by >>):
<stdin>:1185:2: warning: #warning syscall perf_event_open not implemented [-Wcpp]
<stdin>:1239:2: warning: #warning syscall seccomp not implemented [-Wcpp]
<stdin>:1317:2: warning: #warning syscall pkey_mprotect not implemented [-Wcpp]
<stdin>:1320:2: warning: #warning syscall pkey_alloc not implemented [-Wcpp]
<stdin>:1323:2: warning: #warning syscall pkey_free not implemented [-Wcpp]
<stdin>:1326:2: warning: #warning syscall statx not implemented [-Wcpp]
<stdin>:1332:2: warning: #warning syscall io_pgetevents not implemented [-Wcpp]
<stdin>:1335:2: warning: #warning syscall rseq not implemented [-Wcpp]
>> <stdin>:1338:2: warning: #warning syscall pidfd_send_signal not implemented [-Wcpp]
---
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: 5655 bytes --]
^ permalink raw reply
* Re: [PATCH v3 0/2] sysctl: handle overflow for file-max
From: Christian Brauner @ 2018-12-09 16:40 UTC (permalink / raw)
To: Kees Cook
Cc: Christian Brauner, Andrew Morton, Linux Kernel Mailing List,
Eric W. Biederman, mcgrof, joe.lawrence, Waiman Long,
Dominik Brodowski, Al Viro, Alexey Dobriyan, Linux API
In-Reply-To: <CAGXu5jJwSRQ3dOgNW_X0Fk+8gT+ACiBZPJ79g-aCfqiWsm=ewg@mail.gmail.com>
On Mon, Oct 29, 2018 at 10:44 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Mon, Oct 29, 2018 at 7:58 AM, Christian Brauner <christian@brauner.io> wrote:
> > On Wed, Oct 17, 2018 at 12:33:20AM +0200, Christian Brauner wrote:
> >> Hey,
> >>
> >> Here is v3 of this patchset. Changelogs are in the individual commits.
> >>
> >> Currently, when writing
> >>
> >> echo 18446744073709551616 > /proc/sys/fs/file-max
> >>
> >> /proc/sys/fs/file-max will overflow and be set to 0. That quickly
> >> crashes the system.
> >>
> >> The first version of this patch intended to detect the overflow and cap
> >> at ULONG_MAX. However, we should not do this and rather return EINVAL on
> >> overflow. The reasons are:
> >> - this aligns with other sysctl handlers that simply reject overflows
> >> (cf. [1], [2], and a bunch of others)
> >> - we already do a partial fail on overflow right now
> >> Namely, when the TMPBUFLEN is exceeded. So we already reject values
> >> such as 184467440737095516160 (21 chars) but accept values such as
> >> 18446744073709551616 (20 chars) but both are overflows. So we should
> >> just always reject 64bit overflows and not special-case this based on
> >> the number of chars.
> >>
> >> (This patchset is in reference to https://lkml.org/lkml/2018/10/11/585.)
> >
> > Just so that we don't forget, can we make sure that this gets picked
> > into linux-next? :)
>
> I was hoping akpm would take this? Andrew, does the v3 look okay to you?
gentle ping again :)
Christian
^ permalink raw reply
* [PATCH v10 0/4] seccomp trap to userspace
From: Tycho Andersen @ 2018-12-09 18:24 UTC (permalink / raw)
To: Kees Cook
Cc: Andy Lutomirski, Oleg Nesterov, Eric W . Biederman,
Serge E . Hallyn, Christian Brauner, Tyler Hicks, Akihiro Suda,
Aleksa Sarai, Jann Horn, linux-kernel, containers, linux-api,
Tycho Andersen
Hi all,
Here's v10 of the seccomp trap to userspace series. The only changes
since v9 was to fix up a compilation error in the
!CONFIG_ARCH_HAS_SYSCALL_WRAPPER case, and to add Serge's acks.
Link to v9: https://lore.kernel.org/lkml/20181203032827.27978-1-tycho@tycho.ws/T/#u
Cheers,
Tycho
Tycho Andersen (4):
seccomp: hoist struct seccomp_data recalculation higher
seccomp: switch system call argument type to void *
seccomp: add a return code to trap to userspace
samples: add an example of seccomp user trap
Documentation/ioctl/ioctl-number.txt | 1 +
.../userspace-api/seccomp_filter.rst | 84 ++++
include/linux/seccomp.h | 9 +-
include/linux/syscalls.h | 2 +-
include/uapi/linux/seccomp.h | 40 +-
kernel/seccomp.c | 468 +++++++++++++++++-
samples/seccomp/.gitignore | 1 +
samples/seccomp/Makefile | 7 +-
samples/seccomp/user-trap.c | 375 ++++++++++++++
tools/testing/selftests/seccomp/seccomp_bpf.c | 447 ++++++++++++++++-
10 files changed, 1411 insertions(+), 23 deletions(-)
create mode 100644 samples/seccomp/user-trap.c
--
2.19.1
^ permalink raw reply
* [PATCH v10 1/4] seccomp: hoist struct seccomp_data recalculation higher
From: Tycho Andersen @ 2018-12-09 18:24 UTC (permalink / raw)
To: Kees Cook
Cc: Andy Lutomirski, Oleg Nesterov, Eric W . Biederman,
Serge E . Hallyn, Christian Brauner, Tyler Hicks, Akihiro Suda,
Aleksa Sarai, Jann Horn, linux-kernel, containers, linux-api,
Tycho Andersen
In-Reply-To: <20181209182414.30862-1-tycho@tycho.ws>
In the next patch, we're going to use the sd pointer passed to
__seccomp_filter() as the data to pass to userspace. Except that in some
cases (__seccomp_filter(SECCOMP_RET_TRACE), emulate_vsyscall(), every time
seccomp is inovked on power, etc.) the sd pointer will be NULL in order to
force seccomp to recompute the register data. Previously this recomputation
happened one level lower, in seccomp_run_filters(); this patch just moves
it up a level higher to __seccomp_filter().
Thanks Oleg for spotting this.
Signed-off-by: Tycho Andersen <tycho@tycho.ws>
CC: Kees Cook <keescook@chromium.org>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Eric W. Biederman <ebiederm@xmission.com>
CC: "Serge E. Hallyn" <serge@hallyn.com>
Acked-by: Serge Hallyn <serge@hallyn.com>
CC: Christian Brauner <christian@brauner.io>
CC: Tyler Hicks <tyhicks@canonical.com>
CC: Akihiro Suda <suda.akihiro@lab.ntt.co.jp>
---
kernel/seccomp.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index f2ae2324c232..96afc32e041d 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -188,7 +188,6 @@ static int seccomp_check_filter(struct sock_filter *filter, unsigned int flen)
static u32 seccomp_run_filters(const struct seccomp_data *sd,
struct seccomp_filter **match)
{
- struct seccomp_data sd_local;
u32 ret = SECCOMP_RET_ALLOW;
/* Make sure cross-thread synced filter points somewhere sane. */
struct seccomp_filter *f =
@@ -198,11 +197,6 @@ static u32 seccomp_run_filters(const struct seccomp_data *sd,
if (WARN_ON(f == NULL))
return SECCOMP_RET_KILL_PROCESS;
- if (!sd) {
- populate_seccomp_data(&sd_local);
- sd = &sd_local;
- }
-
/*
* All filters in the list are evaluated and the lowest BPF return
* value always takes priority (ignoring the DATA).
@@ -658,6 +652,7 @@ static int __seccomp_filter(int this_syscall, const struct seccomp_data *sd,
u32 filter_ret, action;
struct seccomp_filter *match = NULL;
int data;
+ struct seccomp_data sd_local;
/*
* Make sure that any changes to mode from another thread have
@@ -665,6 +660,11 @@ static int __seccomp_filter(int this_syscall, const struct seccomp_data *sd,
*/
rmb();
+ if (!sd) {
+ populate_seccomp_data(&sd_local);
+ sd = &sd_local;
+ }
+
filter_ret = seccomp_run_filters(sd, &match);
data = filter_ret & SECCOMP_RET_DATA;
action = filter_ret & SECCOMP_RET_ACTION_FULL;
--
2.19.1
^ permalink raw reply related
* [PATCH v10 2/4] seccomp: switch system call argument type to void *
From: Tycho Andersen @ 2018-12-09 18:24 UTC (permalink / raw)
To: Kees Cook
Cc: Andy Lutomirski, Oleg Nesterov, Eric W . Biederman,
Serge E . Hallyn, Christian Brauner, Tyler Hicks, Akihiro Suda,
Aleksa Sarai, Jann Horn, linux-kernel, containers, linux-api,
Tycho Andersen
In-Reply-To: <20181209182414.30862-1-tycho@tycho.ws>
The const qualifier causes problems for any code that wants to write to the
third argument of the seccomp syscall, as we will do in a future patch in
this series.
The third argument to the seccomp syscall is documented as void *, so
rather than just dropping the const, let's switch everything to use void *
as well.
I believe this is safe because of 1. the documentation above, 2. there's no
real type information exported about syscalls anywhere besides the man
pages.
Signed-off-by: Tycho Andersen <tycho@tycho.ws>
CC: Kees Cook <keescook@chromium.org>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Eric W. Biederman <ebiederm@xmission.com>
CC: "Serge E. Hallyn" <serge@hallyn.com>
Acked-by: Serge Hallyn <serge@hallyn.com>
CC: Christian Brauner <christian@brauner.io>
CC: Tyler Hicks <tyhicks@canonical.com>
CC: Akihiro Suda <suda.akihiro@lab.ntt.co.jp>
---
v10: change type in include/linux/syscalls.h too to avoid compilation error
in the !CONFIG_ARCH_HAS_SYSCALL_WRAPPER case
---
include/linux/seccomp.h | 2 +-
include/linux/syscalls.h | 2 +-
kernel/seccomp.c | 8 ++++----
3 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/include/linux/seccomp.h b/include/linux/seccomp.h
index e5320f6c8654..b5103c019cf4 100644
--- a/include/linux/seccomp.h
+++ b/include/linux/seccomp.h
@@ -43,7 +43,7 @@ extern void secure_computing_strict(int this_syscall);
#endif
extern long prctl_get_seccomp(void);
-extern long prctl_set_seccomp(unsigned long, char __user *);
+extern long prctl_set_seccomp(unsigned long, void __user *);
static inline int seccomp_mode(struct seccomp *s)
{
diff --git a/include/linux/syscalls.h b/include/linux/syscalls.h
index 2ac3d13a915b..a60694fb0f58 100644
--- a/include/linux/syscalls.h
+++ b/include/linux/syscalls.h
@@ -879,7 +879,7 @@ asmlinkage long sys_renameat2(int olddfd, const char __user *oldname,
int newdfd, const char __user *newname,
unsigned int flags);
asmlinkage long sys_seccomp(unsigned int op, unsigned int flags,
- const char __user *uargs);
+ void __user *uargs);
asmlinkage long sys_getrandom(char __user *buf, size_t count,
unsigned int flags);
asmlinkage long sys_memfd_create(const char __user *uname_ptr, unsigned int flags);
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 96afc32e041d..393e029f778a 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -924,7 +924,7 @@ static long seccomp_get_action_avail(const char __user *uaction)
/* Common entry point for both prctl and syscall. */
static long do_seccomp(unsigned int op, unsigned int flags,
- const char __user *uargs)
+ void __user *uargs)
{
switch (op) {
case SECCOMP_SET_MODE_STRICT:
@@ -944,7 +944,7 @@ static long do_seccomp(unsigned int op, unsigned int flags,
}
SYSCALL_DEFINE3(seccomp, unsigned int, op, unsigned int, flags,
- const char __user *, uargs)
+ void __user *, uargs)
{
return do_seccomp(op, flags, uargs);
}
@@ -956,10 +956,10 @@ SYSCALL_DEFINE3(seccomp, unsigned int, op, unsigned int, flags,
*
* Returns 0 on success or -EINVAL on failure.
*/
-long prctl_set_seccomp(unsigned long seccomp_mode, char __user *filter)
+long prctl_set_seccomp(unsigned long seccomp_mode, void __user *filter)
{
unsigned int op;
- char __user *uargs;
+ void __user *uargs;
switch (seccomp_mode) {
case SECCOMP_MODE_STRICT:
--
2.19.1
^ permalink raw reply related
* [PATCH v10 3/4] seccomp: add a return code to trap to userspace
From: Tycho Andersen @ 2018-12-09 18:24 UTC (permalink / raw)
To: Kees Cook
Cc: Andy Lutomirski, Oleg Nesterov, Eric W . Biederman,
Serge E . Hallyn, Christian Brauner, Tyler Hicks, Akihiro Suda,
Aleksa Sarai, Jann Horn, linux-kernel, containers, linux-api,
Tycho Andersen
In-Reply-To: <20181209182414.30862-1-tycho@tycho.ws>
This patch introduces a means for syscalls matched in seccomp to notify
some other task that a particular filter has been triggered.
The motivation for this is primarily for use with containers. For example,
if a container does an init_module(), we obviously don't want to load this
untrusted code, which may be compiled for the wrong version of the kernel
anyway. Instead, we could parse the module image, figure out which module
the container is trying to load and load it on the host.
As another example, containers cannot mount() in general since various
filesystems assume a trusted image. However, if an orchestrator knows that
e.g. a particular block device has not been exposed to a container for
writing, it want to allow the container to mount that block device (that
is, handle the mount for it).
This patch adds functionality that is already possible via at least two
other means that I know about, both of which involve ptrace(): first, one
could ptrace attach, and then iterate through syscalls via PTRACE_SYSCALL.
Unfortunately this is slow, so a faster version would be to install a
filter that does SECCOMP_RET_TRACE, which triggers a PTRACE_EVENT_SECCOMP.
Since ptrace allows only one tracer, if the container runtime is that
tracer, users inside the container (or outside) trying to debug it will not
be able to use ptrace, which is annoying. It also means that older
distributions based on Upstart cannot boot inside containers using ptrace,
since upstart itself uses ptrace to monitor services while starting.
The actual implementation of this is fairly small, although getting the
synchronization right was/is slightly complex.
Finally, it's worth noting that the classic seccomp TOCTOU of reading
memory data from the task still applies here, but can be avoided with
careful design of the userspace handler: if the userspace handler reads all
of the task memory that is necessary before applying its security policy,
the tracee's subsequent memory edits will not be read by the tracer.
Signed-off-by: Tycho Andersen <tycho@tycho.ws>
CC: Kees Cook <keescook@chromium.org>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Eric W. Biederman <ebiederm@xmission.com>
CC: "Serge E. Hallyn" <serge@hallyn.com>
Acked-by: Serge Hallyn <serge@hallyn.com>
CC: Christian Brauner <christian@brauner.io>
CC: Tyler Hicks <tyhicks@canonical.com>
CC: Akihiro Suda <suda.akihiro@lab.ntt.co.jp>
---
v2: * make id a u64; the idea here being that it will never overflow,
because 64 is huge (one syscall every nanosecond => wrap every 584
years) (Andy)
* prevent nesting of user notifications: if someone is already attached
the tree in one place, nobody else can attach to the tree (Andy)
* notify the listener of signals the tracee receives as well (Andy)
* implement poll
v3: * lockdep fix (Oleg)
* drop unnecessary WARN()s (Christian)
* rearrange error returns to be more rpetty (Christian)
* fix build in !CONFIG_SECCOMP_USER_NOTIFICATION case
v4: * fix implementation of poll to use poll_wait() (Jann)
* change listener's fd flags to be 0 (Jann)
* hoist filter initialization out of ifdefs to its own function
init_user_notification()
* add some more testing around poll() and closing the listener while a
syscall is in action
* s/GET_LISTENER/NEW_LISTENER, since you can't _get_ a listener, but it
creates a new one (Matthew)
* correctly handle pid namespaces, add some testcases (Matthew)
* use EINPROGRESS instead of EINVAL when a notification response is
written twice (Matthew)
* fix comment typo from older version (SEND vs READ) (Matthew)
* whitespace and logic simplification (Tobin)
* add some Documentation/ bits on userspace trapping
v5: * fix documentation typos (Jann)
* add signalled field to struct seccomp_notif (Jann)
* switch to using ioctls instead of read()/write() for struct passing
(Jann)
* add an ioctl to ensure an id is still valid
v6: * docs typo fixes, update docs for ioctl() change (Christian)
v7: * switch struct seccomp_knotif's id member to a u64 (derp :)
* use notify_lock in IS_ID_VALID query to avoid racing
* s/signalled/signaled (Tyler)
* fix docs to reflect that ids are not globally unique (Tyler)
* add a test to check -ERESTARTSYS behavior (Tyler)
* drop CONFIG_SECCOMP_USER_NOTIFICATION (Tyler)
* reorder USER_NOTIF in seccomp return codes list (Tyler)
* return size instead of sizeof(struct user_notif) (Tyler)
* ENOENT instead of EINVAL when invalid id is passed (Tyler)
* drop CONFIG_SECCOMP_USER_NOTIFICATION guards (Tyler)
* s/IS_ID_VALID/ID_VALID and switch ioctl to be "well behaved" (Tyler)
* add a new struct notification to minimize the additions to
struct seccomp_filter, also pack the necessary additions a bit more
cleverly (Tyler)
* switch to keeping track of the task itself instead of the pid (we'll
use this for implementing PUT_FD)
v8: * in recv, don't copy_to_user() while holding notify lock, in case
userfaultfd blocks and causes all syscalls to block (Kees)
* switch ioctl character to something more fun ! (Kees)
* switch ioctl defines to use their own SECCOMP_IO* macros (Kees)
* rename seccomp ioctls to be SECCOMP_IOCTL_* (Kees)
* move comment of notify_lock to the right place (Jann)
* drop comment abount reference count bounding in __get_seccomp_filter (Jann)
* add lockdep_assert_held() in seccomp_next_notify_id() (Kees)
* in seccomp_do_user_notification(), always increment semaphore before
releasing lock, to prevent use after free of ->notif (Kees)
* add another wake_up_poll() when a signal is received (Jann)
* make all listener fds O_CLOEXEC (Jann/Kees)
* use memset() instead of = {} initialization for structures (Kees)
* move casting of buf pointer to ioctl, instead of in handler functions (Kees)
* fix ENOENT testing in seccomp_notify_send() (Jann)
* use ENOENT instead of -1 (EPERM) for ID_VALID ioctl (Jann)
* use ()s around "nested" bit operations (Kees)
* init struct notification members in the order they're declared (Jann)
* rearrange things so no forward declaration of init_listener() is
required (Kees)
* switch to a flags based future-proofing mechanism for struct
seccomp_notif and seccomp_notif_resp, thus avoiding version issues
with structure length (Kees)
* fix a memory leak in init_listener() in a failure case
* fix a use-after-free of filter->notif in do_user_notification() when
the listener fd is closed after a signal is sent
* add a comment about semaphore state in the interrupt case in
do_user_notification() + seccomp_notify_recv()
v9: * add SECCOMP_GET_NOTIF_SIZES to handle when struct seccomp_data
changes in size
* don't do locking all the way up the seccomp tree (Oleg)
* rearrange the tests so that one test tests one thing
* avoid an unkillable sleep by dropping the signaled flag (Oleg)
---
Documentation/ioctl/ioctl-number.txt | 1 +
.../userspace-api/seccomp_filter.rst | 84 ++++
include/linux/seccomp.h | 7 +-
include/uapi/linux/seccomp.h | 40 +-
kernel/seccomp.c | 448 +++++++++++++++++-
tools/testing/selftests/seccomp/seccomp_bpf.c | 447 ++++++++++++++++-
6 files changed, 1017 insertions(+), 10 deletions(-)
diff --git a/Documentation/ioctl/ioctl-number.txt b/Documentation/ioctl/ioctl-number.txt
index af6f6ba1fe80..c9558146ac58 100644
--- a/Documentation/ioctl/ioctl-number.txt
+++ b/Documentation/ioctl/ioctl-number.txt
@@ -79,6 +79,7 @@ Code Seq#(hex) Include File Comments
0x1b all InfiniBand Subsystem <http://infiniband.sourceforge.net/>
0x20 all drivers/cdrom/cm206.h
0x22 all scsi/sg.h
+'!' 00-1F uapi/linux/seccomp.h
'#' 00-3F IEEE 1394 Subsystem Block for the entire subsystem
'$' 00-0F linux/perf_counter.h, linux/perf_event.h
'%' 00-0F include/uapi/linux/stm.h
diff --git a/Documentation/userspace-api/seccomp_filter.rst b/Documentation/userspace-api/seccomp_filter.rst
index 82a468bc7560..b1b846d8a094 100644
--- a/Documentation/userspace-api/seccomp_filter.rst
+++ b/Documentation/userspace-api/seccomp_filter.rst
@@ -122,6 +122,11 @@ In precedence order, they are:
Results in the lower 16-bits of the return value being passed
to userland as the errno without executing the system call.
+``SECCOMP_RET_USER_NOTIF``:
+ Results in a ``struct seccomp_notif`` message sent on the userspace
+ notification fd, if it is attached, or ``-ENOSYS`` if it is not. See below
+ on discussion of how to handle user notifications.
+
``SECCOMP_RET_TRACE``:
When returned, this value will cause the kernel to attempt to
notify a ``ptrace()``-based tracer prior to executing the system
@@ -183,6 +188,85 @@ The ``samples/seccomp/`` directory contains both an x86-specific example
and a more generic example of a higher level macro interface for BPF
program generation.
+Userspace Notification
+======================
+
+The ``SECCOMP_RET_USER_NOTIF`` return code lets seccomp filters pass a
+particular syscall to userspace to be handled. This may be useful for
+applications like container managers, which wish to intercept particular
+syscalls (``mount()``, ``finit_module()``, etc.) and change their behavior.
+
+To acquire a notification FD, use the ``SECCOMP_FILTER_FLAG_NEW_LISTENER``
+argument to the ``seccomp()`` syscall:
+
+.. code-block:: c
+
+ fd = seccomp(SECCOMP_SET_MODE_FILTER, SECCOMP_FILTER_FLAG_NEW_LISTENER, &prog);
+
+which (on success) will return a listener fd for the filter, which can then be
+passed around via ``SCM_RIGHTS`` or similar. Note that filter fds correspond to
+a particular filter, and not a particular task. So if this task then forks,
+notifications from both tasks will appear on the same filter fd. Reads and
+writes to/from a filter fd are also synchronized, so a filter fd can safely
+have many readers.
+
+The interface for a seccomp notification fd consists of two structures:
+
+.. code-block:: c
+
+ struct seccomp_notif_sizes {
+ __u16 seccomp_notif;
+ __u16 seccomp_notif_resp;
+ __u16 seccomp_data;
+ };
+
+ struct seccomp_notif {
+ __u64 id;
+ __u32 pid;
+ __u32 flags;
+ struct seccomp_data data;
+ };
+
+ struct seccomp_notif_resp {
+ __u64 id;
+ __s64 val;
+ __s32 error;
+ __u32 flags;
+ };
+
+The ``struct seccomp_notif_sizes`` structure can be used to determine the size
+of the various structures used in seccomp notifications. The size of ``struct
+seccomp_data`` may change in the future, so code should use:
+
+.. code-block:: c
+
+ struct seccomp_notif_sizes sizes;
+ seccomp(SECCOMP_GET_NOTIF_SIZES, 0, &sizes);
+
+to determine the size of the various structures to allocate. See
+samples/seccomp/user-trap.c for an example.
+
+Users can read via ``ioctl(SECCOMP_IOCTL_NOTIF_RECV)`` (or ``poll()``) on a
+seccomp notification fd to receive a ``struct seccomp_notif``, which contains
+five members: the input length of the structure, a unique-per-filter ``id``,
+the ``pid`` of the task which triggered this request (which may be 0 if the
+task is in a pid ns not visible from the listener's pid namespace), a ``flags``
+member which for now only has ``SECCOMP_NOTIF_FLAG_SIGNALED``, representing
+whether or not the notification is a result of a non-fatal signal, and the
+``data`` passed to seccomp. Userspace can then make a decision based on this
+information about what to do, and ``ioctl(SECCOMP_IOCTL_NOTIF_SEND)`` a
+response, indicating what should be returned to userspace. The ``id`` member of
+``struct seccomp_notif_resp`` should be the same ``id`` as in ``struct
+seccomp_notif``.
+
+It is worth noting that ``struct seccomp_data`` contains the values of register
+arguments to the syscall, but does not contain pointers to memory. The task's
+memory is accessible to suitably privileged traces via ``ptrace()`` or
+``/proc/pid/mem``. However, care should be taken to avoid the TOCTOU mentioned
+above in this document: all arguments being read from the tracee's memory
+should be read into the tracer's memory before any policy decisions are made.
+This allows for an atomic decision on syscall arguments.
+
Sysctls
=======
diff --git a/include/linux/seccomp.h b/include/linux/seccomp.h
index b5103c019cf4..84868d37b35d 100644
--- a/include/linux/seccomp.h
+++ b/include/linux/seccomp.h
@@ -4,9 +4,10 @@
#include <uapi/linux/seccomp.h>
-#define SECCOMP_FILTER_FLAG_MASK (SECCOMP_FILTER_FLAG_TSYNC | \
- SECCOMP_FILTER_FLAG_LOG | \
- SECCOMP_FILTER_FLAG_SPEC_ALLOW)
+#define SECCOMP_FILTER_FLAG_MASK (SECCOMP_FILTER_FLAG_TSYNC | \
+ SECCOMP_FILTER_FLAG_LOG | \
+ SECCOMP_FILTER_FLAG_SPEC_ALLOW | \
+ SECCOMP_FILTER_FLAG_NEW_LISTENER)
#ifdef CONFIG_SECCOMP
diff --git a/include/uapi/linux/seccomp.h b/include/uapi/linux/seccomp.h
index 9efc0e73d50b..90734aa5aa36 100644
--- a/include/uapi/linux/seccomp.h
+++ b/include/uapi/linux/seccomp.h
@@ -15,11 +15,13 @@
#define SECCOMP_SET_MODE_STRICT 0
#define SECCOMP_SET_MODE_FILTER 1
#define SECCOMP_GET_ACTION_AVAIL 2
+#define SECCOMP_GET_NOTIF_SIZES 3
/* Valid flags for SECCOMP_SET_MODE_FILTER */
-#define SECCOMP_FILTER_FLAG_TSYNC (1UL << 0)
-#define SECCOMP_FILTER_FLAG_LOG (1UL << 1)
-#define SECCOMP_FILTER_FLAG_SPEC_ALLOW (1UL << 2)
+#define SECCOMP_FILTER_FLAG_TSYNC (1UL << 0)
+#define SECCOMP_FILTER_FLAG_LOG (1UL << 1)
+#define SECCOMP_FILTER_FLAG_SPEC_ALLOW (1UL << 2)
+#define SECCOMP_FILTER_FLAG_NEW_LISTENER (1UL << 3)
/*
* All BPF programs must return a 32-bit value.
@@ -35,6 +37,7 @@
#define SECCOMP_RET_KILL SECCOMP_RET_KILL_THREAD
#define SECCOMP_RET_TRAP 0x00030000U /* disallow and force a SIGSYS */
#define SECCOMP_RET_ERRNO 0x00050000U /* returns an errno */
+#define SECCOMP_RET_USER_NOTIF 0x7fc00000U /* notifies userspace */
#define SECCOMP_RET_TRACE 0x7ff00000U /* pass to a tracer or disallow */
#define SECCOMP_RET_LOG 0x7ffc0000U /* allow after logging */
#define SECCOMP_RET_ALLOW 0x7fff0000U /* allow */
@@ -60,4 +63,35 @@ struct seccomp_data {
__u64 args[6];
};
+struct seccomp_notif_sizes {
+ __u16 seccomp_notif;
+ __u16 seccomp_notif_resp;
+ __u16 seccomp_data;
+};
+
+struct seccomp_notif {
+ __u64 id;
+ __u32 pid;
+ __u32 flags;
+ struct seccomp_data data;
+};
+
+struct seccomp_notif_resp {
+ __u64 id;
+ __s64 val;
+ __s32 error;
+ __u32 flags;
+};
+
+#define SECCOMP_IOC_MAGIC '!'
+#define SECCOMP_IO(nr) _IO(SECCOMP_IOC_MAGIC, nr)
+#define SECCOMP_IOR(nr, type) _IOR(SECCOMP_IOC_MAGIC, nr, type)
+#define SECCOMP_IOW(nr, type) _IOW(SECCOMP_IOC_MAGIC, nr, type)
+#define SECCOMP_IOWR(nr, type) _IOWR(SECCOMP_IOC_MAGIC, nr, type)
+
+/* Flags for seccomp notification fd ioctl. */
+#define SECCOMP_IOCTL_NOTIF_RECV SECCOMP_IOWR(0, struct seccomp_notif)
+#define SECCOMP_IOCTL_NOTIF_SEND SECCOMP_IOWR(1, \
+ struct seccomp_notif_resp)
+#define SECCOMP_IOCTL_NOTIF_ID_VALID SECCOMP_IOR(2, __u64)
#endif /* _UAPI_LINUX_SECCOMP_H */
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 393e029f778a..15b6be97fc09 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -33,12 +33,74 @@
#endif
#ifdef CONFIG_SECCOMP_FILTER
+#include <linux/file.h>
#include <linux/filter.h>
#include <linux/pid.h>
#include <linux/ptrace.h>
#include <linux/security.h>
#include <linux/tracehook.h>
#include <linux/uaccess.h>
+#include <linux/anon_inodes.h>
+
+enum notify_state {
+ SECCOMP_NOTIFY_INIT,
+ SECCOMP_NOTIFY_SENT,
+ SECCOMP_NOTIFY_REPLIED,
+};
+
+struct seccomp_knotif {
+ /* The struct pid of the task whose filter triggered the notification */
+ struct task_struct *task;
+
+ /* The "cookie" for this request; this is unique for this filter. */
+ u64 id;
+
+ /*
+ * The seccomp data. This pointer is valid the entire time this
+ * notification is active, since it comes from __seccomp_filter which
+ * eclipses the entire lifecycle here.
+ */
+ const struct seccomp_data *data;
+
+ /*
+ * Notification states. When SECCOMP_RET_USER_NOTIF is returned, a
+ * struct seccomp_knotif is created and starts out in INIT. Once the
+ * handler reads the notification off of an FD, it transitions to SENT.
+ * If a signal is received the state transitions back to INIT and
+ * another message is sent. When the userspace handler replies, state
+ * transitions to REPLIED.
+ */
+ enum notify_state state;
+
+ /* The return values, only valid when in SECCOMP_NOTIFY_REPLIED */
+ int error;
+ long val;
+
+ /* Signals when this has entered SECCOMP_NOTIFY_REPLIED */
+ struct completion ready;
+
+ struct list_head list;
+};
+
+/**
+ * struct notification - container for seccomp userspace notifications. Since
+ * most seccomp filters will not have notification listeners attached and this
+ * structure is fairly large, we store the notification-specific stuff in a
+ * separate structure.
+ *
+ * @request: A semaphore that users of this notification can wait on for
+ * changes. Actual reads and writes are still controlled with
+ * filter->notify_lock.
+ * @next_id: The id of the next request.
+ * @notifications: A list of struct seccomp_knotif elements.
+ * @wqh: A wait queue for poll.
+ */
+struct notification {
+ struct semaphore request;
+ u64 next_id;
+ struct list_head notifications;
+ wait_queue_head_t wqh;
+};
/**
* struct seccomp_filter - container for seccomp BPF programs
@@ -50,6 +112,8 @@
* @log: true if all actions except for SECCOMP_RET_ALLOW should be logged
* @prev: points to a previously installed, or inherited, filter
* @prog: the BPF program to evaluate
+ * @notif: the struct that holds all notification related information
+ * @notify_lock: A lock for all notification-related accesses.
*
* seccomp_filter objects are organized in a tree linked via the @prev
* pointer. For any task, it appears to be a singly-linked list starting
@@ -66,6 +130,8 @@ struct seccomp_filter {
bool log;
struct seccomp_filter *prev;
struct bpf_prog *prog;
+ struct notification *notif;
+ struct mutex notify_lock;
};
/* Limit any path through the tree to 256KB worth of instructions. */
@@ -386,6 +452,7 @@ static struct seccomp_filter *seccomp_prepare_filter(struct sock_fprog *fprog)
if (!sfilter)
return ERR_PTR(-ENOMEM);
+ mutex_init(&sfilter->notify_lock);
ret = bpf_prog_create_from_user(&sfilter->prog, fprog,
seccomp_check_filter, save_orig);
if (ret < 0) {
@@ -479,7 +546,6 @@ static long seccomp_attach_filter(unsigned int flags,
static void __get_seccomp_filter(struct seccomp_filter *filter)
{
- /* Reference count is bounded by the number of total processes. */
refcount_inc(&filter->usage);
}
@@ -550,11 +616,13 @@ static void seccomp_send_sigsys(int syscall, int reason)
#define SECCOMP_LOG_TRACE (1 << 4)
#define SECCOMP_LOG_LOG (1 << 5)
#define SECCOMP_LOG_ALLOW (1 << 6)
+#define SECCOMP_LOG_USER_NOTIF (1 << 7)
static u32 seccomp_actions_logged = SECCOMP_LOG_KILL_PROCESS |
SECCOMP_LOG_KILL_THREAD |
SECCOMP_LOG_TRAP |
SECCOMP_LOG_ERRNO |
+ SECCOMP_LOG_USER_NOTIF |
SECCOMP_LOG_TRACE |
SECCOMP_LOG_LOG;
@@ -575,6 +643,9 @@ static inline void seccomp_log(unsigned long syscall, long signr, u32 action,
case SECCOMP_RET_TRACE:
log = requested && seccomp_actions_logged & SECCOMP_LOG_TRACE;
break;
+ case SECCOMP_RET_USER_NOTIF:
+ log = requested && seccomp_actions_logged & SECCOMP_LOG_USER_NOTIF;
+ break;
case SECCOMP_RET_LOG:
log = seccomp_actions_logged & SECCOMP_LOG_LOG;
break;
@@ -646,6 +717,68 @@ void secure_computing_strict(int this_syscall)
#else
#ifdef CONFIG_SECCOMP_FILTER
+static u64 seccomp_next_notify_id(struct seccomp_filter *filter)
+{
+ /*
+ * Note: overflow is ok here, the id just needs to be unique per
+ * filter.
+ */
+ lockdep_assert_held(&filter->notify_lock);
+ return filter->notif->next_id++;
+}
+
+static void seccomp_do_user_notification(int this_syscall,
+ struct seccomp_filter *match,
+ const struct seccomp_data *sd)
+{
+ int err;
+ long ret = 0;
+ struct seccomp_knotif n = {};
+
+ mutex_lock(&match->notify_lock);
+ err = -ENOSYS;
+ if (!match->notif)
+ goto out;
+
+ n.task = current;
+ n.state = SECCOMP_NOTIFY_INIT;
+ n.data = sd;
+ n.id = seccomp_next_notify_id(match);
+ init_completion(&n.ready);
+ list_add(&n.list, &match->notif->notifications);
+
+ up(&match->notif->request);
+ wake_up_poll(&match->notif->wqh, EPOLLIN | EPOLLRDNORM);
+ mutex_unlock(&match->notify_lock);
+
+ /*
+ * This is where we wait for a reply from userspace.
+ */
+ err = wait_for_completion_interruptible(&n.ready);
+ mutex_lock(&match->notify_lock);
+ if (err == 0) {
+ ret = n.val;
+ err = n.error;
+ }
+
+ /*
+ * Note that it's possible the listener died in between the time when
+ * we were notified of a respons (or a signal) and when we were able to
+ * re-acquire the lock, so only delete from the list if the
+ * notification actually exists.
+ *
+ * Also note that this test is only valid because there's no way to
+ * *reattach* to a notifier right now. If one is added, we'll need to
+ * keep track of the notif itself and make sure they match here.
+ */
+ if (match->notif)
+ list_del(&n.list);
+out:
+ mutex_unlock(&match->notify_lock);
+ syscall_set_return_value(current, task_pt_regs(current),
+ err, ret);
+}
+
static int __seccomp_filter(int this_syscall, const struct seccomp_data *sd,
const bool recheck_after_trace)
{
@@ -728,6 +861,10 @@ static int __seccomp_filter(int this_syscall, const struct seccomp_data *sd,
return 0;
+ case SECCOMP_RET_USER_NOTIF:
+ seccomp_do_user_notification(this_syscall, match, sd);
+ goto skip;
+
case SECCOMP_RET_LOG:
seccomp_log(this_syscall, 0, action, true);
return 0;
@@ -834,6 +971,263 @@ static long seccomp_set_mode_strict(void)
}
#ifdef CONFIG_SECCOMP_FILTER
+static int seccomp_notify_release(struct inode *inode, struct file *file)
+{
+ struct seccomp_filter *filter = file->private_data;
+ struct seccomp_knotif *knotif;
+
+ mutex_lock(&filter->notify_lock);
+
+ /*
+ * If this file is being closed because e.g. the task who owned it
+ * died, let's wake everyone up who was waiting on us.
+ */
+ list_for_each_entry(knotif, &filter->notif->notifications, list) {
+ if (knotif->state == SECCOMP_NOTIFY_REPLIED)
+ continue;
+
+ knotif->state = SECCOMP_NOTIFY_REPLIED;
+ knotif->error = -ENOSYS;
+ knotif->val = 0;
+
+ complete(&knotif->ready);
+ }
+
+ kfree(filter->notif);
+ filter->notif = NULL;
+ mutex_unlock(&filter->notify_lock);
+ __put_seccomp_filter(filter);
+ return 0;
+}
+
+static long seccomp_notify_recv(struct seccomp_filter *filter,
+ void __user *buf)
+{
+ struct seccomp_knotif *knotif = NULL, *cur;
+ struct seccomp_notif unotif;
+ ssize_t ret;
+
+ memset(&unotif, 0, sizeof(unotif));
+
+ ret = down_interruptible(&filter->notif->request);
+ if (ret < 0)
+ return ret;
+
+ mutex_lock(&filter->notify_lock);
+ list_for_each_entry(cur, &filter->notif->notifications, list) {
+ if (cur->state == SECCOMP_NOTIFY_INIT) {
+ knotif = cur;
+ break;
+ }
+ }
+
+ /*
+ * If we didn't find a notification, it could be that the task was
+ * interrupted by a fatal signal between the time we were woken and
+ * when we were able to acquire the rw lock.
+ */
+ if (!knotif) {
+ ret = -ENOENT;
+ goto out;
+ }
+
+ unotif.id = knotif->id;
+ unotif.pid = task_pid_vnr(knotif->task);
+ unotif.data = *(knotif->data);
+
+ knotif->state = SECCOMP_NOTIFY_SENT;
+ wake_up_poll(&filter->notif->wqh, EPOLLOUT | EPOLLWRNORM);
+ ret = 0;
+out:
+ mutex_unlock(&filter->notify_lock);
+
+ if (ret == 0 && copy_to_user(buf, &unotif, sizeof(unotif))) {
+ ret = -EFAULT;
+
+ /*
+ * Userspace screwed up. To make sure that we keep this
+ * notification alive, let's reset it back to INIT. It
+ * may have died when we released the lock, so we need to make
+ * sure it's still around.
+ */
+ knotif = NULL;
+ mutex_lock(&filter->notify_lock);
+ list_for_each_entry(cur, &filter->notif->notifications, list) {
+ if (cur->id == unotif.id) {
+ knotif = cur;
+ break;
+ }
+ }
+
+ if (knotif) {
+ knotif->state = SECCOMP_NOTIFY_INIT;
+ up(&filter->notif->request);
+ }
+ mutex_unlock(&filter->notify_lock);
+ }
+
+ return ret;
+}
+
+static long seccomp_notify_send(struct seccomp_filter *filter,
+ void __user *buf)
+{
+ struct seccomp_notif_resp resp = {};
+ struct seccomp_knotif *knotif = NULL, *cur;
+ long ret;
+
+ if (copy_from_user(&resp, buf, sizeof(resp)))
+ return -EFAULT;
+
+ if (resp.flags)
+ return -EINVAL;
+
+ ret = mutex_lock_interruptible(&filter->notify_lock);
+ if (ret < 0)
+ return ret;
+
+ list_for_each_entry(cur, &filter->notif->notifications, list) {
+ if (cur->id == resp.id) {
+ knotif = cur;
+ break;
+ }
+ }
+
+ if (!knotif) {
+ ret = -ENOENT;
+ goto out;
+ }
+
+ /* Allow exactly one reply. */
+ if (knotif->state != SECCOMP_NOTIFY_SENT) {
+ ret = -EINPROGRESS;
+ goto out;
+ }
+
+ ret = 0;
+ knotif->state = SECCOMP_NOTIFY_REPLIED;
+ knotif->error = resp.error;
+ knotif->val = resp.val;
+ complete(&knotif->ready);
+out:
+ mutex_unlock(&filter->notify_lock);
+ return ret;
+}
+
+static long seccomp_notify_id_valid(struct seccomp_filter *filter,
+ void __user *buf)
+{
+ struct seccomp_knotif *knotif = NULL;
+ u64 id;
+ long ret;
+
+ if (copy_from_user(&id, buf, sizeof(id)))
+ return -EFAULT;
+
+ ret = mutex_lock_interruptible(&filter->notify_lock);
+ if (ret < 0)
+ return ret;
+
+ ret = -ENOENT;
+ list_for_each_entry(knotif, &filter->notif->notifications, list) {
+ if (knotif->id == id) {
+ if (knotif->state == SECCOMP_NOTIFY_SENT)
+ ret = 0;
+ goto out;
+ }
+ }
+
+out:
+ mutex_unlock(&filter->notify_lock);
+ return ret;
+}
+
+static long seccomp_notify_ioctl(struct file *file, unsigned int cmd,
+ unsigned long arg)
+{
+ struct seccomp_filter *filter = file->private_data;
+ void __user *buf = (void __user *)arg;
+
+ switch (cmd) {
+ case SECCOMP_IOCTL_NOTIF_RECV:
+ return seccomp_notify_recv(filter, buf);
+ case SECCOMP_IOCTL_NOTIF_SEND:
+ return seccomp_notify_send(filter, buf);
+ case SECCOMP_IOCTL_NOTIF_ID_VALID:
+ return seccomp_notify_id_valid(filter, buf);
+ default:
+ return -EINVAL;
+ }
+}
+
+static __poll_t seccomp_notify_poll(struct file *file,
+ struct poll_table_struct *poll_tab)
+{
+ struct seccomp_filter *filter = file->private_data;
+ __poll_t ret = 0;
+ struct seccomp_knotif *cur;
+
+ poll_wait(file, &filter->notif->wqh, poll_tab);
+
+ ret = mutex_lock_interruptible(&filter->notify_lock);
+ if (ret < 0)
+ return EPOLLERR;
+
+ list_for_each_entry(cur, &filter->notif->notifications, list) {
+ if (cur->state == SECCOMP_NOTIFY_INIT)
+ ret |= EPOLLIN | EPOLLRDNORM;
+ if (cur->state == SECCOMP_NOTIFY_SENT)
+ ret |= EPOLLOUT | EPOLLWRNORM;
+ if ((ret & EPOLLIN) && (ret & EPOLLOUT))
+ break;
+ }
+
+ mutex_unlock(&filter->notify_lock);
+
+ return ret;
+}
+
+static const struct file_operations seccomp_notify_ops = {
+ .poll = seccomp_notify_poll,
+ .release = seccomp_notify_release,
+ .unlocked_ioctl = seccomp_notify_ioctl,
+};
+
+static struct file *init_listener(struct seccomp_filter *filter)
+{
+ struct file *ret = ERR_PTR(-EBUSY);
+ struct seccomp_filter *cur;
+
+ for (cur = current->seccomp.filter; cur; cur = cur->prev) {
+ if (cur->notif)
+ goto out;
+ }
+
+ ret = ERR_PTR(-ENOMEM);
+ filter->notif = kzalloc(sizeof(*(filter->notif)), GFP_KERNEL);
+ if (!filter->notif)
+ goto out;
+
+ sema_init(&filter->notif->request, 0);
+ filter->notif->next_id = get_random_u64();
+ INIT_LIST_HEAD(&filter->notif->notifications);
+ init_waitqueue_head(&filter->notif->wqh);
+
+ ret = anon_inode_getfile("seccomp notify", &seccomp_notify_ops,
+ filter, O_RDWR);
+ if (IS_ERR(ret))
+ goto out_notif;
+
+ /* The file has a reference to it now */
+ __get_seccomp_filter(filter);
+
+out_notif:
+ if (IS_ERR(ret))
+ kfree(filter->notif);
+out:
+ return ret;
+}
+
/**
* seccomp_set_mode_filter: internal function for setting seccomp filter
* @flags: flags to change filter behavior
@@ -853,6 +1247,8 @@ static long seccomp_set_mode_filter(unsigned int flags,
const unsigned long seccomp_mode = SECCOMP_MODE_FILTER;
struct seccomp_filter *prepared = NULL;
long ret = -EINVAL;
+ int listener = -1;
+ struct file *listener_f = NULL;
/* Validate flags. */
if (flags & ~SECCOMP_FILTER_FLAG_MASK)
@@ -863,13 +1259,28 @@ static long seccomp_set_mode_filter(unsigned int flags,
if (IS_ERR(prepared))
return PTR_ERR(prepared);
+ if (flags & SECCOMP_FILTER_FLAG_NEW_LISTENER) {
+ listener = get_unused_fd_flags(O_CLOEXEC);
+ if (listener < 0) {
+ ret = listener;
+ goto out_free;
+ }
+
+ listener_f = init_listener(prepared);
+ if (IS_ERR(listener_f)) {
+ put_unused_fd(listener);
+ ret = PTR_ERR(listener_f);
+ goto out_free;
+ }
+ }
+
/*
* Make sure we cannot change seccomp or nnp state via TSYNC
* while another thread is in the middle of calling exec.
*/
if (flags & SECCOMP_FILTER_FLAG_TSYNC &&
mutex_lock_killable(¤t->signal->cred_guard_mutex))
- goto out_free;
+ goto out_put_fd;
spin_lock_irq(¤t->sighand->siglock);
@@ -887,6 +1298,16 @@ static long seccomp_set_mode_filter(unsigned int flags,
spin_unlock_irq(¤t->sighand->siglock);
if (flags & SECCOMP_FILTER_FLAG_TSYNC)
mutex_unlock(¤t->signal->cred_guard_mutex);
+out_put_fd:
+ if (flags & SECCOMP_FILTER_FLAG_NEW_LISTENER) {
+ if (ret < 0) {
+ fput(listener_f);
+ put_unused_fd(listener);
+ } else {
+ fd_install(listener, listener_f);
+ ret = listener;
+ }
+ }
out_free:
seccomp_filter_free(prepared);
return ret;
@@ -911,6 +1332,7 @@ static long seccomp_get_action_avail(const char __user *uaction)
case SECCOMP_RET_KILL_THREAD:
case SECCOMP_RET_TRAP:
case SECCOMP_RET_ERRNO:
+ case SECCOMP_RET_USER_NOTIF:
case SECCOMP_RET_TRACE:
case SECCOMP_RET_LOG:
case SECCOMP_RET_ALLOW:
@@ -922,6 +1344,20 @@ static long seccomp_get_action_avail(const char __user *uaction)
return 0;
}
+static long seccomp_get_notif_sizes(void __user *usizes)
+{
+ struct seccomp_notif_sizes sizes = {
+ .seccomp_notif = sizeof(struct seccomp_notif),
+ .seccomp_notif_resp = sizeof(struct seccomp_notif_resp),
+ .seccomp_data = sizeof(struct seccomp_data),
+ };
+
+ if (copy_to_user(usizes, &sizes, sizeof(sizes)))
+ return -EFAULT;
+
+ return 0;
+}
+
/* Common entry point for both prctl and syscall. */
static long do_seccomp(unsigned int op, unsigned int flags,
void __user *uargs)
@@ -938,6 +1374,11 @@ static long do_seccomp(unsigned int op, unsigned int flags,
return -EINVAL;
return seccomp_get_action_avail(uargs);
+ case SECCOMP_GET_NOTIF_SIZES:
+ if (flags != 0)
+ return -EINVAL;
+
+ return seccomp_get_notif_sizes(uargs);
default:
return -EINVAL;
}
@@ -1111,6 +1552,7 @@ long seccomp_get_metadata(struct task_struct *task,
#define SECCOMP_RET_KILL_THREAD_NAME "kill_thread"
#define SECCOMP_RET_TRAP_NAME "trap"
#define SECCOMP_RET_ERRNO_NAME "errno"
+#define SECCOMP_RET_USER_NOTIF_NAME "user_notif"
#define SECCOMP_RET_TRACE_NAME "trace"
#define SECCOMP_RET_LOG_NAME "log"
#define SECCOMP_RET_ALLOW_NAME "allow"
@@ -1120,6 +1562,7 @@ static const char seccomp_actions_avail[] =
SECCOMP_RET_KILL_THREAD_NAME " "
SECCOMP_RET_TRAP_NAME " "
SECCOMP_RET_ERRNO_NAME " "
+ SECCOMP_RET_USER_NOTIF_NAME " "
SECCOMP_RET_TRACE_NAME " "
SECCOMP_RET_LOG_NAME " "
SECCOMP_RET_ALLOW_NAME;
@@ -1134,6 +1577,7 @@ static const struct seccomp_log_name seccomp_log_names[] = {
{ SECCOMP_LOG_KILL_THREAD, SECCOMP_RET_KILL_THREAD_NAME },
{ SECCOMP_LOG_TRAP, SECCOMP_RET_TRAP_NAME },
{ SECCOMP_LOG_ERRNO, SECCOMP_RET_ERRNO_NAME },
+ { SECCOMP_LOG_USER_NOTIF, SECCOMP_RET_USER_NOTIF_NAME },
{ SECCOMP_LOG_TRACE, SECCOMP_RET_TRACE_NAME },
{ SECCOMP_LOG_LOG, SECCOMP_RET_LOG_NAME },
{ SECCOMP_LOG_ALLOW, SECCOMP_RET_ALLOW_NAME },
diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
index e1473234968d..5c9768a1b8cd 100644
--- a/tools/testing/selftests/seccomp/seccomp_bpf.c
+++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
@@ -5,6 +5,7 @@
* Test code for seccomp bpf.
*/
+#define _GNU_SOURCE
#include <sys/types.h>
/*
@@ -40,10 +41,12 @@
#include <sys/fcntl.h>
#include <sys/mman.h>
#include <sys/times.h>
+#include <sys/socket.h>
+#include <sys/ioctl.h>
-#define _GNU_SOURCE
#include <unistd.h>
#include <sys/syscall.h>
+#include <poll.h>
#include "../kselftest_harness.h"
@@ -133,6 +136,10 @@ struct seccomp_data {
#define SECCOMP_GET_ACTION_AVAIL 2
#endif
+#ifndef SECCOMP_GET_NOTIF_SIZES
+#define SECCOMP_GET_NOTIF_SIZES 3
+#endif
+
#ifndef SECCOMP_FILTER_FLAG_TSYNC
#define SECCOMP_FILTER_FLAG_TSYNC (1UL << 0)
#endif
@@ -154,6 +161,44 @@ struct seccomp_metadata {
};
#endif
+#ifndef SECCOMP_FILTER_FLAG_NEW_LISTENER
+#define SECCOMP_FILTER_FLAG_NEW_LISTENER (1UL << 3)
+
+#define SECCOMP_RET_USER_NOTIF 0x7fc00000U
+
+#define SECCOMP_IOC_MAGIC '!'
+#define SECCOMP_IO(nr) _IO(SECCOMP_IOC_MAGIC, nr)
+#define SECCOMP_IOR(nr, type) _IOR(SECCOMP_IOC_MAGIC, nr, type)
+#define SECCOMP_IOW(nr, type) _IOW(SECCOMP_IOC_MAGIC, nr, type)
+#define SECCOMP_IOWR(nr, type) _IOWR(SECCOMP_IOC_MAGIC, nr, type)
+
+/* Flags for seccomp notification fd ioctl. */
+#define SECCOMP_IOCTL_NOTIF_RECV SECCOMP_IOWR(0, struct seccomp_notif)
+#define SECCOMP_IOCTL_NOTIF_SEND SECCOMP_IOWR(1, \
+ struct seccomp_notif_resp)
+#define SECCOMP_IOCTL_NOTIF_ID_VALID SECCOMP_IOR(2, __u64)
+
+struct seccomp_notif {
+ __u64 id;
+ __u32 pid;
+ __u32 flags;
+ struct seccomp_data data;
+};
+
+struct seccomp_notif_resp {
+ __u64 id;
+ __s64 val;
+ __s32 error;
+ __u32 flags;
+};
+
+struct seccomp_notif_sizes {
+ __u16 seccomp_notif;
+ __u16 seccomp_notif_resp;
+ __u16 seccomp_data;
+};
+#endif
+
#ifndef seccomp
int seccomp(unsigned int op, unsigned int flags, void *args)
{
@@ -2077,7 +2122,8 @@ TEST(detect_seccomp_filter_flags)
{
unsigned int flags[] = { SECCOMP_FILTER_FLAG_TSYNC,
SECCOMP_FILTER_FLAG_LOG,
- SECCOMP_FILTER_FLAG_SPEC_ALLOW };
+ SECCOMP_FILTER_FLAG_SPEC_ALLOW,
+ SECCOMP_FILTER_FLAG_NEW_LISTENER };
unsigned int flag, all_flags;
int i;
long ret;
@@ -2933,6 +2979,403 @@ TEST(get_metadata)
ASSERT_EQ(0, kill(pid, SIGKILL));
}
+static int user_trap_syscall(int nr, unsigned int flags)
+{
+ struct sock_filter filter[] = {
+ BPF_STMT(BPF_LD+BPF_W+BPF_ABS,
+ offsetof(struct seccomp_data, nr)),
+ BPF_JUMP(BPF_JMP+BPF_JEQ+BPF_K, nr, 0, 1),
+ BPF_STMT(BPF_RET+BPF_K, SECCOMP_RET_USER_NOTIF),
+ BPF_STMT(BPF_RET+BPF_K, SECCOMP_RET_ALLOW),
+ };
+
+ struct sock_fprog prog = {
+ .len = (unsigned short)ARRAY_SIZE(filter),
+ .filter = filter,
+ };
+
+ return seccomp(SECCOMP_SET_MODE_FILTER, flags, &prog);
+}
+
+#define USER_NOTIF_MAGIC 116983961184613L
+TEST(user_notification_basic)
+{
+ pid_t pid;
+ long ret;
+ int status, listener;
+ struct seccomp_notif req = {};
+ struct seccomp_notif_resp resp = {};
+ struct pollfd pollfd;
+
+ struct sock_filter filter[] = {
+ BPF_STMT(BPF_RET|BPF_K, SECCOMP_RET_ALLOW),
+ };
+ struct sock_fprog prog = {
+ .len = (unsigned short)ARRAY_SIZE(filter),
+ .filter = filter,
+ };
+
+ pid = fork();
+ ASSERT_GE(pid, 0);
+
+ /* Check that we get -ENOSYS with no listener attached */
+ if (pid == 0) {
+ if (user_trap_syscall(__NR_getpid, 0) < 0)
+ exit(1);
+ ret = syscall(__NR_getpid);
+ exit(ret >= 0 || errno != ENOSYS);
+ }
+
+ EXPECT_EQ(waitpid(pid, &status, 0), pid);
+ EXPECT_EQ(true, WIFEXITED(status));
+ EXPECT_EQ(0, WEXITSTATUS(status));
+
+ /* Add some no-op filters so for grins. */
+ EXPECT_EQ(seccomp(SECCOMP_SET_MODE_FILTER, 0, &prog), 0);
+ EXPECT_EQ(seccomp(SECCOMP_SET_MODE_FILTER, 0, &prog), 0);
+ EXPECT_EQ(seccomp(SECCOMP_SET_MODE_FILTER, 0, &prog), 0);
+ EXPECT_EQ(seccomp(SECCOMP_SET_MODE_FILTER, 0, &prog), 0);
+
+ /* Check that the basic notification machinery works */
+ listener = user_trap_syscall(__NR_getpid,
+ SECCOMP_FILTER_FLAG_NEW_LISTENER);
+ EXPECT_GE(listener, 0);
+
+ /* Installing a second listener in the chain should EBUSY */
+ EXPECT_EQ(user_trap_syscall(__NR_getpid,
+ SECCOMP_FILTER_FLAG_NEW_LISTENER),
+ -1);
+ EXPECT_EQ(errno, EBUSY);
+
+ pid = fork();
+ ASSERT_GE(pid, 0);
+
+ if (pid == 0) {
+ ret = syscall(__NR_getpid);
+ exit(ret != USER_NOTIF_MAGIC);
+ }
+
+ pollfd.fd = listener;
+ pollfd.events = POLLIN | POLLOUT;
+
+ EXPECT_GT(poll(&pollfd, 1, -1), 0);
+ EXPECT_EQ(pollfd.revents, POLLIN);
+
+ EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req), 0);
+
+ pollfd.fd = listener;
+ pollfd.events = POLLIN | POLLOUT;
+
+ EXPECT_GT(poll(&pollfd, 1, -1), 0);
+ EXPECT_EQ(pollfd.revents, POLLOUT);
+
+ EXPECT_EQ(req.data.nr, __NR_getpid);
+
+ resp.id = req.id;
+ resp.error = 0;
+ resp.val = USER_NOTIF_MAGIC;
+
+ /* check that we make sure flags == 0 */
+ resp.flags = 1;
+ EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_SEND, &resp), -1);
+ EXPECT_EQ(errno, EINVAL);
+
+ resp.flags = 0;
+ EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_SEND, &resp), 0);
+
+ EXPECT_EQ(waitpid(pid, &status, 0), pid);
+ EXPECT_EQ(true, WIFEXITED(status));
+ EXPECT_EQ(0, WEXITSTATUS(status));
+}
+
+TEST(user_notification_kill_in_middle)
+{
+ pid_t pid;
+ long ret;
+ int listener;
+ struct seccomp_notif req = {};
+ struct seccomp_notif_resp resp = {};
+
+ listener = user_trap_syscall(__NR_getpid,
+ SECCOMP_FILTER_FLAG_NEW_LISTENER);
+ EXPECT_GE(listener, 0);
+
+ /*
+ * Check that nothing bad happens when we kill the task in the middle
+ * of a syscall.
+ */
+ pid = fork();
+ ASSERT_GE(pid, 0);
+
+ if (pid == 0) {
+ ret = syscall(__NR_getpid);
+ exit(ret != USER_NOTIF_MAGIC);
+ }
+
+ EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req), 0);
+ EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_ID_VALID, &req.id), 0);
+
+ EXPECT_EQ(kill(pid, SIGKILL), 0);
+ EXPECT_EQ(waitpid(pid, NULL, 0), pid);
+
+ EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_ID_VALID, &req.id), -1);
+
+ resp.id = req.id;
+ ret = ioctl(listener, SECCOMP_IOCTL_NOTIF_SEND, &resp);
+ EXPECT_EQ(ret, -1);
+ EXPECT_EQ(errno, ENOENT);
+}
+
+static int handled = -1;
+
+static void signal_handler(int signal)
+{
+ if (write(handled, "c", 1) != 1)
+ perror("write from signal");
+}
+
+TEST(user_notification_signal)
+{
+ pid_t pid;
+ long ret;
+ int status, listener, sk_pair[2];
+ struct seccomp_notif req = {};
+ struct seccomp_notif_resp resp = {};
+ char c;
+
+ ASSERT_EQ(socketpair(PF_LOCAL, SOCK_SEQPACKET, 0, sk_pair), 0);
+
+ listener = user_trap_syscall(__NR_gettid,
+ SECCOMP_FILTER_FLAG_NEW_LISTENER);
+ EXPECT_GE(listener, 0);
+
+ pid = fork();
+ ASSERT_GE(pid, 0);
+
+ if (pid == 0) {
+ close(sk_pair[0]);
+ handled = sk_pair[1];
+ if (signal(SIGUSR1, signal_handler) == SIG_ERR) {
+ perror("signal");
+ exit(1);
+ }
+ /*
+ * ERESTARTSYS behavior is a bit hard to test, because we need
+ * to rely on a signal that has not yet been handled. Let's at
+ * least check that the error code gets propagated through, and
+ * hope that it doesn't break when there is actually a signal :)
+ */
+ ret = syscall(__NR_gettid);
+ exit(!(ret == -1 && errno == 512));
+ }
+
+ close(sk_pair[1]);
+
+ EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req), 0);
+
+ EXPECT_EQ(kill(pid, SIGUSR1), 0);
+
+ /*
+ * Make sure the signal really is delivered, which means we're not
+ * stuck in the user notification code any more and the notification
+ * should be dead.
+ */
+ EXPECT_EQ(read(sk_pair[0], &c, 1), 1);
+
+ resp.id = req.id;
+ resp.error = -EPERM;
+ resp.val = 0;
+
+ EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_SEND, &resp), -1);
+ EXPECT_EQ(errno, ENOENT);
+
+ EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req), 0);
+
+ resp.id = req.id;
+ resp.error = -512; /* -ERESTARTSYS */
+ resp.val = 0;
+
+ EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_SEND, &resp), 0);
+
+ EXPECT_EQ(waitpid(pid, &status, 0), pid);
+ EXPECT_EQ(true, WIFEXITED(status));
+ EXPECT_EQ(0, WEXITSTATUS(status));
+}
+
+TEST(user_notification_closed_listener)
+{
+ pid_t pid;
+ long ret;
+ int status, listener;
+
+ listener = user_trap_syscall(__NR_getpid,
+ SECCOMP_FILTER_FLAG_NEW_LISTENER);
+ EXPECT_GE(listener, 0);
+
+ /*
+ * Check that we get an ENOSYS when the listener is closed.
+ */
+ pid = fork();
+ ASSERT_GE(pid, 0);
+ if (pid == 0) {
+ close(listener);
+ ret = syscall(__NR_getpid);
+ exit(ret != -1 && errno != ENOSYS);
+ }
+
+ close(listener);
+
+ EXPECT_EQ(waitpid(pid, &status, 0), pid);
+ EXPECT_EQ(true, WIFEXITED(status));
+ EXPECT_EQ(0, WEXITSTATUS(status));
+}
+
+/*
+ * Check that a pid in a child namespace still shows up as valid in ours.
+ */
+TEST(user_notification_child_pid_ns)
+{
+ pid_t pid;
+ int status, listener;
+ struct seccomp_notif req = {};
+ struct seccomp_notif_resp resp = {};
+
+ ASSERT_EQ(unshare(CLONE_NEWPID), 0);
+
+ listener = user_trap_syscall(__NR_getpid, SECCOMP_FILTER_FLAG_NEW_LISTENER);
+ ASSERT_GE(listener, 0);
+
+ pid = fork();
+ ASSERT_GE(pid, 0);
+
+ if (pid == 0)
+ exit(syscall(__NR_getpid) != USER_NOTIF_MAGIC);
+
+ EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req), 0);
+ EXPECT_EQ(req.pid, pid);
+
+ resp.id = req.id;
+ resp.error = 0;
+ resp.val = USER_NOTIF_MAGIC;
+
+ EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_SEND, &resp), 0);
+
+ EXPECT_EQ(waitpid(pid, &status, 0), pid);
+ EXPECT_EQ(true, WIFEXITED(status));
+ EXPECT_EQ(0, WEXITSTATUS(status));
+ close(listener);
+}
+
+/*
+ * Check that a pid in a sibling (i.e. unrelated) namespace shows up as 0, i.e.
+ * invalid.
+ */
+TEST(user_notification_sibling_pid_ns)
+{
+ pid_t pid, pid2;
+ int status, listener;
+ struct seccomp_notif req = {};
+ struct seccomp_notif_resp resp = {};
+
+ listener = user_trap_syscall(__NR_getpid, SECCOMP_FILTER_FLAG_NEW_LISTENER);
+ ASSERT_GE(listener, 0);
+
+ pid = fork();
+ ASSERT_GE(pid, 0);
+
+ if (pid == 0) {
+ ASSERT_EQ(unshare(CLONE_NEWPID), 0);
+
+ pid2 = fork();
+ ASSERT_GE(pid2, 0);
+
+ if (pid2 == 0)
+ exit(syscall(__NR_getpid) != USER_NOTIF_MAGIC);
+
+ EXPECT_EQ(waitpid(pid2, &status, 0), pid2);
+ EXPECT_EQ(true, WIFEXITED(status));
+ EXPECT_EQ(0, WEXITSTATUS(status));
+ exit(WEXITSTATUS(status));
+ }
+
+ /* Create the sibling ns, and sibling in it. */
+ EXPECT_EQ(unshare(CLONE_NEWPID), 0);
+ EXPECT_EQ(errno, 0);
+
+ pid2 = fork();
+ EXPECT_GE(pid2, 0);
+
+ if (pid2 == 0) {
+ ASSERT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req), 0);
+ /*
+ * The pid should be 0, i.e. the task is in some namespace that
+ * we can't "see".
+ */
+ ASSERT_EQ(req.pid, 0);
+
+ resp.id = req.id;
+ resp.error = 0;
+ resp.val = USER_NOTIF_MAGIC;
+
+ ASSERT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_SEND, &resp), 0);
+ exit(0);
+ }
+
+ close(listener);
+
+ EXPECT_EQ(waitpid(pid, &status, 0), pid);
+ EXPECT_EQ(true, WIFEXITED(status));
+ EXPECT_EQ(0, WEXITSTATUS(status));
+
+ EXPECT_EQ(waitpid(pid2, &status, 0), pid2);
+ EXPECT_EQ(true, WIFEXITED(status));
+ EXPECT_EQ(0, WEXITSTATUS(status));
+}
+
+TEST(user_notification_fault_recv)
+{
+ pid_t pid;
+ int status, listener;
+ struct seccomp_notif req = {};
+ struct seccomp_notif_resp resp = {};
+
+ listener = user_trap_syscall(__NR_getpid, SECCOMP_FILTER_FLAG_NEW_LISTENER);
+ ASSERT_GE(listener, 0);
+
+ pid = fork();
+ ASSERT_GE(pid, 0);
+
+ if (pid == 0)
+ exit(syscall(__NR_getpid) != USER_NOTIF_MAGIC);
+
+ /* Do a bad recv() */
+ EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, NULL), -1);
+ EXPECT_EQ(errno, EFAULT);
+
+ /* We should still be able to receive this notification, though. */
+ EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, &req), 0);
+ EXPECT_EQ(req.pid, pid);
+
+ resp.id = req.id;
+ resp.error = 0;
+ resp.val = USER_NOTIF_MAGIC;
+
+ EXPECT_EQ(ioctl(listener, SECCOMP_IOCTL_NOTIF_SEND, &resp), 0);
+
+ EXPECT_EQ(waitpid(pid, &status, 0), pid);
+ EXPECT_EQ(true, WIFEXITED(status));
+ EXPECT_EQ(0, WEXITSTATUS(status));
+}
+
+TEST(seccomp_get_notif_sizes)
+{
+ struct seccomp_notif_sizes sizes;
+
+ EXPECT_EQ(seccomp(SECCOMP_GET_NOTIF_SIZES, 0, &sizes), 0);
+ EXPECT_EQ(sizes.seccomp_notif, sizeof(struct seccomp_notif));
+ EXPECT_EQ(sizes.seccomp_notif_resp, sizeof(struct seccomp_notif_resp));
+}
+
/*
* TODO:
* - add microbenchmarks
--
2.19.1
^ permalink raw reply related
* [PATCH v10 4/4] samples: add an example of seccomp user trap
From: Tycho Andersen @ 2018-12-09 18:24 UTC (permalink / raw)
To: Kees Cook
Cc: Andy Lutomirski, Oleg Nesterov, Eric W . Biederman,
Serge E . Hallyn, Christian Brauner, Tyler Hicks, Akihiro Suda,
Aleksa Sarai, Jann Horn, linux-kernel, containers, linux-api,
Tycho Andersen
In-Reply-To: <20181209182414.30862-1-tycho@tycho.ws>
The idea here is just to give a demonstration of how one could safely use
the SECCOMP_RET_USER_NOTIF feature to do mount policies. This particular
policy is (as noted in the comment) not very interesting, but it serves to
illustrate how one might apply a policy dodging the various TOCTOU issues.
Signed-off-by: Tycho Andersen <tycho@tycho.ws>
CC: Kees Cook <keescook@chromium.org>
CC: Andy Lutomirski <luto@amacapital.net>
CC: Oleg Nesterov <oleg@redhat.com>
CC: Eric W. Biederman <ebiederm@xmission.com>
CC: "Serge E. Hallyn" <serge@hallyn.com>
CC: Christian Brauner <christian@brauner.io>
CC: Tyler Hicks <tyhicks@canonical.com>
CC: Akihiro Suda <suda.akihiro@lab.ntt.co.jp>
---
v5: new in v5
v7: updates for v7 API changes
v8: * add some more comments about what's happening in main() (Kees)
* move from ptrace API to SECCOMP_FILTER_FLAG_NEW_LISTENER
v9: * s/mknod/mount in error message
* switch to the SECCOMP_GET_NOTIF_SIZES API
* add a note about getting ENOENT from SECCOMP_IOCTL_NOTIF_SEND
---
samples/seccomp/.gitignore | 1 +
samples/seccomp/Makefile | 7 +-
samples/seccomp/user-trap.c | 375 ++++++++++++++++++++++++++++++++++++
3 files changed, 382 insertions(+), 1 deletion(-)
diff --git a/samples/seccomp/.gitignore b/samples/seccomp/.gitignore
index 78fb78184291..d1e2e817d556 100644
--- a/samples/seccomp/.gitignore
+++ b/samples/seccomp/.gitignore
@@ -1,3 +1,4 @@
bpf-direct
bpf-fancy
dropper
+user-trap
diff --git a/samples/seccomp/Makefile b/samples/seccomp/Makefile
index cf34ff6b4065..4920903c8009 100644
--- a/samples/seccomp/Makefile
+++ b/samples/seccomp/Makefile
@@ -1,6 +1,6 @@
# SPDX-License-Identifier: GPL-2.0
ifndef CROSS_COMPILE
-hostprogs-$(CONFIG_SAMPLE_SECCOMP) := bpf-fancy dropper bpf-direct
+hostprogs-$(CONFIG_SAMPLE_SECCOMP) := bpf-fancy dropper bpf-direct user-trap
HOSTCFLAGS_bpf-fancy.o += -I$(objtree)/usr/include
HOSTCFLAGS_bpf-fancy.o += -idirafter $(objtree)/include
@@ -16,6 +16,10 @@ HOSTCFLAGS_bpf-direct.o += -I$(objtree)/usr/include
HOSTCFLAGS_bpf-direct.o += -idirafter $(objtree)/include
bpf-direct-objs := bpf-direct.o
+HOSTCFLAGS_user-trap.o += -I$(objtree)/usr/include
+HOSTCFLAGS_user-trap.o += -idirafter $(objtree)/include
+user-trap-objs := user-trap.o
+
# Try to match the kernel target.
ifndef CONFIG_64BIT
@@ -33,6 +37,7 @@ HOSTCFLAGS_bpf-fancy.o += $(MFLAG)
HOSTLDLIBS_bpf-direct += $(MFLAG)
HOSTLDLIBS_bpf-fancy += $(MFLAG)
HOSTLDLIBS_dropper += $(MFLAG)
+HOSTLDLIBS_user-trap += $(MFLAG)
endif
always := $(hostprogs-m)
endif
diff --git a/samples/seccomp/user-trap.c b/samples/seccomp/user-trap.c
new file mode 100644
index 000000000000..61267cb59c8e
--- /dev/null
+++ b/samples/seccomp/user-trap.c
@@ -0,0 +1,375 @@
+#include <signal.h>
+#include <stdio.h>
+#include <stdlib.h>
+#include <unistd.h>
+#include <errno.h>
+#include <fcntl.h>
+#include <string.h>
+#include <stddef.h>
+#include <sys/sysmacros.h>
+#include <sys/types.h>
+#include <sys/wait.h>
+#include <sys/socket.h>
+#include <sys/stat.h>
+#include <sys/mman.h>
+#include <sys/syscall.h>
+#include <sys/user.h>
+#include <sys/ioctl.h>
+#include <sys/ptrace.h>
+#include <sys/mount.h>
+#include <linux/limits.h>
+#include <linux/filter.h>
+#include <linux/seccomp.h>
+
+#define ARRAY_SIZE(x) (sizeof(x) / sizeof(*(x)))
+
+static int seccomp(unsigned int op, unsigned int flags, void *args)
+{
+ errno = 0;
+ return syscall(__NR_seccomp, op, flags, args);
+}
+
+static int send_fd(int sock, int fd)
+{
+ struct msghdr msg = {};
+ struct cmsghdr *cmsg;
+ char buf[CMSG_SPACE(sizeof(int))] = {0}, c = 'c';
+ struct iovec io = {
+ .iov_base = &c,
+ .iov_len = 1,
+ };
+
+ msg.msg_iov = &io;
+ msg.msg_iovlen = 1;
+ msg.msg_control = buf;
+ msg.msg_controllen = sizeof(buf);
+ cmsg = CMSG_FIRSTHDR(&msg);
+ cmsg->cmsg_level = SOL_SOCKET;
+ cmsg->cmsg_type = SCM_RIGHTS;
+ cmsg->cmsg_len = CMSG_LEN(sizeof(int));
+ *((int *)CMSG_DATA(cmsg)) = fd;
+ msg.msg_controllen = cmsg->cmsg_len;
+
+ if (sendmsg(sock, &msg, 0) < 0) {
+ perror("sendmsg");
+ return -1;
+ }
+
+ return 0;
+}
+
+static int recv_fd(int sock)
+{
+ struct msghdr msg = {};
+ struct cmsghdr *cmsg;
+ char buf[CMSG_SPACE(sizeof(int))] = {0}, c = 'c';
+ struct iovec io = {
+ .iov_base = &c,
+ .iov_len = 1,
+ };
+
+ msg.msg_iov = &io;
+ msg.msg_iovlen = 1;
+ msg.msg_control = buf;
+ msg.msg_controllen = sizeof(buf);
+
+ if (recvmsg(sock, &msg, 0) < 0) {
+ perror("recvmsg");
+ return -1;
+ }
+
+ cmsg = CMSG_FIRSTHDR(&msg);
+
+ return *((int *)CMSG_DATA(cmsg));
+}
+
+static int user_trap_syscall(int nr, unsigned int flags)
+{
+ struct sock_filter filter[] = {
+ BPF_STMT(BPF_LD+BPF_W+BPF_ABS,
+ offsetof(struct seccomp_data, nr)),
+ BPF_JUMP(BPF_JMP+BPF_JEQ+BPF_K, nr, 0, 1),
+ BPF_STMT(BPF_RET+BPF_K, SECCOMP_RET_USER_NOTIF),
+ BPF_STMT(BPF_RET+BPF_K, SECCOMP_RET_ALLOW),
+ };
+
+ struct sock_fprog prog = {
+ .len = (unsigned short)ARRAY_SIZE(filter),
+ .filter = filter,
+ };
+
+ return seccomp(SECCOMP_SET_MODE_FILTER, flags, &prog);
+}
+
+static int handle_req(struct seccomp_notif *req,
+ struct seccomp_notif_resp *resp, int listener)
+{
+ char path[PATH_MAX], source[PATH_MAX], target[PATH_MAX];
+ int ret = -1, mem;
+
+ resp->id = req->id;
+ resp->error = -EPERM;
+ resp->val = 0;
+
+ if (req->data.nr != __NR_mount) {
+ fprintf(stderr, "huh? trapped something besides mount? %d\n", req->data.nr);
+ return -1;
+ }
+
+ /* Only allow bind mounts. */
+ if (!(req->data.args[3] & MS_BIND))
+ return 0;
+
+ /*
+ * Ok, let's read the task's memory to see where they wanted their
+ * mount to go.
+ */
+ snprintf(path, sizeof(path), "/proc/%d/mem", req->pid);
+ mem = open(path, O_RDONLY);
+ if (mem < 0) {
+ perror("open mem");
+ return -1;
+ }
+
+ /*
+ * Now we avoid a TOCTOU: we referred to a pid by its pid, but since
+ * the pid that made the syscall may have died, we need to confirm that
+ * the pid is still valid after we open its /proc/pid/mem file. We can
+ * ask the listener fd this as follows.
+ *
+ * Note that this check should occur *after* any task-specific
+ * resources are opened, to make sure that the task has not died and
+ * we're not wrongly reading someone else's state in order to make
+ * decisions.
+ */
+ if (ioctl(listener, SECCOMP_IOCTL_NOTIF_ID_VALID, &req->id) < 0) {
+ fprintf(stderr, "task died before we could map its memory\n");
+ goto out;
+ }
+
+ /*
+ * Phew, we've got the right /proc/pid/mem. Now we can read it. Note
+ * that to avoid another TOCTOU, we should read all of the pointer args
+ * before we decide to allow the syscall.
+ */
+ if (lseek(mem, req->data.args[0], SEEK_SET) < 0) {
+ perror("seek");
+ goto out;
+ }
+
+ ret = read(mem, source, sizeof(source));
+ if (ret < 0) {
+ perror("read");
+ goto out;
+ }
+
+ if (lseek(mem, req->data.args[1], SEEK_SET) < 0) {
+ perror("seek");
+ goto out;
+ }
+
+ ret = read(mem, target, sizeof(target));
+ if (ret < 0) {
+ perror("read");
+ goto out;
+ }
+
+ /*
+ * Our policy is to only allow bind mounts inside /tmp. This isn't very
+ * interesting, because we could do unprivlieged bind mounts with user
+ * namespaces already, but you get the idea.
+ */
+ if (!strncmp(source, "/tmp", 4) && !strncmp(target, "/tmp", 4)) {
+ if (mount(source, target, NULL, req->data.args[3], NULL) < 0) {
+ ret = -1;
+ perror("actual mount");
+ goto out;
+ }
+ resp->error = 0;
+ }
+
+ /* Even if we didn't allow it because of policy, generating the
+ * response was be a success, because we want to tell the worker EPERM.
+ */
+ ret = 0;
+
+out:
+ close(mem);
+ return ret;
+}
+
+int main(void)
+{
+ int sk_pair[2], ret = 1, status, listener;
+ pid_t worker = 0 , tracer = 0;
+
+ if (socketpair(PF_LOCAL, SOCK_SEQPACKET, 0, sk_pair) < 0) {
+ perror("socketpair");
+ return 1;
+ }
+
+ worker = fork();
+ if (worker < 0) {
+ perror("fork");
+ goto close_pair;
+ }
+
+ if (worker == 0) {
+ listener = user_trap_syscall(__NR_mount,
+ SECCOMP_FILTER_FLAG_NEW_LISTENER);
+ if (listener < 0) {
+ perror("seccomp");
+ exit(1);
+ }
+
+ /*
+ * Drop privileges. We definitely can't mount as uid 1000.
+ */
+ if (setuid(1000) < 0) {
+ perror("setuid");
+ exit(1);
+ }
+
+ /*
+ * Send the listener to the parent; also serves as
+ * synchronization.
+ */
+ if (send_fd(sk_pair[1], listener) < 0)
+ exit(1);
+ close(listener);
+
+ if (mkdir("/tmp/foo", 0755) < 0) {
+ perror("mkdir");
+ exit(1);
+ }
+
+ /*
+ * Try a bad mount just for grins.
+ */
+ if (mount("/dev/sda", "/tmp/foo", NULL, 0, NULL) != -1) {
+ fprintf(stderr, "huh? mounted /dev/sda?\n");
+ exit(1);
+ }
+
+ if (errno != EPERM) {
+ perror("bad error from mount");
+ exit(1);
+ }
+
+ /*
+ * Ok, we expect this one to succeed.
+ */
+ if (mount("/tmp/foo", "/tmp/foo", NULL, MS_BIND, NULL) < 0) {
+ perror("mount");
+ exit(1);
+ }
+
+ exit(0);
+ }
+
+ /*
+ * Get the listener from the child.
+ */
+ listener = recv_fd(sk_pair[0]);
+ if (listener < 0)
+ goto out_kill;
+
+ /*
+ * Fork a task to handle the requests. This isn't strictly necessary,
+ * but it makes the particular writing of this sample easier, since we
+ * can just wait ofr the tracee to exit and kill the tracer.
+ */
+ tracer = fork();
+ if (tracer < 0) {
+ perror("fork");
+ goto out_kill;
+ }
+
+ if (tracer == 0) {
+ struct seccomp_notif *req;
+ struct seccomp_notif_resp *resp;
+ struct seccomp_notif_sizes sizes;
+
+ if (seccomp(SECCOMP_GET_NOTIF_SIZES, 0, &sizes) < 0) {
+ perror("seccomp(GET_NOTIF_SIZES)");
+ goto out_close;
+ }
+
+ req = malloc(sizes.seccomp_notif);
+ if (!req)
+ goto out_close;
+ memset(req, 0, sizeof(*req));
+
+ resp = malloc(sizes.seccomp_notif_resp);
+ if (!resp)
+ goto out_req;
+ memset(resp, 0, sizeof(*resp));
+
+ while (1) {
+ if (ioctl(listener, SECCOMP_IOCTL_NOTIF_RECV, req)) {
+ perror("ioctl recv");
+ goto out_resp;
+ }
+
+ if (handle_req(req, resp, listener) < 0)
+ goto out_resp;
+
+ /*
+ * ENOENT here means that the task may have gotten a
+ * signal and restarted the syscall. It's up to the
+ * handler to decide what to do in this case, but for
+ * the sample code, we just ignore it. Probably
+ * something better should happen, like undoing the
+ * mount, or keeping track of the args to make sure we
+ * don't do it again.
+ */
+ if (ioctl(listener, SECCOMP_IOCTL_NOTIF_SEND, resp) < 0 &&
+ errno != ENOENT) {
+ perror("ioctl send");
+ goto out_resp;
+ }
+ }
+out_resp:
+ free(resp);
+out_req:
+ free(req);
+out_close:
+ close(listener);
+ exit(1);
+ }
+
+ close(listener);
+
+ if (waitpid(worker, &status, 0) != worker) {
+ perror("waitpid");
+ goto out_kill;
+ }
+
+ if (umount2("/tmp/foo", MNT_DETACH) < 0 && errno != EINVAL) {
+ perror("umount2");
+ goto out_kill;
+ }
+
+ if (remove("/tmp/foo") < 0 && errno != ENOENT) {
+ perror("remove");
+ exit(1);
+ }
+
+ if (!WIFEXITED(status) || WEXITSTATUS(status)) {
+ fprintf(stderr, "worker exited nonzero\n");
+ goto out_kill;
+ }
+
+ ret = 0;
+
+out_kill:
+ if (tracer > 0)
+ kill(tracer, SIGKILL);
+ if (worker > 0)
+ kill(worker, SIGKILL);
+
+close_pair:
+ close(sk_pair[0]);
+ close(sk_pair[1]);
+ return ret;
+}
--
2.19.1
^ permalink raw reply related
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