[RFC PATCH] parisc: Define O_NONBLOCK to become 000200000

[RFC PATCH] parisc: Define O_NONBLOCK to become 000200000

[Helge Deller]

HPUX has separate NDELAY & NONBLOCK values. In the past we wanted to
be able to run HP-UX binaries natively on parisc Linux which is why
we defined O_NONBLOCK to 000200004 to distinguish NDELAY & NONBLOCK
bits.
But with 2 bits set in this bitmask we often ran into compatibility
issues with other Linux applications which often only test one bit (or
even compare the values).

To avoid such issues in the future, this patch changes O_NONBLOCK to
become 000200000. That way old programs will still be functional, and
for new programs we now have only one bit set.

Update the comment about SOCK_NONBLOCK too.

Signed-off-by: Helge Deller <deller@gmx.de>

diff --git a/arch/parisc/include/asm/socket.h b/arch/parisc/include/asm/socket.h
index 79feff1b0721..33500c9f6e5e 100644
--- a/arch/parisc/include/asm/socket.h
+++ b/arch/parisc/include/asm/socket.h
@@ -4,8 +4,8 @@

 #include <uapi/asm/socket.h>

-/* O_NONBLOCK clashes with the bits used for socket types.  Therefore we
- * have to define SOCK_NONBLOCK to a different value here.
+/* O_NONBLOCK clashed with the bits used for socket types.  Therefore we
+ * had to define SOCK_NONBLOCK to a different value here.
  */
 #define SOCK_NONBLOCK	0x40000000

diff --git a/arch/parisc/include/uapi/asm/fcntl.h b/arch/parisc/include/uapi/asm/fcntl.h
index e19adc65b1d0..03dee816cb13 100644
--- a/arch/parisc/include/uapi/asm/fcntl.h
+++ b/arch/parisc/include/uapi/asm/fcntl.h
@@ -8,7 +8,7 @@
 #define O_LARGEFILE	000004000
 #define __O_SYNC	000100000
 #define O_SYNC		(__O_SYNC|O_DSYNC)
-#define O_NONBLOCK	000200004 /* HPUX has separate NDELAY & NONBLOCK */
+#define O_NONBLOCK	000200000
 #define O_NOCTTY	000400000 /* not fcntl */
 #define O_DSYNC		001000000
 #define O_NOATIME	004000000

Re: [RFC PATCH] parisc: Define O_NONBLOCK to become 000200000

[Jeroen Roovers]

On Sat, 29 Aug 2020 14:20:17 +0200
Helge Deller <deller@gmx.de> wrote:

> HPUX has separate NDELAY & NONBLOCK values. In the past we wanted to
> be able to run HP-UX binaries natively on parisc Linux which is why
> we defined O_NONBLOCK to 000200004 to distinguish NDELAY & NONBLOCK
> bits.
> But with 2 bits set in this bitmask we often ran into compatibility
> issues with other Linux applications which often only test one bit (or
> even compare the values).
> 
> To avoid such issues in the future, this patch changes O_NONBLOCK to
> become 000200000. That way old programs will still be functional, and
> for new programs we now have only one bit set.

I am seeing a problem with this exact commit in userland, so I think
that last sentence is incorrect:

/usr/src/linux # git bisect good
75ae04206a4d0e4f541c1d692b7febd1c0fdb814 is the first bad commit
commit 75ae04206a4d0e4f541c1d692b7febd1c0fdb814
Author: Helge Deller <deller@gmx.de>
Date:   Sat Aug 29 14:11:35 2020 +0200

    parisc: Define O_NONBLOCK to become 000200000

    HPUX has separate NDELAY & NONBLOCK values. In the past we wanted to
    be able to run HP-UX binaries natively on parisc Linux which is why
    we defined O_NONBLOCK to 000200004 to distinguish NDELAY & NONBLOCK
    bits.
    But with 2 bits set in this bitmask we often ran into compatibility
    issues with other Linux applications which often only test one bit
    (or even compare the values).

    To avoid such issues in the future, this patch changes O_NONBLOCK to
    become 000200000. That way old programs will still be functional,
    and for new programs we now have only one bit set.

    Update the comment about SOCK_NONBLOCK too.

    Signed-off-by: Helge Deller <deller@gmx.de>

 arch/parisc/include/asm/socket.h     | 4 ++--
 arch/parisc/include/uapi/asm/fcntl.h | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)


The first sign (in the boot process) that something is wrong is that
idmapd fails to start:

 * Starting idmapd ...
 * make sure DNOTIFY support is enabled ...
 [ !! ]
 * ERROR: rpc.idmapd failed to start
 * ERROR: cannot start nfsclient as rpc.idmapd would not start

Then, elogind reports an error when I ssh in as regular user:

[  297.825133][ T4273] elogind-daemon[4273]: Failed to register SIGHUP
handler: Invalid argument
[  297.825133][ T4273] elogind-daemon[4273]: Failed to register SIGHUP
handler: Invalid argument [  298.040379][ T4273] elogind-daemon[4273]:
Failed to fully start up daemon: Invalid argument
[  298.040379][T4273] elogind-daemon[4273]: Failed to fully start up
daemon: Invalid argument

Yet the unprivileged user succeeds in logging in over SSH. Following
that, sudo fails:

jeroen@karsten ~ $ sudo -i
sudo: unable to allocate memory

root can still login on the serial console and over SSH.

Would it make sense to rebuild libc against the newer kernel headers?

Or is this an unexpected result from the above commit and would it be
useful to figure out what is going on while the bad kernel is running?


Regards,
     jer

Re: [RFC PATCH] parisc: Define O_NONBLOCK to become 000200000

[Helge Deller]

On 10/20/20 7:21 PM, Jeroen Roovers wrote:
> On Sat, 29 Aug 2020 14:20:17 +0200
> Helge Deller <deller@gmx.de> wrote:
>
>> HPUX has separate NDELAY & NONBLOCK values. In the past we wanted to
>> be able to run HP-UX binaries natively on parisc Linux which is why
>> we defined O_NONBLOCK to 000200004 to distinguish NDELAY & NONBLOCK
>> bits.
>> But with 2 bits set in this bitmask we often ran into compatibility
>> issues with other Linux applications which often only test one bit (or
>> even compare the values).
>>
>> To avoid such issues in the future, this patch changes O_NONBLOCK to
>> become 000200000. That way old programs will still be functional, and
>> for new programs we now have only one bit set.
>
> I am seeing a problem with this exact commit in userland, so I think
> that last sentence is incorrect:

Thanks for testing and bisecting!!!

I'm fine with reverting the change, but we really need to
analyze what is broken (and why).

In general the kernel sources seem ok as it's important,
that code just check if bits are set, not if the value
is equal to something e.g.
good:  if (flags & O_NONBLOCK) { ... }
bad:   if (flags == O_NONBLOCK) { .... }


> The first sign (in the boot process) that something is wrong is that
> idmapd fails to start:
>
>  * Starting idmapd ...
>  * make sure DNOTIFY support is enabled ...
>  [ !! ]
>  * ERROR: rpc.idmapd failed to start
>  * ERROR: cannot start nfsclient as rpc.idmapd would not start

Could you try an strace on it?
idmapd is from glibc, so I'll look into it too.

> Then, elogind reports an error when I ssh in as regular user:
>
> [  297.825133][ T4273] elogind-daemon[4273]: Failed to register SIGHUP
> handler: Invalid argument
> [  297.825133][ T4273] elogind-daemon[4273]: Failed to register SIGHUP
> handler: Invalid argument [  298.040379][ T4273] elogind-daemon[4273]:
> Failed to fully start up daemon: Invalid argument
> [  298.040379][T4273] elogind-daemon[4273]: Failed to fully start up
> daemon: Invalid argument
>
> Yet the unprivileged user succeeds in logging in over SSH. Following
> that, sudo fails:
>
> jeroen@karsten ~ $ sudo -i
> sudo: unable to allocate memory
>
> root can still login on the serial console and over SSH.

At first thought I assume those issues are not related to the O_NONBLOCK
patch. Can you try strace on the sudo too ?

> Would it make sense to rebuild libc against the newer kernel headers?

Yes, might make sense, but then my patch isn't compatible.
So, I'd like to avoid that.

> Or is this an unexpected result from the above commit and would it be
> useful to figure out what is going on while the bad kernel is running?

As I said, idmapd might be related, the elogind/sudo tests needs checking.

Helge

Re: [RFC PATCH] parisc: Define O_NONBLOCK to become 000200000

[Jeroen Roovers]

On Wed, 21 Oct 2020 08:07:15 +0200
Helge Deller <deller@gmx.de> wrote:

