* Re: [PATCH v5 RESEND] signal: add pidfd_send_signal() syscall
From: Christian Brauner @ 2018-12-28 23:37 UTC (permalink / raw)
To: Andrew Morton
Cc: linux-kernel, linux-api, luto, arnd, serge, keescook, jannh, oleg,
cyphar, viro, linux-fsdevel, dancol, timmurray, fweimer, tglx,
x86, ebiederm
In-Reply-To: <20181228152012.dbf0508c2508138efc5f2bbe@linux-foundation.org>
On Fri, Dec 28, 2018 at 03:20:12PM -0800, Andrew Morton wrote:
> On Fri, 28 Dec 2018 23:48:53 +0100 Christian Brauner <christian@brauner.io> 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 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).
So most of the questions you ask have been extensively discussed in
prior threads. All of them have links above in the long commit message.
>
> I can't see a description of what happens when the target process has
> exited. Is the task_struct pinned by the fd? Does the entire procfs
A reference to struct pid is kept. struct pid - as far as I understand -
was created exactly for the reason to not require to pin struct
task_struct. This is also noted in the comments to struct pid:
https://elixir.bootlin.com/linux/v4.20-rc1/source/include/linux/pid.h#L16
> directory remain visible? Just one entry within it? Does the pid
The same thing that happens when you currently keep an fd to /proc/<pid>
open.
> remain reserved? Do attempts to signal that fd return errors?
The pid does not remain reserved. Which leads back to your question
about what happens when you try to signal a process that has exited: you
get ESRCH.
> etcetera. These behaviors should be described in the changelog and
> manipulate please.
I can add those questions to the changelog too.
>
> The code in signal.c appears to be compiled in even when
> CONFIG_PROC_FS=y. Can we add the appropriate ifdefs and an entry in
> sys_ni.c?
Without having looked super close at this from the top of my head I'd
say, yes we can.
>
> A selftest in toole/testing/selftests would be nice. And it will be
> helpful to architecture maintainers as they wire this up.
I can extend the sample program in the commit message to a selftest.
>
> The feature doesn't have its own Kconfig setting. Perhaps it should?
> It should presumably depend on PROC_FS.
Not sure why we should do this.
>
> I must say that I dislike the linkage to procfs. procfs is a
> high-level thing which is manipulated using syscalls. To turn around
> and make a syscall dependent upon the presence of procfs seems just ...
> wrong. Is there a cleaner way of obtaining the fd? Another syscall
> perhaps.
We may do something like this in the future. There's another syscall
lined up at some point translate_pid() which we may extend to also give
back an fd to a process. For now the open() on /proc/<pid> is the
cleanest way of doing this.
>
> This fd-for-a-process sounds like a handy thing and people may well
> think up other uses for it in the future, probably unrelated to
> signals. Are the code and the interface designed to permit such future
> applications? I guess "no" - it presently assumes that anything which
This too has been discussed in prior threads linked in the commit
message. Yes, it does permit of such extension and we have already
publicly discussed future extensions (e.g. wait maybe a new clone
syscall).
> is written to that fd is a signal. Perhaps there should be a tag at
> the start of the message (which is what it is) which identifies the
> message's type?
>
> Now I think about it, why a new syscall? This thing is looking rather
> like an ioctl?
Again, we have had a lengthy discussion about this and by now we all
agree that a dedicated syscall makes more sense than an ioctl() for
security reasons, because processes are a core-kernel concept, and we
intend to extend this api with syscalls in the future (e.g. wait etc.
also discussed in prior threads linked in here). There's also a summary
article on lwn about parts of this (https://lwn.net/Articles/773459/).
Thanks!
Christian
^ permalink raw reply
* Re: [PATCH v5 RESEND] signal: add pidfd_send_signal() syscall
From: Andrew Morton @ 2018-12-28 23:20 UTC (permalink / raw)
To: Christian Brauner
Cc: linux-kernel, linux-api, luto, arnd, serge, keescook, jannh, oleg,
cyphar, viro, linux-fsdevel, dancol, timmurray, fweimer, tglx,
x86, ebiederm
In-Reply-To: <20181228224853.26032-1-christian@brauner.io>
On Fri, 28 Dec 2018 23:48:53 +0100 Christian Brauner <christian@brauner.io> 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 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).
I can't see a description of what happens when the target process has
exited. Is the task_struct pinned by the fd? Does the entire procfs
directory remain visible? Just one entry within it? Does the pid
remain reserved? Do attempts to signal that fd return errors?
etcetera. These behaviors should be described in the changelog and
manipulate please.
The code in signal.c appears to be compiled in even when
CONFIG_PROC_FS=y. Can we add the appropriate ifdefs and an entry in
sys_ni.c?
A selftest in toole/testing/selftests would be nice. And it will be
helpful to architecture maintainers as they wire this up.
The feature doesn't have its own Kconfig setting. Perhaps it should?
It should presumably depend on PROC_FS.
I must say that I dislike the linkage to procfs. procfs is a
high-level thing which is manipulated using syscalls. To turn around
and make a syscall dependent upon the presence of procfs seems just ...
wrong. Is there a cleaner way of obtaining the fd? Another syscall
perhaps.
This fd-for-a-process sounds like a handy thing and people may well
think up other uses for it in the future, probably unrelated to
signals. Are the code and the interface designed to permit such future
applications? I guess "no" - it presently assumes that anything which
is written to that fd is a signal. Perhaps there should be a tag at
the start of the message (which is what it is) which identifies the
message's type?
Now I think about it, why a new syscall? This thing is looking rather
like an ioctl?
^ permalink raw reply
* Re: [Qemu-devel] d_off field in struct dirent and 32-on-64 emulation
From: Andreas Dilger @ 2018-12-28 23:16 UTC (permalink / raw)
To: Peter Maydell
Cc: Florian Weimer, linux-fsdevel, Linux API, Ext4 Developers List,
lucho, libc-alpha, Arnd Bergmann, ericvh, hpa,
lkml - Kernel Mailing List, QEMU Developers, rminnich,
v9fs-developer
In-Reply-To: <CAFEAcA9W+JK7_TrtTnL1P2ES1knNPJX9wcUvhfLwxLq9augq1w@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 4001 bytes --]
On Dec 28, 2018, at 4:18 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Fri, 28 Dec 2018 at 00:23, Andreas Dilger <adilger@dilger.ca> wrote:
>> On Dec 27, 2018, at 10:41 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>>> As you note, this causes breakage for userspace programs which
>>> need to implement an API/ABI with 32-bit offset but which only
>>> have access to the kernel's 64-bit offset API/ABI.
>>
>> This is (IMHO) a bit of an oxymoron, isn't it? Applications using
>> the 64-bit API, but storing the value in a 32-bit field?
>
> I didn't say "which choose to store the value in a 32-bit field",
> I said "which have to implement an API/ABI which has 32-bit fields".
> In QEMU's case, we use the host kernel's ABI, which has 64-bit
> offset fields. We implement a syscall ABI for the guest binary
> we are running under emulation, which may have 32-bit offset fields
> (for instance if we are running a 32-bit Arm binary.) Both of
> these ABIs are fixed -- QEMU doesn't have a choice here, it
> just has to make the best effort it can with what the host kernel
> provides it, to provide the semantics the guest binary needs.
> My suggestion in this thread is that the host kernel provides
> a wider range of facilities so that QEMU can do the job it's
> trying to do.
>
>> The same
>> problem would exist for filesystems with 64-bit inodes or 64-bit
>> file offsets trying to store these values in 32-bit variables.
>> It might work most of the time, but it can also break randomly.
>
> In general inodes and offsets start from 0 and work up --
> so almost all of the time they don't actually overflow.
> The problem with ext4 directory hash "offsets" is that they
> overflow all the time and immediately, so instead of "works
> unless you have a weird edge case" like all the other filesystems,
> it's "never works".
>
>>> I think the best fix for this would be for the kernel to either
>>> (a) consistently use a 32-bit hash or (b) to provide an API
>>> so that userspace can use the FMODE_32BITHASH flag the way
>>> that kernel-internal users already can.
>>
>> It would be relatively straight forward to add a "32bitapi" mount
>> option to return a 32-bit directory hash to userspace for operations
>> on that mountpoint (ext4 doesn't have 64-bit inode numbers yet).
>> However, I can't think of an easy way to do this on a per-process
>> basis without just having it call the 32-bit API directly.
>
> The problem is that there is no 32-bit API in some cases
> (unless I have misunderstood the kernel code) -- not all
> host architectures implement compat syscalls or allow them
> to be called from 64-bit processes or implement all the older
> syscall variants that had smaller offets. If there was a guaranteed
> "this syscall always exists and always gives me 32-bit offsets"
> we could use it.
The "32bitapi" mount option would use 32-bit hash for seekdir
and telldir, regardless of what kernel API was used. That would
just set the FMODE_32BITHASH flag in the file->f_mode for all files.
Using 32-bit directory hash values is not necessarily harmful, but
it returns the possibility to hit the problem with hash collisions
that previously existed before the move to 64-bit hash values.
This becomes more of a problem as directory sizes increase.
>> For ext4 at least, you could just shift the high 32-bit part of
>> the 64-bit hash down into a 32-bit value in telldir(), and
>> shift it back up when seekdir() is called.
>
> Yes, that has been suggested, but it seemed a bit dubious
> to bake in knowledge of ext4's internal implementation details.
> Can we rely on this as an ABI promise that will always work
> for all versions of all file systems going forwards?
Well, the directory cookies need to be relatively stable over
time because they are exported to applications and possibly
remote nodes via NFS, so it can't be changed very much.
Cheers, Andreas
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
^ permalink raw reply
* [PATCH v5 RESEND] signal: add pidfd_send_signal() syscall
From: Christian Brauner @ 2018-12-28 22:48 UTC (permalink / raw)
To: linux-kernel, linux-api, luto, arnd, serge, keescook, akpm
Cc: jannh, oleg, cyphar, viro, linux-fsdevel, dancol, timmurray,
fweimer, tglx, x86, ebiederm, Christian Brauner
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]). Clarification for this has been
requested by Eric (cf. [19]).
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/
[19]: https://lore.kernel.org/lkml/20181208054059.19813-1-christian@brauner.io/
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
Signed-off-by: Christian Brauner <christian@brauner.io>
---
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 d90127298f12..b77538af7aca 100644
--- a/include/uapi/asm-generic/unistd.h
+++ b/include/uapi/asm-generic/unistd.h
@@ -740,9 +740,11 @@ __SC_COMP(__NR_io_pgetevents, sys_io_pgetevents, compat_sys_io_pgetevents)
__SYSCALL(__NR_rseq, sys_rseq)
#define __NR_kexec_file_load 294
__SYSCALL(__NR_kexec_file_load, sys_kexec_file_load)
+#define __NR_pidfd_send_signal 295
+__SYSCALL(__NR_pidfd_send_signal, sys_pidfd_send_signal)
#undef __NR_syscalls
-#define __NR_syscalls 295
+#define __NR_syscalls 296
/*
* 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
* Re: d_off field in struct dirent and 32-on-64 emulation
From: Andy Lutomirski @ 2018-12-28 15:26 UTC (permalink / raw)
To: Florian Weimer
Cc: Linux FS Devel, Linux API, linux-ext4, LKML, V9FS Developers,
libc-alpha, qemu-devel@nongnu.org Developers, Eric Van Hensbergen,
Ron Minnich, Latchesar Ionkov, H. Peter Anvin, Arnd Bergmann
In-Reply-To: <87bm56vqg4.fsf@mid.deneb.enyo.de>
[sending again, slightly edited, due to email client issues]
On Thu, Dec 27, 2018 at 9:25 AM Florian Weimer <fw@deneb.enyo.de> wrote:
>
> We have a bit of an interesting problem with respect to the d_off
> field in struct dirent.
>
> When running a 64-bit kernel on certain file systems, notably ext4,
> this field uses the full 63 bits even for small directories (strace -v
> output, wrapped here for readability):
>
> getdents(3, [
> {d_ino=1494304, d_off=3901177228673045825, d_reclen=40, d_name="authorized_keys", d_type=DT_REG},
> {d_ino=1494277, d_off=7491915799041650922, d_reclen=24, d_name=".", d_type=DT_DIR},
> {d_ino=1314655, d_off=9223372036854775807, d_reclen=24, d_name="..", d_type=DT_DIR}
> ], 32768) = 88
>
> When running in 32-bit compat mode, this value is somehow truncated to
> 31 bits, for both the getdents and the getdents64 (!) system call (at
> least on i386).
>
...
>
> However, both qemu-user and the 9p file system can run in such a way
> that the kernel is entered from a 64-bit process, but the actual usage
> is from a 32-bit process:
I imagine that at least some of the problems you're seeing are due to this bug:
https://lkml.org/lkml/2018/10/18/859
Presumably the right fix involves modifying the relevant VFS file
operations to indicate the relevant ABI to the implementations. I
would guess that 9p is triggering the “not really in the syscall you
think you’re in” issue.
^ permalink raw reply
* Re: d_off field in struct dirent and 32-on-64 emulation
From: Adhemerval Zanella @ 2018-12-28 12:21 UTC (permalink / raw)
To: Florian Weimer
Cc: linux-fsdevel, linux-api, linux-ext4, linux-kernel,
v9fs-developer, libc-alpha, qemu-devel, ericvh, lucho, hpa, arnd
In-Reply-To: <87ftuhvp08.fsf@mid.deneb.enyo.de>
On 28/12/2018 10:01, Florian Weimer wrote:
> * Florian Weimer:
>
>> * Adhemerval Zanella:
>>
>>> On 27/12/2018 16:09, Florian Weimer wrote:
>>>> * Adhemerval Zanella:
>>>>
>>>>> Also for glibc standpoint, although reverting it back to use getdents
>>>>> syscall for non-LFS mode might fix this issue for architectures that
>>>>> provides non-LFS getdents syscall it won't be a fix for architectures
>>>>> that still provides off_t different than off64_t *and* only provides
>>>>> getdents64 syscall.
>>>>>
>>>>> Currently we only have nios2 and csky (unfortunately). But since generic
>>>>> definition for off_t and off64_t still assumes non-LFS support, all new
>>>>> 32-bits ports potentially might carry the issue.
>>>>
>>>> For csky, we could still change the type of the non-standard d_off
>>>> field to long long int. This way, only telldir would have to fail
>>>> when truncation is necessary, as mentioned below:
>>>
>>> I think it makes no sense to continue making non-LFS as default for
>>> newer 32 bits ports, the support will be emulated with LFS syscalls.
>>
>> Sorry, I don't see how this matters. seekdir and telldir are NOT
>> affected by LFS.
>
> Ah, right. If struct dirent is 64-bit only, then the d_off member
> will be 64 bits as well. But it is unclear whether you can use that
> with lseek (probably yes, in its 64-bit variant), and it's unlikely
> it's going to work with seekdir because of the POSIX-required long int
> type.
>
I was referring to all other API that uses off_t as well (pread for
instance), where new ports will have non-LFS variants that will call
only LFS variants.
^ permalink raw reply
* Re: d_off field in struct dirent and 32-on-64 emulation
From: Florian Weimer @ 2018-12-28 12:01 UTC (permalink / raw)
To: Adhemerval Zanella
Cc: linux-fsdevel, linux-api, linux-ext4, linux-kernel,
v9fs-developer, libc-alpha, qemu-devel, ericvh, lucho, hpa, arnd
In-Reply-To: <87k1jtvp8u.fsf@mid.deneb.enyo.de>
* Florian Weimer:
> * Adhemerval Zanella:
>
>> On 27/12/2018 16:09, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>> Also for glibc standpoint, although reverting it back to use getdents
>>>> syscall for non-LFS mode might fix this issue for architectures that
>>>> provides non-LFS getdents syscall it won't be a fix for architectures
>>>> that still provides off_t different than off64_t *and* only provides
>>>> getdents64 syscall.
>>>>
>>>> Currently we only have nios2 and csky (unfortunately). But since generic
>>>> definition for off_t and off64_t still assumes non-LFS support, all new
>>>> 32-bits ports potentially might carry the issue.
>>>
>>> For csky, we could still change the type of the non-standard d_off
>>> field to long long int. This way, only telldir would have to fail
>>> when truncation is necessary, as mentioned below:
>>
>> I think it makes no sense to continue making non-LFS as default for
>> newer 32 bits ports, the support will be emulated with LFS syscalls.
>
> Sorry, I don't see how this matters. seekdir and telldir are NOT
> affected by LFS.
Ah, right. If struct dirent is 64-bit only, then the d_off member
will be 64 bits as well. But it is unclear whether you can use that
with lseek (probably yes, in its 64-bit variant), and it's unlikely
it's going to work with seekdir because of the POSIX-required long int
type.
^ permalink raw reply
* Re: d_off field in struct dirent and 32-on-64 emulation
From: Florian Weimer @ 2018-12-28 11:56 UTC (permalink / raw)
To: Adhemerval Zanella
Cc: linux-fsdevel, linux-api, linux-ext4, linux-kernel,
v9fs-developer, libc-alpha, qemu-devel, ericvh, rminnich, lucho,
hpa, arnd
In-Reply-To: <ae0530c9-5c46-5560-9734-1eacaf173b8d@linaro.org>
* Adhemerval Zanella:
> On 27/12/2018 16:09, Florian Weimer wrote:
>> * Adhemerval Zanella:
>>
>>> Also for glibc standpoint, although reverting it back to use getdents
>>> syscall for non-LFS mode might fix this issue for architectures that
>>> provides non-LFS getdents syscall it won't be a fix for architectures
>>> that still provides off_t different than off64_t *and* only provides
>>> getdents64 syscall.
>>>
>>> Currently we only have nios2 and csky (unfortunately). But since generic
>>> definition for off_t and off64_t still assumes non-LFS support, all new
>>> 32-bits ports potentially might carry the issue.
>>
>> For csky, we could still change the type of the non-standard d_off
>> field to long long int. This way, only telldir would have to fail
>> when truncation is necessary, as mentioned below:
>
> I think it makes no sense to continue making non-LFS as default for
> newer 32 bits ports, the support will be emulated with LFS syscalls.
Sorry, I don't see how this matters. seekdir and telldir are NOT
affected by LFS.
^ permalink raw reply
* Re: d_off field in struct dirent and 32-on-64 emulation
From: Adhemerval Zanella @ 2018-12-28 11:53 UTC (permalink / raw)
To: Florian Weimer
Cc: linux-fsdevel, linux-api, linux-ext4, linux-kernel,
v9fs-developer, libc-alpha, qemu-devel, ericvh, rminnich, lucho,
hpa, arnd
In-Reply-To: <87pntmu9iw.fsf@mid.deneb.enyo.de>
On 27/12/2018 16:09, Florian Weimer wrote:
> * Adhemerval Zanella:
>
>> Also for glibc standpoint, although reverting it back to use getdents
>> syscall for non-LFS mode might fix this issue for architectures that
>> provides non-LFS getdents syscall it won't be a fix for architectures
>> that still provides off_t different than off64_t *and* only provides
>> getdents64 syscall.
>>
>> Currently we only have nios2 and csky (unfortunately). But since generic
>> definition for off_t and off64_t still assumes non-LFS support, all new
>> 32-bits ports potentially might carry the issue.
>
> For csky, we could still change the type of the non-standard d_off
> field to long long int. This way, only telldir would have to fail
> when truncation is necessary, as mentioned below:
I think it makes no sense to continue making non-LFS as default for
newer 32 bits ports, the support will be emulated with LFS syscalls.
^ permalink raw reply
* Re: [Qemu-devel] d_off field in struct dirent and 32-on-64 emulation
From: Peter Maydell @ 2018-12-28 11:18 UTC (permalink / raw)
To: Andreas Dilger
Cc: Florian Weimer, linux-fsdevel, Linux API, Ext4 Developers List,
lucho, libc-alpha, Arnd Bergmann, ericvh, hpa,
lkml - Kernel Mailing List, QEMU Developers, rminnich,
v9fs-developer
In-Reply-To: <9C6A7D45-CF53-4C61-B5DD-12CA0D419972@dilger.ca>
On Fri, 28 Dec 2018 at 00:23, Andreas Dilger <adilger@dilger.ca> wrote:
> On Dec 27, 2018, at 10:41 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
> > As you note, this causes breakage for userspace programs which
> > need to implement an API/ABI with 32-bit offset but which only
> > have access to the kernel's 64-bit offset API/ABI.
>
> This is (IMHO) a bit of an oxymoron, isn't it? Applications using
> the 64-bit API, but storing the value in a 32-bit field?
I didn't say "which choose to store the value in a 32-bit field",
I said "which have to implement an API/ABI which has 32-bit fields".
In QEMU's case, we use the host kernel's ABI, which has 64-bit
offset fields. We implement a syscall ABI for the guest binary
we are running under emulation, which may have 32-bit offset fields
(for instance if we are running a 32-bit Arm binary.) Both of
these ABIs are fixed -- QEMU doesn't have a choice here, it
just has to make the best effort it can with what the host kernel
provides it, to provide the semantics the guest binary needs.
My suggestion in this thread is that the host kernel provides
a wider range of facilities so that QEMU can do the job it's
trying to do.
> The same
> problem would exist for filesystems with 64-bit inodes or 64-bit
> file offsets trying to store these values in 32-bit variables.
> It might work most of the time, but it can also break randomly.
In general inodes and offsets start from 0 and work up --
so almost all of the time they don't actually overflow.
The problem with ext4 directory hash "offsets" is that they
overflow all the time and immediately, so instead of "works
unless you have a weird edge case" like all the other filesystems,
it's "never works".
> > I think the best fix for this would be for the kernel to either
> > (a) consistently use a 32-bit hash or (b) to provide an API
> > so that userspace can use the FMODE_32BITHASH flag the way
> > that kernel-internal users already can.
>
> It would be relatively straight forward to add a "32bitapi" mount
> option to return a 32-bit directory hash to userspace for operations
> on that mountpoint (ext4 doesn't have 64-bit inode numbers yet).
> However, I can't think of an easy way to do this on a per-process
> basis without just having it call the 32-bit API directly.
The problem is that there is no 32-bit API in some cases
(unless I have misunderstood the kernel code) -- not all
host architectures implement compat syscalls or allow them
to be called from 64-bit processes or implement all the older
syscall variants that had smaller offets. If there was a guaranteed
"this syscall always exists and always gives me 32-bit offsets"
we could use it.
> For ext4 at least, you could just shift the high 32-bit part of
> the 64-bit hash down into a 32-bit value in telldir(), and
> shift it back up when seekdir() is called.
Yes, that has been suggested, but it seemed a bit dubious
to bake in knowledge of ext4's internal implementation details.
Can we rely on this as an ABI promise that will always work
for all versions of all file systems going forwards?
thanks
-- PMM
^ permalink raw reply
* Re: d_off field in struct dirent and 32-on-64 emulation
From: Florian Weimer @ 2018-12-28 7:38 UTC (permalink / raw)
To: Dmitry V. Levin
Cc: linux-fsdevel, linux-api, linux-ext4, linux-kernel,
v9fs-developer, libc-alpha, qemu-devel, ericvh, rminnich, lucho,
hpa, arnd
In-Reply-To: <20181228022338.GA27992@altlinux.org>
* Dmitry V. Levin:
> On Thu, Dec 27, 2018 at 06:18:19PM +0100, Florian Weimer wrote:
>> We have a bit of an interesting problem with respect to the d_off
>> field in struct dirent.
>>
>> When running a 64-bit kernel on certain file systems, notably ext4,
>> this field uses the full 63 bits even for small directories (strace -v
>> output, wrapped here for readability):
>>
>> getdents(3, [
>> {d_ino=1494304, d_off=3901177228673045825, d_reclen=40,
>> d_name="authorized_keys", d_type=DT_REG},
>> {d_ino=1494277, d_off=7491915799041650922, d_reclen=24,
>> d_name=".", d_type=DT_DIR},
>> {d_ino=1314655, d_off=9223372036854775807, d_reclen=24,
>> d_name="..", d_type=DT_DIR}
>> ], 32768) = 88
>>
>> When running in 32-bit compat mode, this value is somehow truncated to
>> 31 bits, for both the getdents and the getdents64 (!) system call (at
>> least on i386).
>
> Why getdents64 system call is affected by this truncation,
> isn't it a kernel bug that has to be fixed in the kernel instead?
It's required because POSIX specifies that telldir and seekdir use
long int (and not off_t) as the seek offset. If the kernel does not
truncate while keeping a useful value, these functions would turn
unusable.
^ permalink raw reply
* Re: d_off field in struct dirent and 32-on-64 emulation
From: Dmitry V. Levin @ 2018-12-28 2:23 UTC (permalink / raw)
To: Florian Weimer
Cc: linux-fsdevel, linux-api, linux-ext4, linux-kernel,
v9fs-developer, libc-alpha, qemu-devel, ericvh, rminnich, lucho,
hpa, arnd
In-Reply-To: <87bm56vqg4.fsf@mid.deneb.enyo.de>
[-- Attachment #1: Type: text/plain, Size: 985 bytes --]
On Thu, Dec 27, 2018 at 06:18:19PM +0100, Florian Weimer wrote:
> We have a bit of an interesting problem with respect to the d_off
> field in struct dirent.
>
> When running a 64-bit kernel on certain file systems, notably ext4,
> this field uses the full 63 bits even for small directories (strace -v
> output, wrapped here for readability):
>
> getdents(3, [
> {d_ino=1494304, d_off=3901177228673045825, d_reclen=40, d_name="authorized_keys", d_type=DT_REG},
> {d_ino=1494277, d_off=7491915799041650922, d_reclen=24, d_name=".", d_type=DT_DIR},
> {d_ino=1314655, d_off=9223372036854775807, d_reclen=24, d_name="..", d_type=DT_DIR}
> ], 32768) = 88
>
> When running in 32-bit compat mode, this value is somehow truncated to
> 31 bits, for both the getdents and the getdents64 (!) system call (at
> least on i386).
Why getdents64 system call is affected by this truncation,
isn't it a kernel bug that has to be fixed in the kernel instead?
--
ldv
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 801 bytes --]
^ permalink raw reply
* Re: [Qemu-devel] d_off field in struct dirent and 32-on-64 emulation
From: Andreas Dilger @ 2018-12-28 0:23 UTC (permalink / raw)
To: Peter Maydell
Cc: Florian Weimer, linux-fsdevel, Linux API, Ext4 Developers List,
lucho, libc-alpha, Arnd Bergmann, ericvh, hpa,
lkml - Kernel Mailing List, QEMU Developers, rminnich,
v9fs-developer
In-Reply-To: <CAFEAcA92m4vhzjJ+B=mP_o6Wfhx1XSKo3uWxah3osh=u5UXFuw@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 3621 bytes --]
On Dec 27, 2018, at 10:41 AM, Peter Maydell <peter.maydell@linaro.org> wrote:
>
> On Thu, 27 Dec 2018 at 17:19, Florian Weimer <fw@deneb.enyo.de> wrote:
>> We have a bit of an interesting problem with respect to the d_off
>> field in struct dirent.
>>
>> When running a 64-bit kernel on certain file systems, notably ext4,
>> this field uses the full 63 bits even for small directories (strace -v
>> output, wrapped here for readability):
>>
>> getdents(3, [
>> {d_ino=1494304, d_off=3901177228673045825, d_reclen=40, d_name="authorized_keys", d_type=DT_REG},
>> {d_ino=1494277, d_off=7491915799041650922, d_reclen=24, d_name=".", d_type=DT_DIR},
>> {d_ino=1314655, d_off=9223372036854775807, d_reclen=24, d_name="..", d_type=DT_DIR}
>> ], 32768) = 88
>>
>> When running in 32-bit compat mode, this value is somehow truncated to
>> 31 bits, for both the getdents and the getdents64 (!) system call (at
>> least on i386).
>
> Yes -- look for hash2pos() and friends in fs/ext4/dir.c.
> The ext4 code in the kernel uses a 32 bit hash if (a) the kernel
> is 32 bit (b) this is a compat syscall (b) some other bit of
> the kernel asked it to via the FMODE_32BITHASH flag (currently only
> NFS does that I think).
>
> As you note, this causes breakage for userspace programs which
> need to implement an API/ABI with 32-bit offset but which only
> have access to the kernel's 64-bit offset API/ABI.
This is (IMHO) a bit of an oxymoron, isn't it? Applications using
the 64-bit API, but storing the value in a 32-bit field? The same
problem would exist for filesystems with 64-bit inodes or 64-bit
file offsets trying to store these values in 32-bit variables.
It might work most of the time, but it can also break randomly.
> I think the best fix for this would be for the kernel to either
> (a) consistently use a 32-bit hash or (b) to provide an API
> so that userspace can use the FMODE_32BITHASH flag the way
> that kernel-internal users already can.
It would be relatively straight forward to add a "32bitapi" mount
option to return a 32-bit directory hash to userspace for operations
on that mountpoint (ext4 doesn't have 64-bit inode numbers yet).
However, I can't think of an easy way to do this on a per-process
basis without just having it call the 32-bit API directly.
> I couldn't think of or find any existing way for userspace
> to get the right results here, which is why
> 32-bit-guest-on-64-bit-host QEMU doesn't work on these filesystems
> (depending on what exactly the guest's libc etc do).
>
>> the 32-bit getdents system call emulation in a 64-bit qemu-user
>> process would just silently truncate the d_off field as part of
>> the translation, not reporting an error.
>> [...]
>> This truncation has always been a bug; it breaks telldir/seekdir
>> at least in some cases.
>
> Yes; you can't fit a quart into a pint pot, so if the guest
> only handles 32-bit offsets then truncation is about all we
> can do. This works fine if offsets are offsets, assuming the
> directory isn't so enormous it would have broken the guest
> anyway. I'm not aware of any issues with this other than the
> oddball ext4 offsets-are-hashes situation -- could you expand
> on the telldir/seekdir issue? (I suppose we should probably
> make QEMU's syscall emulation layer return "no more entries"
> rather than entries with truncated hashes.)
For ext4 at least, you could just shift the high 32-bit part of
the 64-bit hash down into a 32-bit value in telldir(), and
shift it back up when seekdir() is called.
Cheers, Andreas
[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]
^ permalink raw reply
* Re: d_off field in struct dirent and 32-on-64 emulation
From: Florian Weimer @ 2018-12-27 18:09 UTC (permalink / raw)
To: Adhemerval Zanella
Cc: linux-fsdevel, linux-api, linux-ext4, linux-kernel,
v9fs-developer, libc-alpha, qemu-devel, ericvh, rminnich, lucho,
hpa, arnd
In-Reply-To: <957967d7-5717-8ada-fb30-dfdf19898b6b@linaro.org>
* Adhemerval Zanella:
> Also for glibc standpoint, although reverting it back to use getdents
> syscall for non-LFS mode might fix this issue for architectures that
> provides non-LFS getdents syscall it won't be a fix for architectures
> that still provides off_t different than off64_t *and* only provides
> getdents64 syscall.
>
> Currently we only have nios2 and csky (unfortunately). But since generic
> definition for off_t and off64_t still assumes non-LFS support, all new
> 32-bits ports potentially might carry the issue.
For csky, we could still change the type of the non-standard d_off
field to long long int. This way, only telldir would have to fail
when truncation is necessary, as mentioned below:
>> There is another annoying aspect: The standards expose d_off through
>> the telldir function, and that returns long int on all architectures
>> (not off_t, so unchanged by _FILE_OFFSET_BITS). That's mostly a
>> userspace issue and thus needing different steps to resolve (possibly
>> standards action).
^ permalink raw reply
* Re: d_off field in struct dirent and 32-on-64 emulation
From: Adhemerval Zanella @ 2018-12-27 17:58 UTC (permalink / raw)
To: Florian Weimer, linux-fsdevel, linux-api, linux-ext4
Cc: linux-kernel, v9fs-developer, libc-alpha, qemu-devel, ericvh,
rminnich, lucho, hpa, arnd
In-Reply-To: <87bm56vqg4.fsf@mid.deneb.enyo.de>
On 27/12/2018 15:18, Florian Weimer wrote:
> We have a bit of an interesting problem with respect to the d_off
> field in struct dirent.
>
> When running a 64-bit kernel on certain file systems, notably ext4,
> this field uses the full 63 bits even for small directories (strace -v
> output, wrapped here for readability):
>
> getdents(3, [
> {d_ino=1494304, d_off=3901177228673045825, d_reclen=40, d_name="authorized_keys", d_type=DT_REG},
> {d_ino=1494277, d_off=7491915799041650922, d_reclen=24, d_name=".", d_type=DT_DIR},
> {d_ino=1314655, d_off=9223372036854775807, d_reclen=24, d_name="..", d_type=DT_DIR}
> ], 32768) = 88
>
> When running in 32-bit compat mode, this value is somehow truncated to
> 31 bits, for both the getdents and the getdents64 (!) system call (at
> least on i386).
>
> In an effort to simplify support for future architectures which only
> have the getdents64 system call, we changed glibc 2.28 to use the
> getdents64 system call unconditionally, and perform translation if
> necessary. This translation is noteworthy because it includes
> overflow checking for the d_ino and d_off members of struct dirent.
> We did not initially observe a regression because the kernel performs
> consistent d_off truncation (with the ext4 file system; small
> directories do not show this issue on XFS), so the overflow check does
> not fire.
>
> However, both qemu-user and the 9p file system can run in such a way
> that the kernel is entered from a 64-bit process, but the actual usage
> is from a 32-bit process:
>
> <https://sourceware.org/bugzilla/show_bug.cgi?id=23960>
>
> I think diagrammatically, this looks like this:
>
> guest process (32-bit)
> | getdents64, 32-bit UAPI
> qemu-user (64-bit)
> | getdents, 64-bit UAPI
> host kernel (64-bit)
>
> Or:
>
> guest process
> | getdents64, 32-bit UAPI
> guest kernel (64-bit)
> | 9p over virtio (64-bit d_off in struct p9_dirent)
> qemu
> | getdents, 64-bit UAPI
> host kernel (64-bit)
>
> Back when we still called getdents, in the first case, the 32-bit
> getdents system call emulation in a 64-bit qemu-user process would
> just silently truncate the d_off field as part of the translation, not
> reporting an error. The second case is more complicated, and I have
> not figured out where the truncation happens.
>
> This truncation has always been a bug; it breaks telldir/seekdir at
> least in some cases. But use of telldir/seekdir is comparatively
> rare. In contrast, now that we detect d_off overflow in glibc,
> readdir will always fail in the sketched configurations, which is bad.
> (glibc exposes the d_off field to applications, and it cannot know
> whether the application will use it or not, so there is no direct way
> to restrict the overflow error to the telldir/seekdir use case.)
>
> We could switch glibc to call getdents again if the system call is
> available. But that merely relies on the existence of the truncation
> bug somewhere else in the file system stack. This is why I don't
> think it's the right solution, just the path of least resistance.
>
> I don't want to reimplement the ext4 truncation behavior in glibc (it
> doesn't look like a straightforward truncation), and it wouldn't work
> for the second scenario where we see the 9p file system in the 32-bit
> glibc, not the ext4 file system. So that's not a good solution.
Also for glibc standpoint, although reverting it back to use getdents
syscall for non-LFS mode might fix this issue for architectures that
provides non-LFS getdents syscall it won't be a fix for architectures
that still provides off_t different than off64_t *and* only provides
getdents64 syscall.
Currently we only have nios2 and csky (unfortunately). But since generic
definition for off_t and off64_t still assumes non-LFS support, all new
32-bits ports potentially might carry the issue.
>
> There is another annoying aspect: The standards expose d_off through
> the telldir function, and that returns long int on all architectures
> (not off_t, so unchanged by _FILE_OFFSET_BITS). That's mostly a
> userspace issue and thus needing different steps to resolve (possibly
> standards action).
>
> Any suggestions how to solve this? Why does the kernel return
> different d_off values for 32-bit and 64-bit processes even when using
> getdents64, for the same directory?
>
^ permalink raw reply
* Re: d_off field in struct dirent and 32-on-64 emulation
From: Florian Weimer @ 2018-12-27 17:56 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-fsdevel, linux-api, linux-ext4, linux-kernel,
v9fs-developer, libc-alpha, qemu-devel, ericvh, lucho, hpa, arnd
In-Reply-To: <C65D3222-723F-4C0B-AF02-38488C302E84@amacapital.net>
* Andy Lutomirski:
>> On Dec 27, 2018, at 10:18 AM, Florian Weimer <fw@deneb.enyo.de> wrote:
>>
>> We have a bit of an interesting problem with respect to the d_off
>> field in struct dirent.
>>
>> When running a 64-bit kernel on certain file systems, notably ext4,
>> this field uses the full 63 bits even for small directories (strace -v
>> output, wrapped here for readability):
>>
>> getdents(3, [
>> {d_ino=1494304, d_off=3901177228673045825, d_reclen=40,
>> d_name="authorized_keys", d_type=DT_REG},
>> {d_ino=1494277, d_off=7491915799041650922, d_reclen=24, d_name=".",
>> d_type=DT_DIR},
>> {d_ino=1314655, d_off=9223372036854775807, d_reclen=24,
>> d_name="..", d_type=DT_DIR}
>> ], 32768) = 88
>>
>> When running in 32-bit compat mode, this value is somehow truncated to
>> 31 bits, for both the getdents and the getdents64 (!) system call (at
>> least on i386).
>
> I imagine you’re encountering this bug:
>
> https://lkml.org/lkml/2018/10/18/859
It's definitely in this area. However, the original collision problem
with 32-bit hashes is also real, so I can see the desire to use more
bits.
> Presumably the right fix involves modifying the relevant VFS file
> operations to indicate the relevant ABI to the implementations.
Not sure. How does NFS solve this problem when access happens from a
32-bit process and the rest (client kernel, transport, server kernel)
is 64-bit all the way?
> I would guess that 9p is triggering the “not really in the syscall you
> think you’re in” issue.
I think the issue is more like the networking case for 9p. In this
scenario, the server shouldn't have to care whether the client process
is in 32-bit mode or 64-bit mode. But maybe the only solution is to
pass through some sort of flag, as Peter Maydell has just suggested.
^ permalink raw reply
* Re: [Qemu-devel] d_off field in struct dirent and 32-on-64 emulation
From: Peter Maydell @ 2018-12-27 17:41 UTC (permalink / raw)
To: Florian Weimer
Cc: linux-fsdevel, linux-api, linux-ext4, lucho, libc-alpha,
Arnd Bergmann, ericvh, hpa, lkml - Kernel Mailing List,
QEMU Developers, rminnich, v9fs-developer
In-Reply-To: <87bm56vqg4.fsf@mid.deneb.enyo.de>
On Thu, 27 Dec 2018 at 17:19, Florian Weimer <fw@deneb.enyo.de> wrote:
> We have a bit of an interesting problem with respect to the d_off
> field in struct dirent.
>
> When running a 64-bit kernel on certain file systems, notably ext4,
> this field uses the full 63 bits even for small directories (strace -v
> output, wrapped here for readability):
>
> getdents(3, [
> {d_ino=1494304, d_off=3901177228673045825, d_reclen=40, d_name="authorized_keys", d_type=DT_REG},
> {d_ino=1494277, d_off=7491915799041650922, d_reclen=24, d_name=".", d_type=DT_DIR},
> {d_ino=1314655, d_off=9223372036854775807, d_reclen=24, d_name="..", d_type=DT_DIR}
> ], 32768) = 88
>
> When running in 32-bit compat mode, this value is somehow truncated to
> 31 bits, for both the getdents and the getdents64 (!) system call (at
> least on i386).
Yes -- look for hash2pos() and friends in fs/ext4/dir.c.
The ext4 code in the kernel uses a 32 bit hash if (a) the kernel
is 32 bit (b) this is a compat syscall (b) some other bit of
the kernel asked it to via the FMODE_32BITHASH flag (currently only
NFS does that I think).
As you note, this causes breakage for userspace programs which
need to implement an API/ABI with 32-bit offset but which only
have access to the kernel's 64-bit offset API/ABI.
I think the best fix for this would be for the kernel to either
(a) consistently use a 32-bit hash or (b) to provide an API
so that userspace can use the FMODE_32BITHASH flag the way
that kernel-internal users already can.
I couldn't think of or find any existing way for userspace
to get the right results here, which is why
32-bit-guest-on-64-bit-host QEMU doesn't work on these filesystems
(depending on what exactly the guest's libc etc do).
> the 32-bit getdents system call emulation in a 64-bit qemu-user
> process would just silently truncate the d_off field as part of
> the translation, not reporting an error.
> [...]
> This truncation has always been a bug; it breaks telldir/seekdir
> at least in some cases.
Yes; you can't fit a quart into a pint pot, so if the guest
only handles 32-bit offsets then truncation is about all we
can do. This works fine if offsets are offsets, assuming the
directory isn't so enormous it would have broken the guest
anyway. I'm not aware of any issues with this other than the
oddball ext4 offsets-are-hashes situation -- could you expand
on the telldir/seekdir issue? (I suppose we should probably
make QEMU's syscall emulation layer return "no more entries"
rather than entries with truncated hashes.)
thanks
-- PMM
^ permalink raw reply
* Re: d_off field in struct dirent and 32-on-64 emulation
From: Andy Lutomirski @ 2018-12-27 17:38 UTC (permalink / raw)
To: Florian Weimer
Cc: lucho, libc-alpha, arnd, ericvh, linux-api, hpa, linux-kernel,
qemu-devel, rminnich, linux-fsdevel, v9fs-developer, linux-ext4
In-Reply-To: <87bm56vqg4.fsf@mid.deneb.enyo.de>
> On Dec 27, 2018, at 10:18 AM, Florian Weimer <fw@deneb.enyo.de> wrote:
>
> We have a bit of an interesting problem with respect to the d_off
> field in struct dirent.
>
> When running a 64-bit kernel on certain file systems, notably ext4,
> this field uses the full 63 bits even for small directories (strace -v
> output, wrapped here for readability):
>
> getdents(3, [
> {d_ino=1494304, d_off=3901177228673045825, d_reclen=40, d_name="authorized_keys", d_type=DT_REG},
> {d_ino=1494277, d_off=7491915799041650922, d_reclen=24, d_name=".", d_type=DT_DIR},
> {d_ino=1314655, d_off=9223372036854775807, d_reclen=24, d_name="..", d_type=DT_DIR}
> ], 32768) = 88
>
> When running in 32-bit compat mode, this value is somehow truncated to
> 31 bits, for both the getdents and the getdents64 (!) system call (at
> least on i386).
I imagine you’re encountering this bug:
https://lkml.org/lkml/2018/10/18/859
Presumably the right fix involves modifying the relevant VFS file operations to indicate the relevant ABI to the implementations.
I would guess that 9p is triggering the “not really in the syscall you think you’re in” issue.
^ permalink raw reply
* d_off field in struct dirent and 32-on-64 emulation
From: Florian Weimer @ 2018-12-27 17:18 UTC (permalink / raw)
To: linux-fsdevel, linux-api, linux-ext4
Cc: linux-kernel, v9fs-developer, libc-alpha, qemu-devel, ericvh,
rminnich, lucho, hpa, arnd
We have a bit of an interesting problem with respect to the d_off
field in struct dirent.
When running a 64-bit kernel on certain file systems, notably ext4,
this field uses the full 63 bits even for small directories (strace -v
output, wrapped here for readability):
getdents(3, [
{d_ino=1494304, d_off=3901177228673045825, d_reclen=40, d_name="authorized_keys", d_type=DT_REG},
{d_ino=1494277, d_off=7491915799041650922, d_reclen=24, d_name=".", d_type=DT_DIR},
{d_ino=1314655, d_off=9223372036854775807, d_reclen=24, d_name="..", d_type=DT_DIR}
], 32768) = 88
When running in 32-bit compat mode, this value is somehow truncated to
31 bits, for both the getdents and the getdents64 (!) system call (at
least on i386).
In an effort to simplify support for future architectures which only
have the getdents64 system call, we changed glibc 2.28 to use the
getdents64 system call unconditionally, and perform translation if
necessary. This translation is noteworthy because it includes
overflow checking for the d_ino and d_off members of struct dirent.
We did not initially observe a regression because the kernel performs
consistent d_off truncation (with the ext4 file system; small
directories do not show this issue on XFS), so the overflow check does
not fire.
However, both qemu-user and the 9p file system can run in such a way
that the kernel is entered from a 64-bit process, but the actual usage
is from a 32-bit process:
<https://sourceware.org/bugzilla/show_bug.cgi?id=23960>
I think diagrammatically, this looks like this:
guest process (32-bit)
| getdents64, 32-bit UAPI
qemu-user (64-bit)
| getdents, 64-bit UAPI
host kernel (64-bit)
Or:
guest process
| getdents64, 32-bit UAPI
guest kernel (64-bit)
| 9p over virtio (64-bit d_off in struct p9_dirent)
qemu
| getdents, 64-bit UAPI
host kernel (64-bit)
Back when we still called getdents, in the first case, the 32-bit
getdents system call emulation in a 64-bit qemu-user process would
just silently truncate the d_off field as part of the translation, not
reporting an error. The second case is more complicated, and I have
not figured out where the truncation happens.
This truncation has always been a bug; it breaks telldir/seekdir at
least in some cases. But use of telldir/seekdir is comparatively
rare. In contrast, now that we detect d_off overflow in glibc,
readdir will always fail in the sketched configurations, which is bad.
(glibc exposes the d_off field to applications, and it cannot know
whether the application will use it or not, so there is no direct way
to restrict the overflow error to the telldir/seekdir use case.)
We could switch glibc to call getdents again if the system call is
available. But that merely relies on the existence of the truncation
bug somewhere else in the file system stack. This is why I don't
think it's the right solution, just the path of least resistance.
I don't want to reimplement the ext4 truncation behavior in glibc (it
doesn't look like a straightforward truncation), and it wouldn't work
for the second scenario where we see the 9p file system in the 32-bit
glibc, not the ext4 file system. So that's not a good solution.
There is another annoying aspect: The standards expose d_off through
the telldir function, and that returns long int on all architectures
(not off_t, so unchanged by _FILE_OFFSET_BITS). That's mostly a
userspace issue and thus needing different steps to resolve (possibly
standards action).
Any suggestions how to solve this? Why does the kernel return
different d_off values for 32-bit and 64-bit processes even when using
getdents64, for the same directory?
^ permalink raw reply
* Re: [GIT PULL] asm-generic: syscall table script for arch/sh
From: pr-tracker-bot @ 2018-12-26 19:20 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Rich Felker, Yoshinori Sato, Linux-sh list, y2038 Mailman List,
Linux API, Linux Kernel Mailing List, Firoz Khan, Linus Torvalds
In-Reply-To: <CAK8P3a3iAF3J8X2yj7JaCaTM_BcZ--2LZj32kkDf55mBn_cQ5g@mail.gmail.com>
The pull request you sent on Fri, 21 Dec 2018 00:28:51 +0100:
> git://git.kernel.org/pub/scm/linux/kernel/git/arnd/asm-generic.git tags/asm-generic-4.21
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/507413a5f88a2240b2c19cb4318166614e2349d9
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker
_______________________________________________
Y2038 mailing list
Y2038@lists.linaro.org
https://lists.linaro.org/mailman/listinfo/y2038
^ permalink raw reply
* Re: [PATCH v2] signal: add procfd_signal() syscall
From: Aleksa Sarai @ 2018-12-25 12:07 UTC (permalink / raw)
To: Lai Jiangshan
Cc: Christian Brauner, Florian Weimer, Andy Lutomirski,
Eric W. Biederman, LKML, Serge E. Hallyn, Jann Horn,
Andrew Morton, Oleg Nesterov, Al Viro, Linux FS Devel, Linux API,
Daniel Colascione, Tim Murray, linux-man, Kees Cook
In-Reply-To: <CAJhGHyAHKm0q4FtHBgbbfaB3g4oix76vJWgjqLYoaHvKCD3gvA@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 2270 bytes --]
On 2018-12-25, Lai Jiangshan <jiangshanlai+lkml@gmail.com> wrote:
> On Tue, Dec 25, 2018 at 1:32 PM Lai Jiangshan
> <jiangshanlai+lkml@gmail.com> wrote:
> >
> > Is it possible to avoid adding any syscall?
> >
> > Since holding /proc/pid/reg_file can also hold the pid.
> > With this guarantee, /proc/pid/uuid (universally unique identifier ) can be
> > introduced to identify tasks, the kernel generates
> > a uuid for every task when created.
> >
> > save_pid_uuid_pair_for_later_kill(int pid) {
> > /* save via /proc/$pid/uuid */
> > /* don't need to keep any fd after save */
> > }
> >
> > safe_kill(pid, uuid, sig) {
> > fd = open(/proc/$pid/uuid); /* also hold the pid until close() if
> > open() successes */
> > if (open successes and read uuid from fd and if it equals to uuid)
> > kill(pid, sig)
> > close(fd)
> > }
> >
> > All things needed to be done is to implement /proc/pid/uuid. And if pid can't
> > be recycled within 1 ticket, or the user can ensure it. The user can use
> > starttime(in /proc/pid/stat) instead.
> >
> > save_pid_starttime_pair_for_later_kill(int pid) {
> > /* save via /proc/$pid/stat */
> > /* don't need to keep any fd after save or keep it for 1 ticket at most */
> > }
> >
> > safe_kill(pid, starttime, sig) {
> > fd = open(/proc/$pid/stat); /* also hold the pid until close() if
> > open() successes */
> > if (open successes and read starttime from fd and if it equals to starttime)
> > kill(pid, sig)
> > close(fd)
> > }
> >
> > In this case, zero LOC is added in the kernel. All of it depends on
> > the guarantee that holding /proc/pid/reg_file also holds the pid,
> > one of which I haven't checked carefully either.
> >
>
> Oh, Sorry, I was wrong, the pid isn't reserved even when
> the fd is kept in the user space. And I'm sorry that I had
> replied to an "old" email thread.
Don't worry, this was a common point of confusion during this (and
sister) threads. All the fd ensures is that access through that fd will
give you -ESRCH if the process is gone (and if the PID is reused it will
still give you -ESRCH).
--
Aleksa Sarai
Senior Software Engineer (Containers)
SUSE Linux GmbH
<https://www.cyphar.com/>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]
^ permalink raw reply
* Re: [PATCH v2] signal: add procfd_signal() syscall
From: Lai Jiangshan @ 2018-12-25 7:11 UTC (permalink / raw)
To: Christian Brauner
Cc: Florian Weimer, Andy Lutomirski, Eric W. Biederman, LKML,
Serge E. Hallyn, Jann Horn, Andrew Morton, Oleg Nesterov,
Aleksa Sarai, Al Viro, Linux FS Devel, Linux API,
Daniel Colascione, Tim Murray, linux-man, Kees Cook
In-Reply-To: <CAJhGHyC=Zc=iG+iL+Ny8GsMd-5xypxdk4Et_1ZViMao5aXreCw@mail.gmail.com>
On Tue, Dec 25, 2018 at 1:32 PM Lai Jiangshan
<jiangshanlai+lkml@gmail.com> wrote:
>
> Is it possible to avoid adding any syscall?
>
> Since holding /proc/pid/reg_file can also hold the pid.
> With this guarantee, /proc/pid/uuid (universally unique identifier ) can be
> introduced to identify tasks, the kernel generates
> a uuid for every task when created.
>
> save_pid_uuid_pair_for_later_kill(int pid) {
> /* save via /proc/$pid/uuid */
> /* don't need to keep any fd after save */
> }
>
> safe_kill(pid, uuid, sig) {
> fd = open(/proc/$pid/uuid); /* also hold the pid until close() if
> open() successes */
> if (open successes and read uuid from fd and if it equals to uuid)
> kill(pid, sig)
> close(fd)
> }
>
> All things needed to be done is to implement /proc/pid/uuid. And if pid can't
> be recycled within 1 ticket, or the user can ensure it. The user can use
> starttime(in /proc/pid/stat) instead.
>
> save_pid_starttime_pair_for_later_kill(int pid) {
> /* save via /proc/$pid/stat */
> /* don't need to keep any fd after save or keep it for 1 ticket at most */
> }
>
> safe_kill(pid, starttime, sig) {
> fd = open(/proc/$pid/stat); /* also hold the pid until close() if
> open() successes */
> if (open successes and read starttime from fd and if it equals to starttime)
> kill(pid, sig)
> close(fd)
> }
>
> In this case, zero LOC is added in the kernel. All of it depends on
> the guarantee that holding /proc/pid/reg_file also holds the pid,
> one of which I haven't checked carefully either.
>
Oh, Sorry, I was wrong, the pid isn't reserved even when
the fd is kept in the user space. And I'm sorry that I had
replied to an "old" email thread.
^ permalink raw reply
* Re: [PATCH v2] signal: add procfd_signal() syscall
From: Lai Jiangshan @ 2018-12-25 5:32 UTC (permalink / raw)
To: Christian Brauner
Cc: Florian Weimer, Andy Lutomirski, Eric W. Biederman, LKML,
Serge E. Hallyn, Jann Horn, Andrew Morton, Oleg Nesterov,
Aleksa Sarai, Al Viro, Linux FS Devel, Linux API,
Daniel Colascione, Tim Murray, linux-man, Kees Cook
In-Reply-To: <C5E71146-DE79-4145-B54B-E161E7372D95@brauner.io>
Is it possible to avoid adding any syscall?
Since holding /proc/pid/reg_file can also hold the pid.
With this guarantee, /proc/pid/uuid (universally unique identifier ) can be
introduced to identify tasks, the kernel generates
a uuid for every task when created.
save_pid_uuid_pair_for_later_kill(int pid) {
/* save via /proc/$pid/uuid */
/* don't need to keep any fd after save */
}
safe_kill(pid, uuid, sig) {
fd = open(/proc/$pid/uuid); /* also hold the pid until close() if
open() successes */
if (open successes and read uuid from fd and if it equals to uuid)
kill(pid, sig)
close(fd)
}
All things needed to be done is to implement /proc/pid/uuid. And if pid can't
be recycled within 1 ticket, or the user can ensure it. The user can use
starttime(in /proc/pid/stat) instead.
save_pid_starttime_pair_for_later_kill(int pid) {
/* save via /proc/$pid/stat */
/* don't need to keep any fd after save or keep it for 1 ticket at most */
}
safe_kill(pid, starttime, sig) {
fd = open(/proc/$pid/stat); /* also hold the pid until close() if
open() successes */
if (open successes and read starttime from fd and if it equals to starttime)
kill(pid, sig)
close(fd)
}
In this case, zero LOC is added in the kernel. All of it depends on
the guarantee that holding /proc/pid/reg_file also holds the pid,
one of which I haven't checked carefully either.
On Fri, Dec 7, 2018 at 3:05 AM Christian Brauner <christian@brauner.io> wrote:
>
> On December 7, 2018 7:56:44 AM GMT+13:00, Florian Weimer <fweimer@redhat.com> wrote:
> >* Andy Lutomirski:
> >
> >>> I suppose that's fine. Or alternatively, when thread group support
> >is
> >>> added, introduce a flag that applications have to use to enable it,
> >so
> >>> that they can probe for support by checking support for the flag.
> >>>
> >>> I wouldn't be opposed to a new system call like this either:
> >>>
> >>> int procfd_open (pid_t thread_group, pid_t thread_id, unsigned
> >flags);
> >>>
> >>> But I think this is frowned upon on the kernel side.
> >>
> >> I have no problem with it, except that I think it shouldn’t return an
> >> fd that can be used for proc filesystem access.
> >
> >Oh no, my intention was that it would just be used with *_send_signal
> >and related functions.
>
> Let's postpone that discussion a little.
> I think we don't need a syscall to base this off of pids.
> As I said I rather send my revived version of CLONE_NEWFD that would serve the same task.
> The same way we could also just add a new open() flag that blocks fs access completely.
> I just pitched that idea to Serge a few days back: O_NOCHDIR or similar.
> That could even be part of Aleksa's path resolution patchset.
>
> >
> >Thanks,
> >Florian
>
^ permalink raw reply
* Re: [GIT PULL] asm-generic: syscall table script for arch/sh
From: Christoph Hellwig @ 2018-12-21 7:06 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Linus Torvalds, Rich Felker, Firoz Khan, Linux API,
y2038 Mailman List, Linux Kernel Mailing List, Linux-sh list,
Yoshinori Sato
In-Reply-To: <CAK8P3a3iAF3J8X2yj7JaCaTM_BcZ--2LZj32kkDf55mBn_cQ5g@mail.gmail.com>
> The conversion does not include the old 64-bit sh5 architecture, which
> has never shipped and not even compiled in a long time.
Maybe it is time to drop the code for it then?
^ permalink raw reply
* [GIT PULL] asm-generic: syscall table script for arch/sh
From: Arnd Bergmann @ 2018-12-20 23:28 UTC (permalink / raw)
To: Linus Torvalds
Cc: Rich Felker, Firoz Khan, Linux API, y2038 Mailman List,
Linux Kernel Mailing List, Linux-sh list, Yoshinori Sato
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.21
for you to fetch changes up to 2b3c5a99d5f314960e00950c1782eac9361de30f:
sh: generate uapi header and syscall table header files (2018-12-19
17:54:40 +0100)
----------------------------------------------------------------
asm-generic: syscall table script for arch/sh
I worked with Firoz Khan to change all architectures to have their system
call tables (syscall.S and asm/unistd.h) generated by a script from a more
readable input file the same way that we already had on x86, s390 and arm.
I offered to take those conversions through the asm-generic tree that
did not get picked up by the architecture maintainers, and fortunately
all but one have now been accepted into arch maintainer trees, so this
branch only contains the conversion for arch/sh/, with permission from
Rich.
The conversion does not include the old 64-bit sh5 architecture, which
has never shipped and not even compiled in a long time. The table
in include/uapi/asm/unistd.h is also not included here, as Firoz is
still working on that one. It will have to wait for the next following
merge window, hopefully together with the addition of the 64-bit
time_t system calls for the y2038 work that led to the system call
table rework.
Acked-by: Rich Felker <dalias@libc.org>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
----------------------------------------------------------------
Firoz Khan (3):
sh: add __NR_syscalls along with NR_syscalls
sh: add system call table generation support
sh: generate uapi header and syscall table header files
arch/sh/Makefile | 3 +
arch/sh/include/asm/Kbuild | 1 +
arch/sh/include/asm/unistd.h | 2 +
arch/sh/include/uapi/asm/Kbuild | 1 +
arch/sh/include/uapi/asm/unistd_32.h | 4 +-
arch/sh/include/uapi/asm/unistd_64.h | 4 +-
arch/sh/kernel/syscalls/Makefile | 38 +++++++++
arch/sh/kernel/syscalls/syscall.tbl | 392
++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
arch/sh/kernel/syscalls/syscallhdr.sh | 36 ++++++++
arch/sh/kernel/syscalls/syscalltbl.sh | 32 +++++++
arch/sh/kernel/syscalls_32.S | 387
+-----------------------------------------------------------------------------------
11 files changed, 514 insertions(+), 386 deletions(-)
create mode 100644 arch/sh/kernel/syscalls/Makefile
create mode 100644 arch/sh/kernel/syscalls/syscall.tbl
create mode 100644 arch/sh/kernel/syscalls/syscallhdr.sh
create mode 100644 arch/sh/kernel/syscalls/syscalltbl.sh
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox