| Message ID | 20201023181847.GA6776@ls3530.fritz.box (mailing list archive) |
|---|---|
| State | Accepted, archived |
| Headers | show |
| Series | [RFC,v3] parisc: Add wrapper syscalls to fix O_NONBLOCK flag usage | expand |
On Fri, 23 Oct 2020 20:18:47 +0200 Helge Deller <deller@gmx.de> wrote: > +static int FIX_O_NONBLOCK(int flags) > +{ > + if (flags & O_NONBLOCK_MASK_OUT) { > + struct task_struct *tsk = current; > + pr_warn_once("%s(%d) uses a deprecated O_NONBLOCK > value.\n", > + tsk->comm, tsk->pid); > + } > + return flags & ~O_NONBLOCK_MASK_OUT; > +} Would it be interesting to additionally report the calling function in search for other syscalls that might not be covered yet? Regards, jer
On Sat, 24 Oct 2020 10:22:18 +0200 Jeroen Roovers <jer@xs4all.nl> wrote: > On Fri, 23 Oct 2020 20:18:47 +0200 > Helge Deller <deller@gmx.de> wrote: > > > +static int FIX_O_NONBLOCK(int flags) > > +{ > > + if (flags & O_NONBLOCK_MASK_OUT) { > > + struct task_struct *tsk = current; > > + pr_warn_once("%s(%d) uses a deprecated O_NONBLOCK > > value.\n", > > + tsk->comm, tsk->pid); > > + } > > + return flags & ~O_NONBLOCK_MASK_OUT; > > +} > > Would it be interesting to additionally report the calling function in > search for other syscalls that might not be covered yet? Wait, that doesn't make sense, does it? Regards, jer
On 10/24/20 10:24 AM, Jeroen Roovers wrote: > On Sat, 24 Oct 2020 10:22:18 +0200 > Jeroen Roovers <jer@xs4all.nl> wrote: > >> On Fri, 23 Oct 2020 20:18:47 +0200 >> Helge Deller <deller@gmx.de> wrote: >> >>> +static int FIX_O_NONBLOCK(int flags) >>> +{ >>> + if (flags & O_NONBLOCK_MASK_OUT) { >>> + struct task_struct *tsk = current; >>> + pr_warn_once("%s(%d) uses a deprecated O_NONBLOCK >>> value.\n", >>> + tsk->comm, tsk->pid); >>> + } >>> + return flags & ~O_NONBLOCK_MASK_OUT; >>> +} >> >> Would it be interesting to additionally report the calling function in >> search for other syscalls that might not be covered yet? > > Wait, that doesn't make sense, does it? makes no sense :-) The function is only called by syscalls where we know they are affected. Helge
On Fri, 23 Oct 2020 20:18:47 +0200 Helge Deller <deller@gmx.de> wrote: > v3: Added inotify_init1() syscall wrapper > Changed warning to pr_warn_once() I do not know how pr_warn_once decides when "once" has already occurred, but with this patch I do not see any warnings at all. Regards, jer
On 2020-10-24 4:34 a.m., Helge Deller wrote: > On 10/24/20 10:24 AM, Jeroen Roovers wrote: >> On Sat, 24 Oct 2020 10:22:18 +0200 >> Jeroen Roovers <jer@xs4all.nl> wrote: >> >>> On Fri, 23 Oct 2020 20:18:47 +0200 >>> Helge Deller <deller@gmx.de> wrote: >>> >>>> +static int FIX_O_NONBLOCK(int flags) >>>> +{ >>>> + if (flags & O_NONBLOCK_MASK_OUT) { >>>> + struct task_struct *tsk = current; >>>> + pr_warn_once("%s(%d) uses a deprecated O_NONBLOCK >>>> value.\n", >>>> + tsk->comm, tsk->pid); >>>> + } >>>> + return flags & ~O_NONBLOCK_MASK_OUT; >>>> +} >>> Would it be interesting to additionally report the calling function in >>> search for other syscalls that might not be covered yet? >> Wait, that doesn't make sense, does it? > makes no sense :-) > The function is only called by syscalls where we know they are affected. I tend to think the warning is annoying. We will have to keep compatibility with old binaries forever. Dave
On 10/24/20 11:59 AM, Jeroen Roovers wrote: > On Fri, 23 Oct 2020 20:18:47 +0200 > Helge Deller <deller@gmx.de> wrote: > > >> v3: Added inotify_init1() syscall wrapper >> Changed warning to pr_warn_once() > > I do not know how pr_warn_once decides when "once" has already > occurred It's a static variable.... > , but with this patch I do not see any warnings at all. Works for me: [ 19.944173] systemd[1]: systemd 246-2 running in system mode. (+PAM +AUDIT +SELINUX +IMA +APPARMOR +SMACK +SYSVINIT +UTMP +LIBCRYPTSETUP +GCRYPT +GNUTLS +ACL +XZ +LZ4 +ZSTD -SECCOMP +BLKID +ELFUTILS +KMOD +IDN2 -IDN +PCRE2 default-hierarchy=hybrid) [ 19.954344] systemd[1]: Detected architecture parisc. Welcome to Debian GNU/Linux bullseye/sid! [ 20.254667] systemd[1]: Set hostname to <debian>. [ 20.374343] systemd(1) uses a deprecated O_NONBLOCK value. .... Helge
diff --git a/arch/parisc/kernel/sys_parisc.c b/arch/parisc/kernel/sys_parisc.c index 5d458a44b09c..9549496f5523 100644 --- a/arch/parisc/kernel/sys_parisc.c +++ b/arch/parisc/kernel/sys_parisc.c @@ -6,7 +6,7 @@ * Copyright (C) 1999-2003 Matthew Wilcox <willy at parisc-linux.org> * Copyright (C) 2000-2003 Paul Bame <bame at parisc-linux.org> * Copyright (C) 2001 Thomas Bogendoerfer <tsbogend at parisc-linux.org> - * Copyright (C) 1999-2014 Helge Deller <deller@gmx.de> + * Copyright (C) 1999-2020 Helge Deller <deller@gmx.de> */ #include <linux/uaccess.h> @@ -23,6 +23,7 @@ #include <linux/utsname.h> #include <linux/personality.h> #include <linux/random.h> +#include <linux/compat.h> /* we construct an artificial offset for the mapping based on the physical * address of the kernel mapping variable */ @@ -373,3 +374,73 @@ long parisc_personality(unsigned long personality) return err; } + +/* + * Up to kernel v5.9 we defined O_NONBLOCK as 000200004, + * since then O_NONBLOCK is defined as 000200000. + * + * The following wrapper functions mask out the old + * O_NDELAY bit from calls which use O_NONBLOCK. + * + * XXX: Remove those in year 2022 (or later)? + */ + +#define O_NONBLOCK_OLD 000200004 +#define O_NONBLOCK_MASK_OUT (O_NONBLOCK_OLD & ~O_NONBLOCK) + +static int FIX_O_NONBLOCK(int flags) +{ + if (flags & O_NONBLOCK_MASK_OUT) { + struct task_struct *tsk = current; + pr_warn_once("%s(%d) uses a deprecated O_NONBLOCK value.\n", + tsk->comm, tsk->pid); + } + return flags & ~O_NONBLOCK_MASK_OUT; +} + +asmlinkage long parisc_timerfd_create(int clockid, int flags) +{ + flags = FIX_O_NONBLOCK(flags); + return sys_timerfd_create(clockid, flags); +} + +asmlinkage long parisc_signalfd4(int ufd, sigset_t __user *user_mask, + size_t sizemask, int flags) +{ + flags = FIX_O_NONBLOCK(flags); + return sys_signalfd4(ufd, user_mask, sizemask, flags); +} + +#ifdef CONFIG_COMPAT +asmlinkage long parisc_compat_signalfd4(int ufd, + compat_sigset_t __user *user_mask, + compat_size_t sizemask, int flags) +{ + flags = FIX_O_NONBLOCK(flags); + return compat_sys_signalfd4(ufd, user_mask, sizemask, flags); +} +#endif + +asmlinkage long parisc_eventfd2(unsigned int count, int flags) +{ + flags = FIX_O_NONBLOCK(flags); + return sys_eventfd2(count, flags); +} + +asmlinkage long parisc_userfaultfd(int flags) +{ + flags = FIX_O_NONBLOCK(flags); + return sys_userfaultfd(flags); +} + +asmlinkage long parisc_pipe2(int __user *fildes, int flags) +{ + flags = FIX_O_NONBLOCK(flags); + return sys_pipe2(fildes, flags); +} + +asmlinkage long parisc_inotify_init1(int flags) +{ + flags = FIX_O_NONBLOCK(flags); + return sys_inotify_init1(flags); +} diff --git a/arch/parisc/kernel/syscalls/syscall.tbl b/arch/parisc/kernel/syscalls/syscall.tbl index 38c63e5404bc..f375ea528e59 100644 --- a/arch/parisc/kernel/syscalls/syscall.tbl +++ b/arch/parisc/kernel/syscalls/syscall.tbl @@ -344,17 +344,17 @@ 304 common eventfd sys_eventfd 305 32 fallocate parisc_fallocate 305 64 fallocate sys_fallocate -306 common timerfd_create sys_timerfd_create +306 common timerfd_create parisc_timerfd_create 307 32 timerfd_settime sys_timerfd_settime32 307 64 timerfd_settime sys_timerfd_settime 308 32 timerfd_gettime sys_timerfd_gettime32 308 64 timerfd_gettime sys_timerfd_gettime -309 common signalfd4 sys_signalfd4 compat_sys_signalfd4 -310 common eventfd2 sys_eventfd2 +309 common signalfd4 parisc_signalfd4 parisc_compat_signalfd4 +310 common eventfd2 parisc_eventfd2 311 common epoll_create1 sys_epoll_create1 312 common dup3 sys_dup3 -313 common pipe2 sys_pipe2 -314 common inotify_init1 sys_inotify_init1 +313 common pipe2 parisc_pipe2 +314 common inotify_init1 parisc_inotify_init1 315 common preadv sys_preadv compat_sys_preadv 316 common pwritev sys_pwritev compat_sys_pwritev 317 common rt_tgsigqueueinfo sys_rt_tgsigqueueinfo compat_sys_rt_tgsigqueueinfo @@ -387,7 +387,7 @@ 341 common bpf sys_bpf 342 common execveat sys_execveat compat_sys_execveat 343 common membarrier sys_membarrier -344 common userfaultfd sys_userfaultfd +344 common userfaultfd parisc_userfaultfd 345 common mlock2 sys_mlock2 346 common copy_file_range sys_copy_file_range 347 common preadv2 sys_preadv2 compat_sys_preadv2
The commit 75ae04206a4d ("parisc: Define O_NONBLOCK to become 000200000") changed the O_NONBLOCK constant to have only one bit set (like all other architectures). This change broke some existing userspace code (e.g. udevadm, systemd-udevd, elogind) which called specific syscalls which do strict value checking on their flag parameter. This patch adds wrapper functions for the relevant syscalls. The wrappers masks out any old invalid O_NONBLOCK flags, reports in the syslog if the old O_NONBLOCK value was used and then calls the target syscall with the new O_NONBLOCK value. Fixes: 75ae04206a4d ("parisc: Define O_NONBLOCK to become 000200000") Signed-off-by: Helge Deller <deller@gmx.de> Tested-by: Meelis Roos <mroos@linux.ee> Tested-by: Jeroen Roovers <jer@xs4all.nl> -- v3: Added inotify_init1() syscall wrapper Changed warning to pr_warn_once()