> On 10/20/20 7:21 PM, Jeroen Roovers wrote:
> > On Sat, 29 Aug 2020 14:20:17 +0200
> > Helge Deller <deller@gmx.de> wrote:
> >  
> >> HPUX has separate NDELAY & NONBLOCK values. In the past we wanted
> >> to be able to run HP-UX binaries natively on parisc Linux which is
> >> why we defined O_NONBLOCK to 000200004 to distinguish NDELAY &
> >> NONBLOCK bits.
> >> But with 2 bits set in this bitmask we often ran into compatibility
> >> issues with other Linux applications which often only test one bit
> >> (or even compare the values).
> >>
> >> To avoid such issues in the future, this patch changes O_NONBLOCK
> >> to become 000200000. That way old programs will still be
> >> functional, and for new programs we now have only one bit set.  
> >
> > I am seeing a problem with this exact commit in userland, so I think
> > that last sentence is incorrect:  
> 
> Thanks for testing and bisecting!!!
> 
> I'm fine with reverting the change, but we really need to
> analyze what is broken (and why).
> 
> In general the kernel sources seem ok as it's important,
> that code just check if bits are set, not if the value
> is equal to something e.g.
> good:  if (flags & O_NONBLOCK) { ... }
> bad:   if (flags == O_NONBLOCK) { .... }
> 
> 
> > The first sign (in the boot process) that something is wrong is that
> > idmapd fails to start:
> >
> >  * Starting idmapd ...
> >  * make sure DNOTIFY support is enabled ...
> >  [ !! ]
> >  * ERROR: rpc.idmapd failed to start
> >  * ERROR: cannot start nfsclient as rpc.idmapd would not start  
> 
> Could you try an strace on it?

[after editing the startup script to run `strace -f .. rpc.idmapd`:]

https://rooversj.home.xs4all.nl/rpc.idmapd.strace

> idmapd is from glibc, so I'll look into it too.
> 
> > Then, elogind reports an error when I ssh in as regular user:
> >
> > [  297.825133][ T4273] elogind-daemon[4273]: Failed to register
> > SIGHUP handler: Invalid argument
> > [  297.825133][ T4273] elogind-daemon[4273]: Failed to register
> > SIGHUP handler: Invalid argument [  298.040379][ T4273]
> > elogind-daemon[4273]: Failed to fully start up daemon: Invalid
> > argument [  298.040379][T4273] elogind-daemon[4273]: Failed to
> > fully start up daemon: Invalid argument

strace -f -o /tmp/elogind.strace /lib/elogind/elogind

https://rooversj.home.xs4all.nl/elogind.strace

> >
> > Yet the unprivileged user succeeds in logging in over SSH. Following
> > that, sudo fails:
> >
> > jeroen@karsten ~ $ sudo -i
> > sudo: unable to allocate memory
> >
> > root can still login on the serial console and over SSH.  
> 
> At first thought I assume those issues are not related to the
> O_NONBLOCK patch. Can you try strace on the sudo too ?

strace -f -u jeroen sudo -i
[...]
pipe2(0x42f4712c, O_NONBLOCK|O_CLOEXEC) = -1 EINVAL (Invalid argument)
openat(AT_FDCWD, 0xfadd9b50, O_RDONLY|O_CLOEXEC) = 3
fstat64(3, 0xfadd9e88)                  = 0
read(3, 0x42f47258, 4096)               = 2998
read(3, "", 4096)                       = 0
close(3)                                = 0
openat(AT_FDCWD, 0x42f46bc0, O_RDONLY)  = -1 ENOENT (No such file or
directory) openat(AT_FDCWD, 0x42f46f48, O_RDONLY)  = -1 ENOENT (No such
file or directory) openat(AT_FDCWD, 0x42f46fc8, O_RDONLY)  = -1 ENOENT
(No such file or directory) openat(AT_FDCWD, 0x42f46f78, O_RDONLY)  =
-1 ENOENT (No such file or directory) ioctl(2, TCGETS, 0xfadd9c08)
      = 0 write(2, 0xfadd7f46, 4sudo)                 = 4
ioctl(2, TCGETS, 0xfadd9c08)            = 0
write(2, 0xf8151e94, 2: )                 = 2
ioctl(2, TCGETS, 0xfadd9c08)            = 0
write(2, 0xfadd9608, 25unable to allocate memory)                = 25
ioctl(2, TCGETS, 0xfadd9c08)            = 0
write(2, 0x42e39934, 2
)                 = 2
exit_group(1)                           = ?
+++ exited with 1 +++

https://rooversj.home.xs4all.nl/sudo-i.strace


Regards,
     jer

Re: [RFC PATCH] parisc: Define O_NONBLOCK to become 000200000

[Helge Deller]

On 10/22/20 5:38 PM, Jeroen Roovers wrote:
> On Wed, 21 Oct 2020 08:07:15 +0200
> Helge Deller <deller@gmx.de> wrote:
>
>> On 10/20/20 7:21 PM, Jeroen Roovers wrote:
>>> On Sat, 29 Aug 2020 14:20:17 +0200
>>> Helge Deller <deller@gmx.de> wrote:
>>>
>>>> HPUX has separate NDELAY & NONBLOCK values. In the past we wanted
>>>> to be able to run HP-UX binaries natively on parisc Linux which is
>>>> why we defined O_NONBLOCK to 000200004 to distinguish NDELAY &
>>>> NONBLOCK bits.
>>>> But with 2 bits set in this bitmask we often ran into compatibility
>>>> issues with other Linux applications which often only test one bit
>>>> (or even compare the values).
>>>>
>>>> To avoid such issues in the future, this patch changes O_NONBLOCK
>>>> to become 000200000. That way old programs will still be
>>>> functional, and for new programs we now have only one bit set.
>>>
>>> I am seeing a problem with this exact commit in userland, so I think
>>> that last sentence is incorrect:
>>
>> Thanks for testing and bisecting!!!
>>
>> I'm fine with reverting the change, but we really need to
>> analyze what is broken (and why).
>>
>> In general the kernel sources seem ok as it's important,
>> that code just check if bits are set, not if the value
>> is equal to something e.g.
>> good:  if (flags & O_NONBLOCK) { ... }
>> bad:   if (flags == O_NONBLOCK) { .... }
>>
>>
>>> The first sign (in the boot process) that something is wrong is that
>>> idmapd fails to start:
>>>
>>>  * Starting idmapd ...
>>>  * make sure DNOTIFY support is enabled ...
>>>  [ !! ]
>>>  * ERROR: rpc.idmapd failed to start
>>>  * ERROR: cannot start nfsclient as rpc.idmapd would not start
>>
>> Could you try an strace on it?
>
> [after editing the startup script to run `strace -f .. rpc.idmapd`:]
>
> https://rooversj.home.xs4all.nl/rpc.idmapd.strace
>
>> idmapd is from glibc, so I'll look into it too.
>>
>>> Then, elogind reports an error when I ssh in as regular user:
>>>
>>> [  297.825133][ T4273] elogind-daemon[4273]: Failed to register
>>> SIGHUP handler: Invalid argument
>>> [  297.825133][ T4273] elogind-daemon[4273]: Failed to register
>>> SIGHUP handler: Invalid argument [  298.040379][ T4273]
>>> elogind-daemon[4273]: Failed to fully start up daemon: Invalid
>>> argument [  298.040379][T4273] elogind-daemon[4273]: Failed to
>>> fully start up daemon: Invalid argument
>
> strace -f -o /tmp/elogind.strace /lib/elogind/elogind
>
> https://rooversj.home.xs4all.nl/elogind.strace
>
>>>
>>> Yet the unprivileged user succeeds in logging in over SSH. Following
>>> that, sudo fails:
>>>
>>> jeroen@karsten ~ $ sudo -i
>>> sudo: unable to allocate memory
>>>
>>> root can still login on the serial console and over SSH.
>>
>> At first thought I assume those issues are not related to the
>> O_NONBLOCK patch. Can you try strace on the sudo too ?
>
> strace -f -u jeroen sudo -i
> [...]
> pipe2(0x42f4712c, O_NONBLOCK|O_CLOEXEC) = -1 EINVAL (Invalid argument)
> openat(AT_FDCWD, 0xfadd9b50, O_RDONLY|O_CLOEXEC) = 3
> fstat64(3, 0xfadd9e88)                  = 0
> read(3, 0x42f47258, 4096)               = 2998
> read(3, "", 4096)                       = 0
> close(3)                                = 0
> openat(AT_FDCWD, 0x42f46bc0, O_RDONLY)  = -1 ENOENT (No such file or
> directory) openat(AT_FDCWD, 0x42f46f48, O_RDONLY)  = -1 ENOENT (No such
> file or directory) openat(AT_FDCWD, 0x42f46fc8, O_RDONLY)  = -1 ENOENT
> (No such file or directory) openat(AT_FDCWD, 0x42f46f78, O_RDONLY)  =
> -1 ENOENT (No such file or directory) ioctl(2, TCGETS, 0xfadd9c08)
>       = 0 write(2, 0xfadd7f46, 4sudo)                 = 4
> ioctl(2, TCGETS, 0xfadd9c08)            = 0
> write(2, 0xf8151e94, 2: )                 = 2
> ioctl(2, TCGETS, 0xfadd9c08)            = 0
> write(2, 0xfadd9608, 25unable to allocate memory)                = 25
> ioctl(2, TCGETS, 0xfadd9c08)            = 0
> write(2, 0x42e39934, 2
> )                 = 2
> exit_group(1)                           = ?
> +++ exited with 1 +++
>
> https://rooversj.home.xs4all.nl/sudo-i.strace

Thanks!

I found the issue.
The syscalls timerfd_create(), signalfd4(), eventfd2(), userfaultfd()
and pipe2() have a fcntl flags parameter which is checked hard.
They return EINVAL and as such the programs fail.
I found systemd-udevd and udevadm failing because of this.
sudo and elogind seem affected too.
I'm sending a RFC patch in a few minutes.

Helge

[RFC PATCH] parisc: Add wrapper syscalls to fix O_NONBLOCK flag usage

[Helge Deller]

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>

diff --git a/arch/parisc/kernel/sys_parisc.c b/arch/parisc/kernel/sys_parisc.c
index 5d458a44b09c..6dbb2315a80d 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,8 @@
 #include <linux/utsname.h>
 #include <linux/personality.h>
 #include <linux/random.h>
+#include <linux/compat.h>
+#include <linux/version.h>

 /* we construct an artificial offset for the mapping based on the physical
  * address of the kernel mapping variable */
@@ -373,3 +375,69 @@ 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.
+ */
+
+#if ((LINUX_VERSION_CODE >> 16) & 0x0ff) > 5
+#warning "Remove O_NONBLOCK compatibility wrappers now?"
+#endif
+
+#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("%s(%d) uses old O_NONBLOCK value. "
+			"Please recompile the application.\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);
+}
diff --git a/arch/parisc/kernel/syscalls/syscall.tbl b/arch/parisc/kernel/syscalls/syscall.tbl
index def64d221cd4..20ad16e762a1 100644
--- a/arch/parisc/kernel/syscalls/syscall.tbl
+++ b/arch/parisc/kernel/syscalls/syscall.tbl
@@ -344,16 +344,16 @@
 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
+313	common	pipe2			parisc_pipe2
 314	common	inotify_init1		sys_inotify_init1
 315	common	preadv	sys_preadv	compat_sys_preadv
 316	common	pwritev	sys_pwritev	compat_sys_pwritev
@@ -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

Re: [RFC PATCH] parisc: Add wrapper syscalls to fix O_NONBLOCK flag usage

[Meelis Roos]

22.10.20 19:40 Helge Deller wrotw:
> 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>

Works for me on RP2470 - boots up in full functionality and logs the recompilation
warning about systemd-udevd and syslog-ng.

Tested-by: Meelis Roos <mroos@linux.ee>

-- 
Meelis Roos

Re: [RFC PATCH] parisc: Add wrapper syscalls to fix O_NONBLOCK flag usage

[Helge Deller]

On 10/22/20 9:11 PM, Meelis Roos wrote:
>
> 22.10.20 19:40 Helge Deller wrotw:
>> 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>
>
> Works for me on RP2470 - boots up in full functionality and logs the recompilation
> warning about systemd-udevd and syslog-ng.
>
> Tested-by: Meelis Roos <mroos@linux.ee>

Thank you Meelis & Jeroen for testing!

The big question is:
We have two options
a) revert my original commit 75ae04206a4d ("parisc: Define O_NONBLOCK to become 000200000"), or
b) apply and submit this new patch on top of it.

The benefit in b) is that we will get long term rid of the two-bit
O_NONBLOCK define and thus will get more compatible to other Linux
architectures. This comes with the downside of (at least for a few
years) added overhead for those non-performance critical syscalls.

The benefit with a) is that we then step back again and stay compatible
now. The downside is, that in the future we may run into other issues
and need to keep special-handling in some other syscalls forever.

I'm still for going with b), and I hope we got all corner cases ruled out.
Opinions?

Helge

Re: [RFC PATCH] parisc: Add wrapper syscalls to fix O_NONBLOCK flag usage

[Jeroen Roovers]

On Thu, 22 Oct 2020 22:29:18 +0200
Helge Deller <deller@gmx.de> wrote:

> On 10/22/20 9:11 PM, Meelis Roos wrote:
> >
> > 22.10.20 19:40 Helge Deller wrotw:  
> >> 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>  
> >
> > Works for me on RP2470 - boots up in full functionality and logs
> > the recompilation warning about systemd-udevd and syslog-ng.
> >
> > Tested-by: Meelis Roos <mroos@linux.ee>  
> 
> Thank you Meelis & Jeroen for testing!
> 
> The big question is:
> We have two options
> a) revert my original commit 75ae04206a4d ("parisc: Define O_NONBLOCK
> to become 000200000"), or b) apply and submit this new patch on top
> of it.
> 
> The benefit in b) is that we will get long term rid of the two-bit
> O_NONBLOCK define and thus will get more compatible to other Linux
> architectures. This comes with the downside of (at least for a few
> years) added overhead for those non-performance critical syscalls.

The performance issue is resolved once the installed kernel
headers/libc have been updated accordingly. I think after that the
overhead should be minimal.

> 
> The benefit with a) is that we then step back again and stay
> compatible now. The downside is, that in the future we may run into
> other issues and need to keep special-handling in some other syscalls
> forever.
> 
> I'm still for going with b), and I hope we got all corner cases ruled
> out. Opinions?

I think the performance penalty isn't that great, but could be
improved, so I'd go for b) with a small change. The warning that it
issues seems redundant, because the immediate problem has already been
"solved", and because the proposed solution does not work:

+{
+	if (flags & O_NONBLOCK_MASK_OUT) {
+		struct task_struct *tsk = current;
+		pr_warn("%s(%d) uses old O_NONBLOCK value. "
+			"Please recompile the application.\n",
+			tsk->comm, tsk->pid);
+	}
+	return flags & ~O_NONBLOCK_MASK_OUT;
+}
+

The text assumes that the officially packaged kernel headers are in
sync with the kernel, which normally isn't the case as most
distributions seem to either pick an LTS kernel for their toolchain, or
keep the installed kernel in sync with the installed kernel headers,
but do not prevent running newer kernels and may even encourage doing
so. Simply recompiling the programs that use the old O_NONBLOCK value
does not solve the problem in most cases.

If you'd remove that if() { pr_warn } entirely, you'd probably be
rid of most of the performance penalty anyway.


Regards,
     jer

Re: [RFC PATCH] parisc: Add wrapper syscalls to fix O_NONBLOCK flag usage

[Helge Deller]

On 10/23/20 9:02 AM, Jeroen Roovers wrote:
> On Thu, 22 Oct 2020 22:29:18 +0200
> Helge Deller <deller@gmx.de> wrote:
>
>> On 10/22/20 9:11 PM, Meelis Roos wrote:
>>>
>>> 22.10.20 19:40 Helge Deller wrotw:
>>>> 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>
>>>
>>> Works for me on RP2470 - boots up in full functionality and logs
>>> the recompilation warning about systemd-udevd and syslog-ng.
>>>
>>> Tested-by: Meelis Roos <mroos@linux.ee>
>>
>> Thank you Meelis & Jeroen for testing!
>>
>> The big question is:
>> We have two options
>> a) revert my original commit 75ae04206a4d ("parisc: Define O_NONBLOCK
>> to become 000200000"), or b) apply and submit this new patch on top
>> of it.
>>
>> The benefit in b) is that we will get long term rid of the two-bit
>> O_NONBLOCK define and thus will get more compatible to other Linux
>> architectures. This comes with the downside of (at least for a few
>> years) added overhead for those non-performance critical syscalls.
>
> The performance issue is resolved once the installed kernel
> headers/libc have been updated accordingly. I think after that the
> overhead should be minimal.

Yes, probably.

>> The benefit with a) is that we then step back again and stay
>> compatible now. The downside is, that in the future we may run into
>> other issues and need to keep special-handling in some other syscalls
>> forever.
>>
>> I'm still for going with b), and I hope we got all corner cases ruled

I meant "proposing to go with option b)"....

>> out. Opinions?
>
> I think the performance penalty isn't that great, but could be
> improved, so I'd go for b) with a small change. The warning that it
> issues seems redundant, because the immediate problem has already been
> "solved", and because the proposed solution does not work:
>
> +{
> +	if (flags & O_NONBLOCK_MASK_OUT) {
> +		struct task_struct *tsk = current;
> +		pr_warn("%s(%d) uses old O_NONBLOCK value. "
> +			"Please recompile the application.\n",
> +			tsk->comm, tsk->pid);
> +	}
> +	return flags & ~O_NONBLOCK_MASK_OUT;
> +}
> +
>
> The text assumes that the officially packaged kernel headers are in
> sync with the kernel, which normally isn't the case as most
> distributions seem to either pick an LTS kernel for their toolchain, or
> keep the installed kernel in sync with the installed kernel headers,
> but do not prevent running newer kernels and may even encourage doing
> so. Simply recompiling the programs that use the old O_NONBLOCK value
> does not solve the problem in most cases.

True.

> If you'd remove that if() { pr_warn } entirely, you'd probably be
> rid of most of the performance penalty anyway.

I think it makes sense to keep at least one warning.
Rolf Eike proposed a pr_warn_once() which seems the best compromise
as it keeps us at least informed which packages needs updating and
how relevant it is to keep those wrappers....

Helge

Re: [RFC PATCH] parisc: Add wrapper syscalls to fix O_NONBLOCK flag usage

[Jeroen Roovers]

On Fri, 23 Oct 2020 10:35:34 +0200
Helge Deller <deller@gmx.de> wrote:

> I think it makes sense to keep at least one warning.
> Rolf Eike proposed a pr_warn_once() which seems the best compromise
> as it keeps us at least informed which packages needs updating and
> how relevant it is to keep those wrappers....

I agree that if we are to keep the warning, pr_warn_once is a better
choice.

In the rpc.idmapd strace I still see

4635  inotify_init1(IN_NONBLOCK)        = -1 EINVAL (Invalid argument)

so maybe you want to wrap around inotify_init1 as well?


Regards,
      jer

Re: [RFC PATCH] parisc: Add wrapper syscalls to fix O_NONBLOCK flag usage

[Helge Deller]

On 10/23/20 10:53 AM, Jeroen Roovers wrote:
> On Fri, 23 Oct 2020 10:35:34 +0200
> Helge Deller <deller@gmx.de> wrote:
>
>> I think it makes sense to keep at least one warning.
>> Rolf Eike proposed a pr_warn_once() which seems the best compromise
>> as it keeps us at least informed which packages needs updating and
>> how relevant it is to keep those wrappers....
>
> I agree that if we are to keep the warning, pr_warn_once is a better
> choice.
>
> In the rpc.idmapd strace I still see
>
> 4635  inotify_init1(IN_NONBLOCK)        = -1 EINVAL (Invalid argument)
>
> so maybe you want to wrap around inotify_init1 as well?

Done.
Added in the new patch version.

Helge

Re: [RFC PATCH] parisc: Add wrapper syscalls to fix O_NONBLOCK flag usage

[Jeroen Roovers]

On Thu, 22 Oct 2020 18:40:07 +0200
Helge Deller <deller@gmx.de> wrote:

> 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>

That fixes all the previous problems.


Regards,
     jer

Re: [RFC PATCH] parisc: Add wrapper syscalls to fix O_NONBLOCK flag usage

[Rolf Eike Beer]

[-- Attachment #1: Type: text/plain, Size: 827 bytes --]

> +#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("%s(%d) uses old O_NONBLOCK value. "
> +			"Please recompile the application.\n",
> +			tsk->comm, tsk->pid);
> +	}
> +	return flags & ~O_NONBLOCK_MASK_OUT;
> +}

This will also trigger if I just pass 0x4 in flags, no? The check should be 

	if ((flags & O_NONBLOCK_OLD) == O_NONBLOCK_OLD)

because that would correctly reject a bare 0x4, at least I hope that this 
would already happen with the strict checking you mentioned.

Would a pr_warn_once make sense? Otherwise your log may get flooded by them if 
e.g. sudo is the problem and my nagios comes every minute to check something.

Eike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: [RFC PATCH] parisc: Add wrapper syscalls to fix O_NONBLOCK flag usage

[Helge Deller]

[-- Attachment #1.1: Type: text/plain, Size: 1172 bytes --]

On 10/23/20 9:25 AM, Rolf Eike Beer wrote:
>> +#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("%s(%d) uses old O_NONBLOCK value. "
>> +			"Please recompile the application.\n",
>> +			tsk->comm, tsk->pid);
>> +	}
>> +	return flags & ~O_NONBLOCK_MASK_OUT;
>> +}
> 
> This will also trigger if I just pass 0x4 in flags, no? The check should be 
> 
> 	if ((flags & O_NONBLOCK_OLD) == O_NONBLOCK_OLD)

RIGHT!
That's a very good point.
I was thinking about what would happen if over time a new (unrelated) define
gets created which then gets 0x4 as value. My code would then have wrongly 
modified it.
I'll fix this.
 
> because that would correctly reject a bare 0x4, at least I hope that this 
> would already happen with the strict checking you mentioned.
> 
> Would a pr_warn_once make sense? Otherwise your log may get flooded by them if 
> e.g. sudo is the problem and my nagios comes every minute to check something.

I like the idea.

Helge


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [RFC PATCH] parisc: Add wrapper syscalls to fix O_NONBLOCK flag usage

[Helge Deller]

On 10/23/20 10:18 AM, Helge Deller wrote:
> On 10/23/20 9:25 AM, Rolf Eike Beer wrote:
>>> +#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("%s(%d) uses old O_NONBLOCK value. "
>>> +			"Please recompile the application.\n",
>>> +			tsk->comm, tsk->pid);
>>> +	}
>>> +	return flags & ~O_NONBLOCK_MASK_OUT;
>>> +}
>>
>> This will also trigger if I just pass 0x4 in flags, no? The check should be
>>
>> 	if ((flags & O_NONBLOCK_OLD) == O_NONBLOCK_OLD)
>
> RIGHT!
> That's a very good point.
> I was thinking about what would happen if over time a new (unrelated) define
> gets created which then gets 0x4 as value. My code would then have wrongly
> modified it.

After some more thinking....

It's not that easy.
Let's assume there will be another new flag at some time with value 0x4.
Now, the caller sets this flag (0x4) and new O_NONBLOCK (000200000),
so you end up with 000200004 again, which then triggers your check:
 	if ((flags & O_NONBLOCK_OLD) == O_NONBLOCK_OLD)

So, my check to test only the mask for 0x4 was basically better,
because it would prevent the usage of 0x4 as any new value.
In any way, it seems we need to avoid using 0x4 for a long time...

Helge

Re: [RFC PATCH] parisc: Add wrapper syscalls to fix O_NONBLOCK flag usage

[Rolf Eike Beer]

[-- Attachment #1: Type: text/plain, Size: 1801 bytes --]

Am Freitag, 23. Oktober 2020, 10:31:14 CEST schrieb Helge Deller:
> On 10/23/20 10:18 AM, Helge Deller wrote:
> > On 10/23/20 9:25 AM, Rolf Eike Beer wrote:
> >>> +#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("%s(%d) uses old O_NONBLOCK value. "
> >>> +			"Please recompile the application.\n",
> >>> +			tsk->comm, tsk->pid);
> >>> +	}
> >>> +	return flags & ~O_NONBLOCK_MASK_OUT;
> >>> +}
> >> 
> >> This will also trigger if I just pass 0x4 in flags, no? The check should
> >> be
> >> 
> >> 	if ((flags & O_NONBLOCK_OLD) == O_NONBLOCK_OLD)
> > 
> > RIGHT!
> > That's a very good point.
> > I was thinking about what would happen if over time a new (unrelated)
> > define gets created which then gets 0x4 as value. My code would then have
> > wrongly modified it.
> 
> After some more thinking....
> 
> It's not that easy.
> Let's assume there will be another new flag at some time with value 0x4.
> Now, the caller sets this flag (0x4) and new O_NONBLOCK (000200000),
> so you end up with 000200004 again, which then triggers your check:
>  	if ((flags & O_NONBLOCK_OLD) == O_NONBLOCK_OLD)
> 
> So, my check to test only the mask for 0x4 was basically better,
> because it would prevent the usage of 0x4 as any new value.
> In any way, it seems we need to avoid using 0x4 for a long time...

Then maybe just add that as an explicit comment in the header so noone picks 
it by accident, and add a comment pointing to these wrappers so they are more 
likely to be kept in sync.

And for extra fun we can just scrap all this for 64 bit userspace as we don't 
need the compat there ;)

Eike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[RFC PATCH v3] parisc: Add wrapper syscalls to fix O_NONBLOCK flag usage

[Helge Deller]

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()

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

Re: [RFC PATCH v3] parisc: Add wrapper syscalls to fix O_NONBLOCK flag usage

[Jeroen Roovers]

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

Re: [RFC PATCH v3] parisc: Add wrapper syscalls to fix O_NONBLOCK flag usage

[Jeroen Roovers]

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

Re: [RFC PATCH v3] parisc: Add wrapper syscalls to fix O_NONBLOCK flag usage

[Helge Deller]

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

Re: [RFC PATCH v3] parisc: Add wrapper syscalls to fix O_NONBLOCK flag usage

[John David Anglin]

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

-- 
John David Anglin  dave.anglin@bell.net

Re: [RFC PATCH v3] parisc: Add wrapper syscalls to fix O_NONBLOCK flag usage

[Jeroen Roovers]

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

Re: [RFC PATCH v3] parisc: Add wrapper syscalls to fix O_NONBLOCK flag usage

[Helge Deller]

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