All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/4] parisc: Drop strnlen_user() in favour of generic version
@ 2021-09-08 20:44 Helge Deller
  2021-09-08 20:44 ` [PATCH 2/4] parisc: Drop useless debug info and comments from signal.c Helge Deller
                   ` (3 more replies)
  0 siblings, 4 replies; 15+ messages in thread
From: Helge Deller @ 2021-09-08 20:44 UTC (permalink / raw)
  To: linux-parisc; +Cc: James Bottomley, John David Anglin, Arnd Bergmann

As suggested by Arnd Bergmann, drop the parisc version of
strnlen_user() and switch to the generic version.

user_addr_max() was wrong too, fix it by using TASK_SIZE.

Suggested-by: Arnd Bergmann <arnd@kernel.org>
Signed-off-by: Helge Deller <deller@gmx.de>
---
 arch/parisc/Kconfig               |  1 -
 arch/parisc/include/asm/uaccess.h |  5 ++---
 arch/parisc/kernel/parisc_ksyms.c |  1 -
 arch/parisc/lib/lusercopy.S       | 34 -------------------------------
 4 files changed, 2 insertions(+), 39 deletions(-)

diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
index 95d4bbf4e455..3ae71994399c 100644
--- a/arch/parisc/Kconfig
+++ b/arch/parisc/Kconfig
@@ -10,7 +10,6 @@ config PARISC
 	select ARCH_HAS_ELF_RANDOMIZE
 	select ARCH_HAS_STRICT_KERNEL_RWX
 	select ARCH_HAS_UBSAN_SANITIZE_ALL
-	select ARCH_HAS_STRNLEN_USER
 	select ARCH_NO_SG_CHAIN
 	select ARCH_SUPPORTS_HUGETLBFS if PA20
 	select ARCH_SUPPORTS_MEMORY_FAILURE
diff --git a/arch/parisc/include/asm/uaccess.h b/arch/parisc/include/asm/uaccess.h
index ed2cd4fb479b..2442ed2929ae 100644
--- a/arch/parisc/include/asm/uaccess.h
+++ b/arch/parisc/include/asm/uaccess.h
@@ -201,13 +201,12 @@ struct exception_table_entry {

 extern long strncpy_from_user(char *, const char __user *, long);
 extern unsigned lclear_user(void __user *, unsigned long);
-extern long lstrnlen_user(const char __user *, long);
+extern __must_check long strnlen_user(const char __user *src, long n);
 /*
  * Complex access routines -- macros
  */
-#define user_addr_max() (~0UL)
+#define user_addr_max() (uaccess_kernel() ? ~0UL : TASK_SIZE)

-#define strnlen_user lstrnlen_user
 #define clear_user lclear_user
 #define __clear_user lclear_user

diff --git a/arch/parisc/kernel/parisc_ksyms.c b/arch/parisc/kernel/parisc_ksyms.c
index e8a6a751dfd8..00297e8e1c88 100644
--- a/arch/parisc/kernel/parisc_ksyms.c
+++ b/arch/parisc/kernel/parisc_ksyms.c
@@ -32,7 +32,6 @@ EXPORT_SYMBOL(__xchg64);

 #include <linux/uaccess.h>
 EXPORT_SYMBOL(lclear_user);
-EXPORT_SYMBOL(lstrnlen_user);

 #ifndef CONFIG_64BIT
 /* Needed so insmod can set dp value */
diff --git a/arch/parisc/lib/lusercopy.S b/arch/parisc/lib/lusercopy.S
index 36d6a8638ead..0aad5ce89f4d 100644
--- a/arch/parisc/lib/lusercopy.S
+++ b/arch/parisc/lib/lusercopy.S
@@ -67,40 +67,6 @@ $lclu_done:
 ENDPROC_CFI(lclear_user)


-	/*
-	 * long lstrnlen_user(char *s, long n)
-	 *
-	 * Returns 0 if exception before zero byte or reaching N,
-	 *         N+1 if N would be exceeded,
-	 *         else strlen + 1 (i.e. includes zero byte).
-	 */
-
-ENTRY_CFI(lstrnlen_user)
-	comib,=     0,%r25,$lslen_nzero
-	copy	    %r26,%r24
-	get_sr
-1:      ldbs,ma     1(%sr1,%r26),%r1
-$lslen_loop:
-	comib,=,n   0,%r1,$lslen_done
-	addib,<>    -1,%r25,$lslen_loop
-2:      ldbs,ma     1(%sr1,%r26),%r1
-$lslen_done:
-	bv          %r0(%r2)
-	sub	    %r26,%r24,%r28
-
-$lslen_nzero:
-	b           $lslen_done
-	ldo         1(%r26),%r26 /* special case for N == 0 */
-
-3:      b	    $lslen_done
-	copy        %r24,%r26    /* reset r26 so 0 is returned on fault */
-
-	ASM_EXCEPTIONTABLE_ENTRY(1b,3b)
-	ASM_EXCEPTIONTABLE_ENTRY(2b,3b)
-
-ENDPROC_CFI(lstrnlen_user)
-
-
 /*
  * unsigned long pa_memcpy(void *dstp, const void *srcp, unsigned long len)
  *
--
2.31.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 2/4] parisc: Drop useless debug info and comments from signal.c
  2021-09-08 20:44 [PATCH 1/4] parisc: Drop strnlen_user() in favour of generic version Helge Deller
@ 2021-09-08 20:44 ` Helge Deller
  2021-09-08 20:44 ` [PATCH 3/4] parisc: Check user signal stack trampoline is inside TASK_SIZE Helge Deller
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 15+ messages in thread
From: Helge Deller @ 2021-09-08 20:44 UTC (permalink / raw)
  To: linux-parisc; +Cc: James Bottomley, John David Anglin

Signed-off-by: Helge Deller <deller@gmx.de>
---
 arch/parisc/kernel/signal.c | 15 ---------------
 1 file changed, 15 deletions(-)

diff --git a/arch/parisc/kernel/signal.c b/arch/parisc/kernel/signal.c
index db1a47cf424d..77db707ce391 100644
--- a/arch/parisc/kernel/signal.c
+++ b/arch/parisc/kernel/signal.c
@@ -293,16 +293,6 @@ setup_rt_frame(struct ksignal *ksig, sigset_t *set, struct pt_regs *regs,
 			&frame->tramp[SIGRESTARTBLOCK_TRAMP+2]);
 	err |= __put_user(INSN_NOP, &frame->tramp[SIGRESTARTBLOCK_TRAMP+3]);

-#if DEBUG_SIG
-	/* Assert that we're flushing in the correct space... */
-	{
-		unsigned long sid;
-		asm ("mfsp %%sr3,%0" : "=r" (sid));
-		DBG(1,"setup_rt_frame: Flushing 64 bytes at space %#x offset %p\n",
-		       sid, frame->tramp);
-	}
-#endif
-
 	start = (unsigned long) &frame->tramp[0];
 	end = (unsigned long) &frame->tramp[TRAMP_SIZE];
 	flush_user_dcache_range_asm(start, end);
@@ -501,7 +491,6 @@ syscall_restart(struct pt_regs *regs, struct k_sigaction *ka)
 		DBG(1,"ERESTARTNOHAND: returning -EINTR\n");
 		regs->gr[28] = -EINTR;
 		break;
-
 	case -ERESTARTSYS:
 		if (!(ka->sa.sa_flags & SA_RESTART)) {
 			DBG(1,"ERESTARTSYS: putting -EINTR\n");
@@ -569,10 +558,6 @@ insert_restart_trampoline(struct pt_regs *regs)
 }

 /*
- * Note that 'init' is a special process: it doesn't get signals it doesn't
- * want to handle. Thus you cannot kill init even with a SIGKILL even by
- * mistake.
- *
  * We need to be able to restore the syscall arguments (r21-r26) to
  * restart syscalls.  Thus, the syscall path should save them in the
  * pt_regs structure (it's okay to do so since they are caller-save
--
2.31.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 3/4] parisc: Check user signal stack trampoline is inside TASK_SIZE
  2021-09-08 20:44 [PATCH 1/4] parisc: Drop strnlen_user() in favour of generic version Helge Deller
  2021-09-08 20:44 ` [PATCH 2/4] parisc: Drop useless debug info and comments from signal.c Helge Deller
@ 2021-09-08 20:44 ` Helge Deller
  2021-09-08 20:44 ` [PATCH 4/4] parisc: Reduce sigreturn trampoline to 3 instructions Helge Deller
  2021-09-08 21:26 ` [PATCH 1/4] parisc: Drop strnlen_user() in favour of generic version Arnd Bergmann
  3 siblings, 0 replies; 15+ messages in thread
From: Helge Deller @ 2021-09-08 20:44 UTC (permalink / raw)
  To: linux-parisc; +Cc: James Bottomley, John David Anglin

Add some additional checks to ensure the signal stack is inside
userspace bounds.

Signed-off-by: Helge Deller <deller@gmx.de>
---
 arch/parisc/kernel/signal.c | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/arch/parisc/kernel/signal.c b/arch/parisc/kernel/signal.c
index 77db707ce391..46b1050640b8 100644
--- a/arch/parisc/kernel/signal.c
+++ b/arch/parisc/kernel/signal.c
@@ -237,18 +237,22 @@ setup_rt_frame(struct ksignal *ksig, sigset_t *set, struct pt_regs *regs,
 #endif

 	usp = (regs->gr[30] & ~(0x01UL));
+	sigframe_size = PARISC_RT_SIGFRAME_SIZE;
 #ifdef CONFIG_64BIT
 	if (is_compat_task()) {
 		/* The gcc alloca implementation leaves garbage in the upper 32 bits of sp */
 		usp = (compat_uint_t)usp;
+		sigframe_size = PARISC_RT_SIGFRAME_SIZE32;
 	}
 #endif
-	/*FIXME: frame_size parameter is unused, remove it. */
-	frame = get_sigframe(&ksig->ka, usp, sizeof(*frame));
+	frame = get_sigframe(&ksig->ka, usp, sigframe_size);

 	DBG(1,"SETUP_RT_FRAME: START\n");
 	DBG(1,"setup_rt_frame: frame %p info %p\n", frame, ksig->info);

+	start = (unsigned long) frame;
+	if (start >= user_addr_max() - sigframe_size)
+		return -EFAULT;

 #ifdef CONFIG_64BIT

@@ -343,11 +347,6 @@ setup_rt_frame(struct ksignal *ksig, sigset_t *set, struct pt_regs *regs,

 	/* The syscall return path will create IAOQ values from r31.
 	 */
-	sigframe_size = PARISC_RT_SIGFRAME_SIZE;
-#ifdef CONFIG_64BIT
-	if (is_compat_task())
-		sigframe_size = PARISC_RT_SIGFRAME_SIZE32;
-#endif
 	if (in_syscall) {
 		regs->gr[31] = haddr;
 #ifdef CONFIG_64BIT
@@ -518,6 +517,10 @@ insert_restart_trampoline(struct pt_regs *regs)
 		unsigned long end  = (unsigned long) &usp[5];
 		long err = 0;

+		/* check that we don't exceed the stack */
+		if (A(&usp[0]) >= user_addr_max() - 5 * sizeof(int))
+			return;
+
 		/* Setup a trampoline to restart the syscall
 		 * with __NR_restart_syscall
 		 *
--
2.31.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* [PATCH 4/4] parisc: Reduce sigreturn trampoline to 3 instructions
  2021-09-08 20:44 [PATCH 1/4] parisc: Drop strnlen_user() in favour of generic version Helge Deller
  2021-09-08 20:44 ` [PATCH 2/4] parisc: Drop useless debug info and comments from signal.c Helge Deller
  2021-09-08 20:44 ` [PATCH 3/4] parisc: Check user signal stack trampoline is inside TASK_SIZE Helge Deller
@ 2021-09-08 20:44 ` Helge Deller
  2021-11-15 22:25   ` John David Anglin
  2021-09-08 21:26 ` [PATCH 1/4] parisc: Drop strnlen_user() in favour of generic version Arnd Bergmann
  3 siblings, 1 reply; 15+ messages in thread
From: Helge Deller @ 2021-09-08 20:44 UTC (permalink / raw)
  To: linux-parisc; +Cc: James Bottomley, John David Anglin

We can move the INSN_LDI_R20 instruction into the branch delay slot.

Signed-off-by: Helge Deller <deller@gmx.de>
---
 arch/parisc/include/asm/rt_sigframe.h |  2 +-
 arch/parisc/kernel/signal.c           | 13 ++++++-------
 arch/parisc/kernel/signal32.h         |  2 +-
 3 files changed, 8 insertions(+), 9 deletions(-)

diff --git a/arch/parisc/include/asm/rt_sigframe.h b/arch/parisc/include/asm/rt_sigframe.h
index 2b3010ade00e..4b9e3d707571 100644
--- a/arch/parisc/include/asm/rt_sigframe.h
+++ b/arch/parisc/include/asm/rt_sigframe.h
@@ -2,7 +2,7 @@
 #ifndef _ASM_PARISC_RT_SIGFRAME_H
 #define _ASM_PARISC_RT_SIGFRAME_H

-#define SIGRETURN_TRAMP 4
+#define SIGRETURN_TRAMP 3
 #define SIGRESTARTBLOCK_TRAMP 5
 #define TRAMP_SIZE (SIGRETURN_TRAMP + SIGRESTARTBLOCK_TRAMP)

diff --git a/arch/parisc/kernel/signal.c b/arch/parisc/kernel/signal.c
index 46b1050640b8..bbfe23c40c01 100644
--- a/arch/parisc/kernel/signal.c
+++ b/arch/parisc/kernel/signal.c
@@ -288,22 +288,21 @@ setup_rt_frame(struct ksignal *ksig, sigset_t *set, struct pt_regs *regs,
 	   already in userspace. The first words of tramp are used to
 	   save the previous sigrestartblock trampoline that might be
 	   on the stack. We start the sigreturn trampoline at
-	   SIGRESTARTBLOCK_TRAMP+X. */
+	   SIGRESTARTBLOCK_TRAMP. */
 	err |= __put_user(in_syscall ? INSN_LDI_R25_1 : INSN_LDI_R25_0,
 			&frame->tramp[SIGRESTARTBLOCK_TRAMP+0]);
-	err |= __put_user(INSN_LDI_R20,
-			&frame->tramp[SIGRESTARTBLOCK_TRAMP+1]);
 	err |= __put_user(INSN_BLE_SR2_R0,
+			&frame->tramp[SIGRESTARTBLOCK_TRAMP+1]);
+	err |= __put_user(INSN_LDI_R20,
 			&frame->tramp[SIGRESTARTBLOCK_TRAMP+2]);
-	err |= __put_user(INSN_NOP, &frame->tramp[SIGRESTARTBLOCK_TRAMP+3]);

-	start = (unsigned long) &frame->tramp[0];
-	end = (unsigned long) &frame->tramp[TRAMP_SIZE];
+	start = (unsigned long) &frame->tramp[SIGRESTARTBLOCK_TRAMP+0];
+	end = (unsigned long) &frame->tramp[SIGRESTARTBLOCK_TRAMP+3];
 	flush_user_dcache_range_asm(start, end);
 	flush_user_icache_range_asm(start, end);

 	/* TRAMP Words 0-4, Length 5 = SIGRESTARTBLOCK_TRAMP
-	 * TRAMP Words 5-9, Length 4 = SIGRETURN_TRAMP
+	 * TRAMP Words 5-7, Length 3 = SIGRETURN_TRAMP
 	 * So the SIGRETURN_TRAMP is at the end of SIGRESTARTBLOCK_TRAMP
 	 */
 	rp = (unsigned long) &frame->tramp[SIGRESTARTBLOCK_TRAMP];
diff --git a/arch/parisc/kernel/signal32.h b/arch/parisc/kernel/signal32.h
index f166250f2d06..a5bdbb5678b7 100644
--- a/arch/parisc/kernel/signal32.h
+++ b/arch/parisc/kernel/signal32.h
@@ -36,7 +36,7 @@ struct compat_regfile {
         compat_int_t rf_sar;
 };

-#define COMPAT_SIGRETURN_TRAMP 4
+#define COMPAT_SIGRETURN_TRAMP 3
 #define COMPAT_SIGRESTARTBLOCK_TRAMP 5
 #define COMPAT_TRAMP_SIZE (COMPAT_SIGRETURN_TRAMP + \
 				COMPAT_SIGRESTARTBLOCK_TRAMP)
--
2.31.1


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/4] parisc: Drop strnlen_user() in favour of generic version
  2021-09-08 20:44 [PATCH 1/4] parisc: Drop strnlen_user() in favour of generic version Helge Deller
                   ` (2 preceding siblings ...)
  2021-09-08 20:44 ` [PATCH 4/4] parisc: Reduce sigreturn trampoline to 3 instructions Helge Deller
@ 2021-09-08 21:26 ` Arnd Bergmann
  2021-09-08 21:40   ` Helge Deller
  3 siblings, 1 reply; 15+ messages in thread
From: Arnd Bergmann @ 2021-09-08 21:26 UTC (permalink / raw)
  To: Helge Deller
  Cc: Parisc List, James Bottomley, John David Anglin, Christoph Hellwig

On Wed, Sep 8, 2021 at 10:44 PM Helge Deller <deller@gmx.de> wrote:
>
> As suggested by Arnd Bergmann, drop the parisc version of
> strnlen_user() and switch to the generic version.

The strnlen_user() removal looks good,

Acked-by: Arnd Bergmann <arnd@arndb.de>

> user_addr_max() was wrong too, fix it by using TASK_SIZE.

Not sure about this part though:

>  /*
>   * Complex access routines -- macros
>   */
> -#define user_addr_max() (~0UL)
> +#define user_addr_max() (uaccess_kernel() ? ~0UL : TASK_SIZE)

We are getting very close to completely removing set_fs()/get_fs(),
uaccess_kernel() and some related bits from the kernel, so I think
it would be better to the other way here and finish off removing
CONFIG_SET_FS from parisc.

I think this will also simplify your asm/uaccess.h a lot, in particular
since it has separate address spaces for __get_user() and
__get_kernel_nofault(), and without set_fs() you can leave out
the runtime conditional to switch between them.

       Arnd

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/4] parisc: Drop strnlen_user() in favour of generic version
  2021-09-08 21:26 ` [PATCH 1/4] parisc: Drop strnlen_user() in favour of generic version Arnd Bergmann
@ 2021-09-08 21:40   ` Helge Deller
  2021-09-08 22:08     ` Arnd Bergmann
  0 siblings, 1 reply; 15+ messages in thread
From: Helge Deller @ 2021-09-08 21:40 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Parisc List, James Bottomley, John David Anglin, Christoph Hellwig

On 9/8/21 11:26 PM, Arnd Bergmann wrote:
> On Wed, Sep 8, 2021 at 10:44 PM Helge Deller <deller@gmx.de> wrote:
>>
>> As suggested by Arnd Bergmann, drop the parisc version of
>> strnlen_user() and switch to the generic version.
>
> The strnlen_user() removal looks good,
>
> Acked-by: Arnd Bergmann <arnd@arndb.de>

Good.

>> user_addr_max() was wrong too, fix it by using TASK_SIZE.
>
> Not sure about this part though:
>
>>   /*
>>    * Complex access routines -- macros
>>    */
>> -#define user_addr_max() (~0UL)
>> +#define user_addr_max() (uaccess_kernel() ? ~0UL : TASK_SIZE)

I noticed that our user_addr_max() was actually wrong.
It's used in the generic strnlen_user() so fixing it seemed appropriate.

> We are getting very close to completely removing set_fs()/get_fs(),
> uaccess_kernel() and some related bits from the kernel, so I think
> it would be better to the other way here and finish off removing
> CONFIG_SET_FS from parisc.
>
> I think this will also simplify your asm/uaccess.h a lot, in particular
> since it has separate address spaces for __get_user() and
> __get_kernel_nofault(), and without set_fs() you can leave out
> the runtime conditional to switch between them.

That's a good idea and should probably be done.
Do you have some pointers where to start, e.g. initial commits from other arches ?

Helge

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/4] parisc: Drop strnlen_user() in favour of generic version
  2021-09-08 21:40   ` Helge Deller
@ 2021-09-08 22:08     ` Arnd Bergmann
  2021-09-09  2:03       ` Helge Deller
  0 siblings, 1 reply; 15+ messages in thread
From: Arnd Bergmann @ 2021-09-08 22:08 UTC (permalink / raw)
  To: Helge Deller
  Cc: Parisc List, James Bottomley, John David Anglin, Christoph Hellwig

On Wed, Sep 8, 2021 at 11:40 PM Helge Deller <deller@gmx.de> wrote:
> On 9/8/21 11:26 PM, Arnd Bergmann wrote:
> >>   /*
> >>    * Complex access routines -- macros
> >>    */
> >> -#define user_addr_max() (~0UL)
> >> +#define user_addr_max() (uaccess_kernel() ? ~0UL : TASK_SIZE)
>
> I noticed that our user_addr_max() was actually wrong.
> It's used in the generic strnlen_user() so fixing it seemed appropriate.

The user_addr_max() definition should match what you do in access_ok()
for USER_DS though, which is a nop on architectures that have split user
address space, so I think the ~0UL was correct here.

No matter what the arguments are, there is no danger of spilling over
into kernel space, which is what the user_addr_max() check is trying
to prevent on other architectures.

> > We are getting very close to completely removing set_fs()/get_fs(),
> > uaccess_kernel() and some related bits from the kernel, so I think
> > it would be better to the other way here and finish off removing
> > CONFIG_SET_FS from parisc.
> >
> > I think this will also simplify your asm/uaccess.h a lot, in particular
> > since it has separate address spaces for __get_user() and
> > __get_kernel_nofault(), and without set_fs() you can leave out
> > the runtime conditional to switch between them.
>
> That's a good idea and should probably be done.
> Do you have some pointers where to start, e.g. initial commits from other arches ?

Russell just merged my series for arch/arm in linux-5.15, you
can look at that but it's probably easier for parisc.

I think the only part you need to add is __get_kernel_nofault()
and __put_kernel_nofault(). You can see in mm/maccess.c
what the difference between the two versions in there is.

Once you have those, you define HAVE_GET_KERNEL_NOFAULT
and then remove CONFIG_SET_FS, set_fs(), get_fs(), load_sr2(),
thread_info->addr_limit, KERNEL_DS, and USER_DS.

        Arnd

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/4] parisc: Drop strnlen_user() in favour of generic version
  2021-09-08 22:08     ` Arnd Bergmann
@ 2021-09-09  2:03       ` Helge Deller
  2021-09-09  6:51         ` Arnd Bergmann
  0 siblings, 1 reply; 15+ messages in thread
From: Helge Deller @ 2021-09-09  2:03 UTC (permalink / raw)
  To: Arnd Bergmann
  Cc: Helge Deller, Parisc List, James Bottomley, John David Anglin,
	Christoph Hellwig

* Arnd Bergmann <arnd@kernel.org>:
> On Wed, Sep 8, 2021 at 11:40 PM Helge Deller <deller@gmx.de> wrote:
> > On 9/8/21 11:26 PM, Arnd Bergmann wrote:
> > >>   /*
> > >>    * Complex access routines -- macros
> > >>    */
> > >> -#define user_addr_max() (~0UL)
> > >> +#define user_addr_max() (uaccess_kernel() ? ~0UL : TASK_SIZE)
> >
> > I noticed that our user_addr_max() was actually wrong.
> > It's used in the generic strnlen_user() so fixing it seemed appropriate.
>
> The user_addr_max() definition should match what you do in access_ok()
> for USER_DS though, which is a nop on architectures that have split user
> address space, so I think the ~0UL was correct here.
>
> No matter what the arguments are, there is no danger of spilling over
> into kernel space, which is what the user_addr_max() check is trying
> to prevent on other architectures.
>
> > > We are getting very close to completely removing set_fs()/get_fs(),
> > > uaccess_kernel() and some related bits from the kernel, so I think
> > > it would be better to the other way here and finish off removing
> > > CONFIG_SET_FS from parisc.
> > >
> > > I think this will also simplify your asm/uaccess.h a lot, in particular
> > > since it has separate address spaces for __get_user() and
> > > __get_kernel_nofault(), and without set_fs() you can leave out
> > > the runtime conditional to switch between them.
> >
> > That's a good idea and should probably be done.
> > Do you have some pointers where to start, e.g. initial commits from other arches ?
>
> Russell just merged my series for arch/arm in linux-5.15, you
> can look at that but it's probably easier for parisc.
>
> I think the only part you need to add is __get_kernel_nofault()
> and __put_kernel_nofault(). You can see in mm/maccess.c
> what the difference between the two versions in there is.
>
> Once you have those, you define HAVE_GET_KERNEL_NOFAULT
> and then remove CONFIG_SET_FS, set_fs(), get_fs(), load_sr2(),
> thread_info->addr_limit, KERNEL_DS, and USER_DS.

Thanks for those hints!
The attached initial patch below seems to work, it boots into systemd.
It needs further cleanups though.

I wonder if you should add a workaround for 32-bit kernels
to not copy 64bit-wise, see my patch below in
copy_from_kernel_nofault():

-       copy_from_kernel_nofault_loop(dst, src, size, u64, Efault);
+       /* avoid 64-bit accesses on 32-bit platforms */
+       if (sizeof(unsigned long) > 4)
+               copy_from_kernel_nofault_loop(dst, src, size, u64, Efault);

Helge

---

[PATCH] parisc: Implement __get/put_kernel_nofault()

Add __get_kernel_nofault() and __put_kernel_nofault(), define
HAVE_GET_KERNEL_NOFAULT and remove CONFIG_SET_FS, set_fs(), get_fs(),
load_sr2(), thread_info->addr_limit, KERNEL_DS, and USER_DS.

Signed-off-by: Helge Deller <deller@gmx.de>
Suggested-by: Arnd Bergmann <arnd@kernel.org>

diff --git a/arch/parisc/Kconfig b/arch/parisc/Kconfig
index 3ae71994399c..b5bc346d464a 100644
--- a/arch/parisc/Kconfig
+++ b/arch/parisc/Kconfig
@@ -64,7 +64,6 @@ config PARISC
 	select HAVE_KPROBES_ON_FTRACE
 	select HAVE_DYNAMIC_FTRACE_WITH_REGS
 	select HAVE_SOFTIRQ_ON_OWN_STACK if IRQSTACKS
-	select SET_FS

 	help
 	  The PA-RISC microprocessor is designed by Hewlett-Packard and used
diff --git a/arch/parisc/include/asm/processor.h b/arch/parisc/include/asm/processor.h
index b5fbcd2c1780..eeb7da064289 100644
--- a/arch/parisc/include/asm/processor.h
+++ b/arch/parisc/include/asm/processor.h
@@ -101,10 +101,6 @@ DECLARE_PER_CPU(struct cpuinfo_parisc, cpu_data);

 #define CPU_HVERSION ((boot_cpu_data.hversion >> 4) & 0x0FFF)

-typedef struct {
-	int seg;
-} mm_segment_t;
-
 #define ARCH_MIN_TASKALIGN	8

 struct thread_struct {
diff --git a/arch/parisc/include/asm/thread_info.h b/arch/parisc/include/asm/thread_info.h
index 0bd38a972cea..00ad50fef769 100644
--- a/arch/parisc/include/asm/thread_info.h
+++ b/arch/parisc/include/asm/thread_info.h
@@ -11,7 +11,6 @@
 struct thread_info {
 	struct task_struct *task;	/* main task structure */
 	unsigned long flags;		/* thread_info flags (see TIF_*) */
-	mm_segment_t addr_limit;	/* user-level address space limit */
 	__u32 cpu;			/* current CPU */
 	int preempt_count;		/* 0=premptable, <0=BUG; will also serve as bh-counter */
 };
@@ -21,7 +20,6 @@ struct thread_info {
 	.task		= &tsk,			\
 	.flags		= 0,			\
 	.cpu		= 0,			\
-	.addr_limit	= KERNEL_DS,		\
 	.preempt_count	= INIT_PREEMPT_COUNT,	\
 }

diff --git a/arch/parisc/include/asm/uaccess.h b/arch/parisc/include/asm/uaccess.h
index a4368731f2b4..6e0ca7756ceb 100644
--- a/arch/parisc/include/asm/uaccess.h
+++ b/arch/parisc/include/asm/uaccess.h
@@ -11,14 +11,6 @@
 #include <linux/bug.h>
 #include <linux/string.h>

-#define KERNEL_DS	((mm_segment_t){0})
-#define USER_DS 	((mm_segment_t){1})
-
-#define uaccess_kernel() (get_fs().seg == KERNEL_DS.seg)
-
-#define get_fs()	(current_thread_info()->addr_limit)
-#define set_fs(x)	(current_thread_info()->addr_limit = (x))
-
 /*
  * Note that since kernel addresses are in a separate address space on
  * parisc, we don't need to do anything for access_ok().
@@ -33,11 +25,11 @@
 #define get_user __get_user

 #if !defined(CONFIG_64BIT)
-#define LDD_USER(val, ptr)	__get_user_asm64(val, ptr)
-#define STD_USER(x, ptr)	__put_user_asm64(x, ptr)
+#define LDD_USER(sr, val, ptr, err_label)	__get_user_asm64(sr, val, ptr, err_label)
+#define STD_USER(sr, x, ptr, err_label)		__put_user_asm64(sr, x, ptr, err_label)
 #else
-#define LDD_USER(val, ptr)	__get_user_asm(val, "ldd", ptr)
-#define STD_USER(x, ptr)	__put_user_asm("std", x, ptr)
+#define LDD_USER(sr, val, ptr, err_label)	__get_user_asm(sr, val, "ldd", ptr, err_label)
+#define STD_USER(sr, x, ptr, err_label)		__put_user_asm(sr, "std", x, ptr, err_label)
 #endif

 /*
@@ -67,28 +59,15 @@ struct exception_table_entry {
 #define ASM_EXCEPTIONTABLE_ENTRY_EFAULT( fault_addr, except_addr )\
 	ASM_EXCEPTIONTABLE_ENTRY( fault_addr, except_addr + 1)

-/*
- * load_sr2() preloads the space register %%sr2 - based on the value of
- * get_fs() - with either a value of 0 to access kernel space (KERNEL_DS which
- * is 0), or with the current value of %%sr3 to access user space (USER_DS)
- * memory. The following __get_user_asm() and __put_user_asm() functions have
- * %%sr2 hard-coded to access the requested memory.
- */
-#define load_sr2() \
-	__asm__(" or,=  %0,%%r0,%%r0\n\t"	\
-		" mfsp %%sr3,%0\n\t"		\
-		" mtsp %0,%%sr2\n\t"		\
-		: : "r"(get_fs()) : )
-
-#define __get_user_internal(val, ptr)			\
+#define __get_user_internal(sr, val, ptr, err_label)	\
 ({							\
 	register long __gu_err __asm__ ("r8") = 0;	\
 							\
 	switch (sizeof(*(ptr))) {			\
-	case 1: __get_user_asm(val, "ldb", ptr); break;	\
-	case 2: __get_user_asm(val, "ldh", ptr); break; \
-	case 4: __get_user_asm(val, "ldw", ptr); break; \
-	case 8: LDD_USER(val, ptr); break;		\
+	case 1: __get_user_asm(sr, val, "ldb", ptr, err_label); break;	\
+	case 2: __get_user_asm(sr, val, "ldh", ptr, err_label); break;	\
+	case 4: __get_user_asm(sr, val, "ldw", ptr, err_label); break;	\
+	case 8: LDD_USER(sr, val, ptr, err_label); break;		\
 	default: BUILD_BUG();				\
 	}						\
 							\
@@ -97,26 +76,38 @@ struct exception_table_entry {

 #define __get_user(val, ptr)				\
 ({							\
-	load_sr2();					\
-	__get_user_internal(val, ptr);			\
+	__get_user_internal("%%sr3,", val, ptr, 9b);	\
 })

-#define __get_user_asm(val, ldx, ptr)			\
+#define __get_user_asm(sr, val, ldx, ptr, err_label)	\
 {							\
 	register long __gu_val;				\
 							\
-	__asm__("1: " ldx " 0(%%sr2,%2),%0\n"		\
+	__asm__("1: " ldx " 0(" sr "%2),%0\n"		\
 		"9:\n"					\
-		ASM_EXCEPTIONTABLE_ENTRY_EFAULT(1b, 9b)	\
+		ASM_EXCEPTIONTABLE_ENTRY_EFAULT(1b, err_label) \
 		: "=r"(__gu_val), "=r"(__gu_err)        \
 		: "r"(ptr), "1"(__gu_err));		\
 							\
 	(val) = (__force __typeof__(*(ptr))) __gu_val;	\
 }

+#define HAVE_GET_KERNEL_NOFAULT
+#define __get_kernel_nofault(dst, src, type, err_label)	\
+{							\
+	type __z;					\
+	long __err;					\
+	__err = __get_user_internal("", __z, (type *)(src), 9b); \
+	if (unlikely(__err))				\
+		goto err_label;				\
+	else						\
+		*(type *)(dst) = __z;			\
+}
+
+
 #if !defined(CONFIG_64BIT)

-#define __get_user_asm64(val, ptr)			\
+#define __get_user_asm64(sr, val, ptr, err_label)		\
 {							\
 	union {						\
 		unsigned long long	l;		\
@@ -124,11 +115,11 @@ struct exception_table_entry {
 	} __gu_tmp;					\
 							\
 	__asm__("   copy %%r0,%R0\n"			\
-		"1: ldw 0(%%sr2,%2),%0\n"		\
-		"2: ldw 4(%%sr2,%2),%R0\n"		\
+		"1: ldw 0(" sr "%2),%0\n"		\
+		"2: ldw 4(" sr "%2),%R0\n"		\
 		"9:\n"					\
-		ASM_EXCEPTIONTABLE_ENTRY_EFAULT(1b, 9b)	\
-		ASM_EXCEPTIONTABLE_ENTRY_EFAULT(2b, 9b)	\
+		ASM_EXCEPTIONTABLE_ENTRY_EFAULT(1b, err_label) \
+		ASM_EXCEPTIONTABLE_ENTRY_EFAULT(2b, err_label) \
 		: "=&r"(__gu_tmp.l), "=r"(__gu_err)	\
 		: "r"(ptr), "1"(__gu_err));		\
 							\
@@ -138,16 +129,16 @@ struct exception_table_entry {
 #endif /* !defined(CONFIG_64BIT) */


-#define __put_user_internal(x, ptr)				\
+#define __put_user_internal(sr, x, ptr, err_label)			\
 ({								\
 	register long __pu_err __asm__ ("r8") = 0;      	\
         __typeof__(*(ptr)) __x = (__typeof__(*(ptr)))(x);	\
 								\
 	switch (sizeof(*(ptr))) {				\
-	case 1: __put_user_asm("stb", __x, ptr); break;		\
-	case 2: __put_user_asm("sth", __x, ptr); break;		\
-	case 4: __put_user_asm("stw", __x, ptr); break;		\
-	case 8: STD_USER(__x, ptr); break;			\
+	case 1: __put_user_asm(sr, "stb", __x, ptr, err_label); break; \
+	case 2: __put_user_asm(sr, "sth", __x, ptr, err_label); break; \
+	case 4: __put_user_asm(sr, "stw", __x, ptr, err_label); break; \
+	case 8: STD_USER(sr, __x, ptr, err_label); break;		\
 	default: BUILD_BUG();					\
 	}							\
 								\
@@ -156,10 +147,20 @@ struct exception_table_entry {

 #define __put_user(x, ptr)					\
 ({								\
-	load_sr2();						\
-	__put_user_internal(x, ptr);				\
+	__put_user_internal("%%sr3,", x, ptr, 9b);		\
 })

+#define __put_kernel_nofault(dst, src, type, err_label)		\
+{								\
+	type __z = *(type *)(src);				\
+	long __err;						\
+	__err = __put_user_internal("", __z, (type *)(dst), 9b);\
+	if (unlikely(__err))					\
+		goto err_label;					\
+}
+
+
+

 /*
  * The "__put_user/kernel_asm()" macros tell gcc they read from memory
@@ -170,26 +171,26 @@ struct exception_table_entry {
  * r8 is already listed as err.
  */

-#define __put_user_asm(stx, x, ptr)                         \
-	__asm__ __volatile__ (                              \
-		"1: " stx " %2,0(%%sr2,%1)\n"		    \
-		"9:\n"					    \
-		ASM_EXCEPTIONTABLE_ENTRY_EFAULT(1b, 9b)	    \
-		: "=r"(__pu_err)                            \
+#define __put_user_asm(sr, stx, x, ptr, err_label)		\
+	__asm__ __volatile__ (					\
+		"1: " stx " %2,0(" sr "%1)\n"			\
+		"9:\n"						\
+		ASM_EXCEPTIONTABLE_ENTRY_EFAULT(1b, err_label)	\
+		: "=r"(__pu_err)				\
 		: "r"(ptr), "r"(x), "0"(__pu_err))


 #if !defined(CONFIG_64BIT)

-#define __put_user_asm64(__val, ptr) do {	    	    \
-	__asm__ __volatile__ (				    \
-		"1: stw %2,0(%%sr2,%1)\n"		    \
-		"2: stw %R2,4(%%sr2,%1)\n"		    \
-		"9:\n"					    \
-		ASM_EXCEPTIONTABLE_ENTRY_EFAULT(1b, 9b)	    \
-		ASM_EXCEPTIONTABLE_ENTRY_EFAULT(2b, 9b)	    \
-		: "=r"(__pu_err)                            \
-		: "r"(ptr), "r"(__val), "0"(__pu_err));	    \
+#define __put_user_asm64(sr, __val, ptr, err_label) do {	\
+	__asm__ __volatile__ (					\
+		"1: stw %2,0(" sr "%1)\n"			\
+		"2: stw %R2,4(" sr "%1)\n"			\
+		"9:\n"						\
+		ASM_EXCEPTIONTABLE_ENTRY_EFAULT(1b, err_label)	\
+		ASM_EXCEPTIONTABLE_ENTRY_EFAULT(2b, err_label)	\
+		: "=r"(__pu_err)				\
+		: "r"(ptr), "r"(__val), "0"(__pu_err));		\
 } while (0)

 #endif /* !defined(CONFIG_64BIT) */
@@ -205,7 +206,6 @@ extern __must_check long strnlen_user(const char __user *src, long n);
 /*
  * Complex access routines -- macros
  */
-#define user_addr_max() (~0UL)

 #define clear_user lclear_user
 #define __clear_user lclear_user
diff --git a/arch/parisc/kernel/asm-offsets.c b/arch/parisc/kernel/asm-offsets.c
index 33113ba24054..7c83915e4aed 100644
--- a/arch/parisc/kernel/asm-offsets.c
+++ b/arch/parisc/kernel/asm-offsets.c
@@ -230,7 +230,7 @@ int main(void)
 	DEFINE(TI_TASK, offsetof(struct thread_info, task));
 	DEFINE(TI_FLAGS, offsetof(struct thread_info, flags));
 	DEFINE(TI_CPU, offsetof(struct thread_info, cpu));
-	DEFINE(TI_SEGMENT, offsetof(struct thread_info, addr_limit));
+//	DEFINE(TI_SEGMENT, offsetof(struct thread_info, addr_limit));
 	DEFINE(TI_PRE_COUNT, offsetof(struct thread_info, preempt_count));
 	DEFINE(THREAD_SZ, sizeof(struct thread_info));
 	/* THREAD_SZ_ALGN includes space for a stack frame. */
diff --git a/arch/parisc/lib/lusercopy.S b/arch/parisc/lib/lusercopy.S
index 0aad5ce89f4d..29eafd963ac0 100644
--- a/arch/parisc/lib/lusercopy.S
+++ b/arch/parisc/lib/lusercopy.S
@@ -34,11 +34,11 @@
 	 */

 	.macro  get_sr
-	mfctl       %cr30,%r1
-	ldw         TI_SEGMENT(%r1),%r22
+//	mfctl       %cr30,%r1
+//	ldw         TI_SEGMENT(%r1),%r22
 	mfsp        %sr3,%r1
-	or,<>       %r22,%r0,%r0
-	copy        %r0,%r1
+//	or,<>       %r22,%r0,%r0
+//	copy        %r0,%r1
 	mtsp        %r1,%sr1
 	.endm

diff --git a/mm/maccess.c b/mm/maccess.c
index 3bd70405f2d8..85eb86d6cab4 100644
--- a/mm/maccess.c
+++ b/mm/maccess.c
@@ -28,7 +28,9 @@ long copy_from_kernel_nofault(void *dst, const void *src, size_t size)
 		return -ERANGE;

 	pagefault_disable();
-	copy_from_kernel_nofault_loop(dst, src, size, u64, Efault);
+	/* avoid 64-bit accesses on 32-bit platforms */
+	if (sizeof(unsigned long) > 4)
+		copy_from_kernel_nofault_loop(dst, src, size, u64, Efault);
 	copy_from_kernel_nofault_loop(dst, src, size, u32, Efault);
 	copy_from_kernel_nofault_loop(dst, src, size, u16, Efault);
 	copy_from_kernel_nofault_loop(dst, src, size, u8, Efault);
@@ -51,7 +53,9 @@ EXPORT_SYMBOL_GPL(copy_from_kernel_nofault);
 long copy_to_kernel_nofault(void *dst, const void *src, size_t size)
 {
 	pagefault_disable();
-	copy_to_kernel_nofault_loop(dst, src, size, u64, Efault);
+	/* avoid 64-bit accesses on 32-bit platforms */
+	if (sizeof(unsigned long) > 4)
+		copy_to_kernel_nofault_loop(dst, src, size, u64, Efault);
 	copy_to_kernel_nofault_loop(dst, src, size, u32, Efault);
 	copy_to_kernel_nofault_loop(dst, src, size, u16, Efault);
 	copy_to_kernel_nofault_loop(dst, src, size, u8, Efault);


^ permalink raw reply related	[flat|nested] 15+ messages in thread

* Re: [PATCH 1/4] parisc: Drop strnlen_user() in favour of generic version
  2021-09-09  2:03       ` Helge Deller
@ 2021-09-09  6:51         ` Arnd Bergmann
  0 siblings, 0 replies; 15+ messages in thread
From: Arnd Bergmann @ 2021-09-09  6:51 UTC (permalink / raw)
  To: Helge Deller
  Cc: Parisc List, James Bottomley, John David Anglin, Christoph Hellwig

On Thu, Sep 9, 2021 at 4:03 AM Helge Deller <deller@gmx.de> wrote:
> * Arnd Bergmann <arnd@kernel.org>:
> >
> > I think the only part you need to add is __get_kernel_nofault()
> > and __put_kernel_nofault(). You can see in mm/maccess.c
> > what the difference between the two versions in there is.
> >
> > Once you have those, you define HAVE_GET_KERNEL_NOFAULT
> > and then remove CONFIG_SET_FS, set_fs(), get_fs(), load_sr2(),
> > thread_info->addr_limit, KERNEL_DS, and USER_DS.
>
> Thanks for those hints!
> The attached initial patch below seems to work, it boots into systemd.
> It needs further cleanups though.
>
> I wonder if you should add a workaround for 32-bit kernels
> to not copy 64bit-wise, see my patch below in
> copy_from_kernel_nofault():
>
> -       copy_from_kernel_nofault_loop(dst, src, size, u64, Efault);
> +       /* avoid 64-bit accesses on 32-bit platforms */
> +       if (sizeof(unsigned long) > 4)
> +               copy_from_kernel_nofault_loop(dst, src, size, u64, Efault);

I would assume that the 8-byte copy would just turn into two 4-byte
accesses here, with exactly the same behavior. Note that this function
changed in 5.15 when I added support for architectures without
unaligned access. You need to include 132b548af559 ("mm/maccess:
fix unaligned copy_{from,to}_kernel_nofault") here for parsic as well,
I assume.

If you prefer to avoid the 8-byte access, this could be done in the
'align = (unsigned long)dst | (unsigned long)src;' statement by
masking out that bit.

> [PATCH] parisc: Implement __get/put_kernel_nofault()
>
> Add __get_kernel_nofault() and __put_kernel_nofault(), define
> HAVE_GET_KERNEL_NOFAULT and remove CONFIG_SET_FS, set_fs(), get_fs(),
> load_sr2(), thread_info->addr_limit, KERNEL_DS, and USER_DS.
>
> Signed-off-by: Helge Deller <deller@gmx.de>
> Suggested-by: Arnd Bergmann <arnd@kernel.org>

Looks good to me overall, with the missing cleanups added.

You should probably include a TASK_SIZE_MAX definition that matches the
old user_addr_max() to avoid looking up the per-task value every time.

       Arnd

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/4] parisc: Reduce sigreturn trampoline to 3 instructions
  2021-09-08 20:44 ` [PATCH 4/4] parisc: Reduce sigreturn trampoline to 3 instructions Helge Deller
@ 2021-11-15 22:25   ` John David Anglin
  2021-11-16 13:54     ` Helge Deller
  0 siblings, 1 reply; 15+ messages in thread
From: John David Anglin @ 2021-11-15 22:25 UTC (permalink / raw)
  To: Helge Deller, linux-parisc; +Cc: James Bottomley

This change breaks signal delivery and causes various glibc tests to fail.

commit 3e4a1aff2a97cb4fd7f0268e4b69e8c9d3641277:
dave@atlas:~/gnu/glibc/objdir$ env GCONV_PATH=/home/dave/gnu/glibc/objdir/iconvdata LOCPATH=/home/dave/gnu/glibc/objdir/localedata LC_ALL=C 
/home/dave/gnu/glibc/objdir/elf/ld.so.1 --library-path 
/home/dave/gnu/glibc/objdir:/home/dave/gnu/glibc/objdir/math:/home/dave/gnu/glibc/objdir/elf:/home/dave/gnu/glibc/objdir/dlfcn:/home/dave/gnu/glibc/objdir/nss:/home/dave/gnu/glibc/objdir/nis:/home/dave/gnu/glibc/objdir/rt:/home/dave/gnu/glibc/objdir/resolv:/home/dave/gnu/glibc/objdir/mathvec:/home/dave/gnu/glibc/objdir/support:/home/dave/gnu/glibc/objdir/crypt:/home/dave/gnu/glibc/objdir/nptl 
/home/dave/gnu/glibc/objdir/nptl/tst-cancelx4
got size 4512
in-time cancel test of 'read' successful
in-time cancel test of 'readv' successful
in-time cancel test of 'select' successful
in-time cancel test of 'pselect' successful
in-time cancel test of 'poll' successful
in-time cancel test of 'ppoll' successful
in-time cancel test of 'write' successful
in-time cancel test of 'writev' successful
in-time cancel test of 'sleep' successful
in-time cancel test of 'usleep' successful
in-time cancel test of 'nanosleep' successful
in-time cancel test of 'wait' successful
in-time cancel test of 'waitid' successful
in-time cancel test of 'waitpid' successful
in-time cancel test of 'sigpause' successful
in-time cancel test of 'sigsuspend' successful
in-time cancel test of 'sigwait' successful
in-time cancel test of 'sigwaitinfo' successful
in-time cancel test of 'sigtimedwait' successful
in-time cancel test of 'pause' successful
in-time cancel test of 'accept' successful
got size 4512
in-time cancel test of 'send' successful
in-time cancel test of 'recv' successful
in-time cancel test of 'recvfrom' successful
in-time cancel test of 'recvmsg' successful
in-time cancel test of 'msgrcv' successful
early cancel test of 'read' successful
early cancel test of 'readv' successful
early cancel test of 'select' successful
early cancel test of 'pselect' successful
early cancel test of 'poll' successful
early cancel test of 'ppoll' successful
early cancel test of 'write' successful
early cancel test of 'writev' successful
early cancel test of 'sleep' successful
early cancel test of 'usleep' successful
early cancel test of 'nanosleep' successful
early cancel test of 'wait' successful
early cancel test of 'waitid' successful
early cancel test of 'waitpid' successful
early cancel test of 'sigpause' successful
early cancel test of 'sigsuspend' successful
early cancel test of 'sigwait' successful
early cancel test of 'sigwaitinfo' successful
early cancel test of 'sigtimedwait' successful
early cancel test of 'pause' successful
early cancel test of 'accept' successful
got size 4512
early cancel test of 'send' successful
early cancel test of 'recv' successful
early cancel test of 'recvfrom' successful
early cancel test of 'recvmsg' successful
early cancel test of 'preadv' successful
early cancel test of 'preadv2' successful
early cancel test of 'pwritev' successful
early cancel test of 'pwritev2' successful
early cancel test of 'open' successful
early cancel test of 'close' successful
early cancel test of 'pread' successful
early cancel test of 'pwrite' successful
early cancel test of 'fsync' successful
early cancel test of 'fdatasync' successful
early cancel test of 'msync' successful
got size 4512
early cancel test of 'sendto' successful
early cancel test of 'sendmsg' successful
early cancel test of 'creat' successful
early cancel test of 'connect' successful
early cancel test of 'tcdrain' successful
early cancel test of 'msgrcv' successful
early cancel test of 'msgsnd' successful
dave@atlas:~/gnu/glibc/objdir$ echo $?
0

commit e4f2006f1287e7ea17660490569cff323772dac4:
dave@atlas:~/gnu/glibc/objdir$ env GCONV_PATH=/home/dave/gnu/glibc/objdir/iconvdata LOCPATH=/home/dave/gnu/glibc/objdir/localedata LC_ALL=C 
/home/dave/gnu/glibc/objdir/elf/ld.so.1 --library-path 
/home/dave/gnu/glibc/objdir:/home/dave/gnu/glibc/objdir/math:/home/dave/gnu/glibc/objdir/elf:/home/dave/gnu/glibc/objdir/dlfcn:/home/dave/gnu/glibc/objdir/nss:/home/dave/gnu/glibc/objdir/nis:/home/dave/gnu/glibc/objdir/rt:/home/dave/gnu/glibc/objdir/resolv:/home/dave/gnu/glibc/objdir/mathvec:/home/dave/gnu/glibc/objdir/support:/home/dave/gnu/glibc/objdir/crypt:/home/dave/gnu/glibc/objdir/nptl 
/home/dave/gnu/glibc/objdir/nptl/tst-cancelx4
got size 4512
cleanup handler not called for 'read'
cleanup handler not called for 'readv'
cleanup handler not called for 'select'
cleanup handler not called for 'pselect'
cleanup handler not called for 'poll'
cleanup handler not called for 'ppoll'
cleanup handler not called for 'write'
cleanup handler not called for 'writev'
cleanup handler not called for 'sleep'
cleanup handler not called for 'usleep'
cleanup handler not called for 'nanosleep'
cleanup handler not called for 'wait'
cleanup handler not called for 'waitid'
cleanup handler not called for 'waitpid'
cleanup handler not called for 'sigpause'
cleanup handler not called for 'sigsuspend'
cleanup handler not called for 'sigwait'
cleanup handler not called for 'sigwaitinfo'
cleanup handler not called for 'sigtimedwait'
cleanup handler not called for 'pause'
cleanup handler not called for 'accept'
got size 4512
cleanup handler not called for 'send'
cleanup handler not called for 'recv'
cleanup handler not called for 'recvfrom'
cleanup handler not called for 'recvmsg'
cleanup handler not called for 'msgrcv'
early cancel test of 'read' successful
early cancel test of 'readv' successful
early cancel test of 'select' successful
early cancel test of 'pselect' successful
early cancel test of 'poll' successful
early cancel test of 'ppoll' successful
early cancel test of 'write' successful
early cancel test of 'writev' successful
early cancel test of 'sleep' successful
early cancel test of 'usleep' successful
early cancel test of 'nanosleep' successful
early cancel test of 'wait' successful
early cancel test of 'waitid' successful
early cancel test of 'waitpid' successful
early cancel test of 'sigpause' successful
early cancel test of 'sigsuspend' successful
early cancel test of 'sigwait' successful
early cancel test of 'sigwaitinfo' successful
early cancel test of 'sigtimedwait' successful
early cancel test of 'pause' successful
early cancel test of 'accept' successful
got size 4512
early cancel test of 'send' successful
early cancel test of 'recv' successful
early cancel test of 'recvfrom' successful
early cancel test of 'recvmsg' successful
early cancel test of 'preadv' successful
early cancel test of 'preadv2' successful
early cancel test of 'pwritev' successful
early cancel test of 'pwritev2' successful
early cancel test of 'open' successful
early cancel test of 'close' successful
early cancel test of 'pread' successful
early cancel test of 'pwrite' successful
early cancel test of 'fsync' successful
early cancel test of 'fdatasync' successful
early cancel test of 'msync' successful
got size 4512
early cancel test of 'sendto' successful
early cancel test of 'sendmsg' successful
early cancel test of 'creat' successful
early cancel test of 'connect' successful
early cancel test of 'tcdrain' successful
early cancel test of 'msgrcv' successful
early cancel test of 'msgsnd' successful
dave@atlas:~/gnu/glibc/objdir$ echo $?
1

Dave

On 2021-09-08 4:44 p.m., Helge Deller wrote:
> We can move the INSN_LDI_R20 instruction into the branch delay slot.
>
> Signed-off-by: Helge Deller <deller@gmx.de>
> ---
>   arch/parisc/include/asm/rt_sigframe.h |  2 +-
>   arch/parisc/kernel/signal.c           | 13 ++++++-------
>   arch/parisc/kernel/signal32.h         |  2 +-
>   3 files changed, 8 insertions(+), 9 deletions(-)
>
> diff --git a/arch/parisc/include/asm/rt_sigframe.h b/arch/parisc/include/asm/rt_sigframe.h
> index 2b3010ade00e..4b9e3d707571 100644
> --- a/arch/parisc/include/asm/rt_sigframe.h
> +++ b/arch/parisc/include/asm/rt_sigframe.h
> @@ -2,7 +2,7 @@
>   #ifndef _ASM_PARISC_RT_SIGFRAME_H
>   #define _ASM_PARISC_RT_SIGFRAME_H
>
> -#define SIGRETURN_TRAMP 4
> +#define SIGRETURN_TRAMP 3
>   #define SIGRESTARTBLOCK_TRAMP 5
>   #define TRAMP_SIZE (SIGRETURN_TRAMP + SIGRESTARTBLOCK_TRAMP)
>
> diff --git a/arch/parisc/kernel/signal.c b/arch/parisc/kernel/signal.c
> index 46b1050640b8..bbfe23c40c01 100644
> --- a/arch/parisc/kernel/signal.c
> +++ b/arch/parisc/kernel/signal.c
> @@ -288,22 +288,21 @@ setup_rt_frame(struct ksignal *ksig, sigset_t *set, struct pt_regs *regs,
>   	   already in userspace. The first words of tramp are used to
>   	   save the previous sigrestartblock trampoline that might be
>   	   on the stack. We start the sigreturn trampoline at
> -	   SIGRESTARTBLOCK_TRAMP+X. */
> +	   SIGRESTARTBLOCK_TRAMP. */
>   	err |= __put_user(in_syscall ? INSN_LDI_R25_1 : INSN_LDI_R25_0,
>   			&frame->tramp[SIGRESTARTBLOCK_TRAMP+0]);
> -	err |= __put_user(INSN_LDI_R20,
> -			&frame->tramp[SIGRESTARTBLOCK_TRAMP+1]);
>   	err |= __put_user(INSN_BLE_SR2_R0,
> +			&frame->tramp[SIGRESTARTBLOCK_TRAMP+1]);
> +	err |= __put_user(INSN_LDI_R20,
>   			&frame->tramp[SIGRESTARTBLOCK_TRAMP+2]);
> -	err |= __put_user(INSN_NOP, &frame->tramp[SIGRESTARTBLOCK_TRAMP+3]);
>
> -	start = (unsigned long) &frame->tramp[0];
> -	end = (unsigned long) &frame->tramp[TRAMP_SIZE];
> +	start = (unsigned long) &frame->tramp[SIGRESTARTBLOCK_TRAMP+0];
> +	end = (unsigned long) &frame->tramp[SIGRESTARTBLOCK_TRAMP+3];
>   	flush_user_dcache_range_asm(start, end);
>   	flush_user_icache_range_asm(start, end);
>
>   	/* TRAMP Words 0-4, Length 5 = SIGRESTARTBLOCK_TRAMP
> -	 * TRAMP Words 5-9, Length 4 = SIGRETURN_TRAMP
> +	 * TRAMP Words 5-7, Length 3 = SIGRETURN_TRAMP
>   	 * So the SIGRETURN_TRAMP is at the end of SIGRESTARTBLOCK_TRAMP
>   	 */
>   	rp = (unsigned long) &frame->tramp[SIGRESTARTBLOCK_TRAMP];
> diff --git a/arch/parisc/kernel/signal32.h b/arch/parisc/kernel/signal32.h
> index f166250f2d06..a5bdbb5678b7 100644
> --- a/arch/parisc/kernel/signal32.h
> +++ b/arch/parisc/kernel/signal32.h
> @@ -36,7 +36,7 @@ struct compat_regfile {
>           compat_int_t rf_sar;
>   };
>
> -#define COMPAT_SIGRETURN_TRAMP 4
> +#define COMPAT_SIGRETURN_TRAMP 3
>   #define COMPAT_SIGRESTARTBLOCK_TRAMP 5
>   #define COMPAT_TRAMP_SIZE (COMPAT_SIGRETURN_TRAMP + \
>   				COMPAT_SIGRESTARTBLOCK_TRAMP)
> --
> 2.31.1
>


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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/4] parisc: Reduce sigreturn trampoline to 3 instructions
  2021-11-15 22:25   ` John David Anglin
@ 2021-11-16 13:54     ` Helge Deller
  2021-11-16 15:31       ` John David Anglin
  0 siblings, 1 reply; 15+ messages in thread
From: Helge Deller @ 2021-11-16 13:54 UTC (permalink / raw)
  To: John David Anglin, linux-parisc; +Cc: James Bottomley

Dave,

Thanks for tracking this down!

I wonder what's wrong with it.
Based on the outcome I should either completely revert that patch or fix it.

Helge

On 11/15/21 23:25, John David Anglin wrote:
> This change breaks signal delivery and causes various glibc tests to fail.
>
> commit 3e4a1aff2a97cb4fd7f0268e4b69e8c9d3641277:
> dave@atlas:~/gnu/glibc/objdir$ env GCONV_PATH=/home/dave/gnu/glibc/objdir/iconvdata LOCPATH=/home/dave/gnu/glibc/objdir/localedata LC_ALL=C /home/dave/gnu/glibc/objdir/elf/ld.so.1 --library-path /home/dave/gnu/glibc/objdir:/home/dave/gnu/glibc/objdir/math:/home/dave/gnu/glibc/objdir/elf:/home/dave/gnu/glibc/objdir/dlfcn:/home/dave/gnu/glibc/objdir/nss:/home/dave/gnu/glibc/objdir/nis:/home/dave/gnu/glibc/objdir/rt:/home/dave/gnu/glibc/objdir/resolv:/home/dave/gnu/glibc/objdir/mathvec:/home/dave/gnu/glibc/objdir/support:/home/dave/gnu/glibc/objdir/crypt:/home/dave/gnu/glibc/objdir/nptl /home/dave/gnu/glibc/objdir/nptl/tst-cancelx4
> got size 4512
> in-time cancel test of 'read' successful
> in-time cancel test of 'readv' successful
> in-time cancel test of 'select' successful
> in-time cancel test of 'pselect' successful
> in-time cancel test of 'poll' successful
> in-time cancel test of 'ppoll' successful
> in-time cancel test of 'write' successful
> in-time cancel test of 'writev' successful
> in-time cancel test of 'sleep' successful
> in-time cancel test of 'usleep' successful
> in-time cancel test of 'nanosleep' successful
> in-time cancel test of 'wait' successful
> in-time cancel test of 'waitid' successful
> in-time cancel test of 'waitpid' successful
> in-time cancel test of 'sigpause' successful
> in-time cancel test of 'sigsuspend' successful
> in-time cancel test of 'sigwait' successful
> in-time cancel test of 'sigwaitinfo' successful
> in-time cancel test of 'sigtimedwait' successful
> in-time cancel test of 'pause' successful
> in-time cancel test of 'accept' successful
> got size 4512
> in-time cancel test of 'send' successful
> in-time cancel test of 'recv' successful
> in-time cancel test of 'recvfrom' successful
> in-time cancel test of 'recvmsg' successful
> in-time cancel test of 'msgrcv' successful
> early cancel test of 'read' successful
> early cancel test of 'readv' successful
> early cancel test of 'select' successful
> early cancel test of 'pselect' successful
> early cancel test of 'poll' successful
> early cancel test of 'ppoll' successful
> early cancel test of 'write' successful
> early cancel test of 'writev' successful
> early cancel test of 'sleep' successful
> early cancel test of 'usleep' successful
> early cancel test of 'nanosleep' successful
> early cancel test of 'wait' successful
> early cancel test of 'waitid' successful
> early cancel test of 'waitpid' successful
> early cancel test of 'sigpause' successful
> early cancel test of 'sigsuspend' successful
> early cancel test of 'sigwait' successful
> early cancel test of 'sigwaitinfo' successful
> early cancel test of 'sigtimedwait' successful
> early cancel test of 'pause' successful
> early cancel test of 'accept' successful
> got size 4512
> early cancel test of 'send' successful
> early cancel test of 'recv' successful
> early cancel test of 'recvfrom' successful
> early cancel test of 'recvmsg' successful
> early cancel test of 'preadv' successful
> early cancel test of 'preadv2' successful
> early cancel test of 'pwritev' successful
> early cancel test of 'pwritev2' successful
> early cancel test of 'open' successful
> early cancel test of 'close' successful
> early cancel test of 'pread' successful
> early cancel test of 'pwrite' successful
> early cancel test of 'fsync' successful
> early cancel test of 'fdatasync' successful
> early cancel test of 'msync' successful
> got size 4512
> early cancel test of 'sendto' successful
> early cancel test of 'sendmsg' successful
> early cancel test of 'creat' successful
> early cancel test of 'connect' successful
> early cancel test of 'tcdrain' successful
> early cancel test of 'msgrcv' successful
> early cancel test of 'msgsnd' successful
> dave@atlas:~/gnu/glibc/objdir$ echo $?
> 0
>
> commit e4f2006f1287e7ea17660490569cff323772dac4:
> dave@atlas:~/gnu/glibc/objdir$ env GCONV_PATH=/home/dave/gnu/glibc/objdir/iconvdata LOCPATH=/home/dave/gnu/glibc/objdir/localedata LC_ALL=C /home/dave/gnu/glibc/objdir/elf/ld.so.1 --library-path /home/dave/gnu/glibc/objdir:/home/dave/gnu/glibc/objdir/math:/home/dave/gnu/glibc/objdir/elf:/home/dave/gnu/glibc/objdir/dlfcn:/home/dave/gnu/glibc/objdir/nss:/home/dave/gnu/glibc/objdir/nis:/home/dave/gnu/glibc/objdir/rt:/home/dave/gnu/glibc/objdir/resolv:/home/dave/gnu/glibc/objdir/mathvec:/home/dave/gnu/glibc/objdir/support:/home/dave/gnu/glibc/objdir/crypt:/home/dave/gnu/glibc/objdir/nptl /home/dave/gnu/glibc/objdir/nptl/tst-cancelx4
> got size 4512
> cleanup handler not called for 'read'
> cleanup handler not called for 'readv'
> cleanup handler not called for 'select'
> cleanup handler not called for 'pselect'
> cleanup handler not called for 'poll'
> cleanup handler not called for 'ppoll'
> cleanup handler not called for 'write'
> cleanup handler not called for 'writev'
> cleanup handler not called for 'sleep'
> cleanup handler not called for 'usleep'
> cleanup handler not called for 'nanosleep'
> cleanup handler not called for 'wait'
> cleanup handler not called for 'waitid'
> cleanup handler not called for 'waitpid'
> cleanup handler not called for 'sigpause'
> cleanup handler not called for 'sigsuspend'
> cleanup handler not called for 'sigwait'
> cleanup handler not called for 'sigwaitinfo'
> cleanup handler not called for 'sigtimedwait'
> cleanup handler not called for 'pause'
> cleanup handler not called for 'accept'
> got size 4512
> cleanup handler not called for 'send'
> cleanup handler not called for 'recv'
> cleanup handler not called for 'recvfrom'
> cleanup handler not called for 'recvmsg'
> cleanup handler not called for 'msgrcv'
> early cancel test of 'read' successful
> early cancel test of 'readv' successful
> early cancel test of 'select' successful
> early cancel test of 'pselect' successful
> early cancel test of 'poll' successful
> early cancel test of 'ppoll' successful
> early cancel test of 'write' successful
> early cancel test of 'writev' successful
> early cancel test of 'sleep' successful
> early cancel test of 'usleep' successful
> early cancel test of 'nanosleep' successful
> early cancel test of 'wait' successful
> early cancel test of 'waitid' successful
> early cancel test of 'waitpid' successful
> early cancel test of 'sigpause' successful
> early cancel test of 'sigsuspend' successful
> early cancel test of 'sigwait' successful
> early cancel test of 'sigwaitinfo' successful
> early cancel test of 'sigtimedwait' successful
> early cancel test of 'pause' successful
> early cancel test of 'accept' successful
> got size 4512
> early cancel test of 'send' successful
> early cancel test of 'recv' successful
> early cancel test of 'recvfrom' successful
> early cancel test of 'recvmsg' successful
> early cancel test of 'preadv' successful
> early cancel test of 'preadv2' successful
> early cancel test of 'pwritev' successful
> early cancel test of 'pwritev2' successful
> early cancel test of 'open' successful
> early cancel test of 'close' successful
> early cancel test of 'pread' successful
> early cancel test of 'pwrite' successful
> early cancel test of 'fsync' successful
> early cancel test of 'fdatasync' successful
> early cancel test of 'msync' successful
> got size 4512
> early cancel test of 'sendto' successful
> early cancel test of 'sendmsg' successful
> early cancel test of 'creat' successful
> early cancel test of 'connect' successful
> early cancel test of 'tcdrain' successful
> early cancel test of 'msgrcv' successful
> early cancel test of 'msgsnd' successful
> dave@atlas:~/gnu/glibc/objdir$ echo $?
> 1
>
> Dave
>
> On 2021-09-08 4:44 p.m., Helge Deller wrote:
>> We can move the INSN_LDI_R20 instruction into the branch delay slot.
>>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>> ---
>>   arch/parisc/include/asm/rt_sigframe.h |  2 +-
>>   arch/parisc/kernel/signal.c           | 13 ++++++-------
>>   arch/parisc/kernel/signal32.h         |  2 +-
>>   3 files changed, 8 insertions(+), 9 deletions(-)
>>
>> diff --git a/arch/parisc/include/asm/rt_sigframe.h b/arch/parisc/include/asm/rt_sigframe.h
>> index 2b3010ade00e..4b9e3d707571 100644
>> --- a/arch/parisc/include/asm/rt_sigframe.h
>> +++ b/arch/parisc/include/asm/rt_sigframe.h
>> @@ -2,7 +2,7 @@
>>   #ifndef _ASM_PARISC_RT_SIGFRAME_H
>>   #define _ASM_PARISC_RT_SIGFRAME_H
>>
>> -#define SIGRETURN_TRAMP 4
>> +#define SIGRETURN_TRAMP 3
>>   #define SIGRESTARTBLOCK_TRAMP 5
>>   #define TRAMP_SIZE (SIGRETURN_TRAMP + SIGRESTARTBLOCK_TRAMP)
>>
>> diff --git a/arch/parisc/kernel/signal.c b/arch/parisc/kernel/signal.c
>> index 46b1050640b8..bbfe23c40c01 100644
>> --- a/arch/parisc/kernel/signal.c
>> +++ b/arch/parisc/kernel/signal.c
>> @@ -288,22 +288,21 @@ setup_rt_frame(struct ksignal *ksig, sigset_t *set, struct pt_regs *regs,
>>          already in userspace. The first words of tramp are used to
>>          save the previous sigrestartblock trampoline that might be
>>          on the stack. We start the sigreturn trampoline at
>> -       SIGRESTARTBLOCK_TRAMP+X. */
>> +       SIGRESTARTBLOCK_TRAMP. */
>>       err |= __put_user(in_syscall ? INSN_LDI_R25_1 : INSN_LDI_R25_0,
>>               &frame->tramp[SIGRESTARTBLOCK_TRAMP+0]);
>> -    err |= __put_user(INSN_LDI_R20,
>> -            &frame->tramp[SIGRESTARTBLOCK_TRAMP+1]);
>>       err |= __put_user(INSN_BLE_SR2_R0,
>> +            &frame->tramp[SIGRESTARTBLOCK_TRAMP+1]);
>> +    err |= __put_user(INSN_LDI_R20,
>>               &frame->tramp[SIGRESTARTBLOCK_TRAMP+2]);
>> -    err |= __put_user(INSN_NOP, &frame->tramp[SIGRESTARTBLOCK_TRAMP+3]);
>>
>> -    start = (unsigned long) &frame->tramp[0];
>> -    end = (unsigned long) &frame->tramp[TRAMP_SIZE];
>> +    start = (unsigned long) &frame->tramp[SIGRESTARTBLOCK_TRAMP+0];
>> +    end = (unsigned long) &frame->tramp[SIGRESTARTBLOCK_TRAMP+3];
>>       flush_user_dcache_range_asm(start, end);
>>       flush_user_icache_range_asm(start, end);
>>
>>       /* TRAMP Words 0-4, Length 5 = SIGRESTARTBLOCK_TRAMP
>> -     * TRAMP Words 5-9, Length 4 = SIGRETURN_TRAMP
>> +     * TRAMP Words 5-7, Length 3 = SIGRETURN_TRAMP
>>        * So the SIGRETURN_TRAMP is at the end of SIGRESTARTBLOCK_TRAMP
>>        */
>>       rp = (unsigned long) &frame->tramp[SIGRESTARTBLOCK_TRAMP];
>> diff --git a/arch/parisc/kernel/signal32.h b/arch/parisc/kernel/signal32.h
>> index f166250f2d06..a5bdbb5678b7 100644
>> --- a/arch/parisc/kernel/signal32.h
>> +++ b/arch/parisc/kernel/signal32.h
>> @@ -36,7 +36,7 @@ struct compat_regfile {
>>           compat_int_t rf_sar;
>>   };
>>
>> -#define COMPAT_SIGRETURN_TRAMP 4
>> +#define COMPAT_SIGRETURN_TRAMP 3
>>   #define COMPAT_SIGRESTARTBLOCK_TRAMP 5
>>   #define COMPAT_TRAMP_SIZE (COMPAT_SIGRETURN_TRAMP + \
>>                   COMPAT_SIGRESTARTBLOCK_TRAMP)
>> --
>> 2.31.1
>>
>
>


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/4] parisc: Reduce sigreturn trampoline to 3 instructions
  2021-11-16 13:54     ` Helge Deller
@ 2021-11-16 15:31       ` John David Anglin
  2021-11-16 17:08         ` John David Anglin
  0 siblings, 1 reply; 15+ messages in thread
From: John David Anglin @ 2021-11-16 15:31 UTC (permalink / raw)
  To: Helge Deller, linux-parisc; +Cc: James Bottomley

I found the change with a regression search.

I don't know what's wrong with it.  I have reverted it for now.

Here are test results with 5.16.0-rc1+ #1 SMP Mon Nov 15 17:56:32 EST 2021 parisc64:
XPASS: conform/UNIX98/ndbm.h/linknamespace
XPASS: conform/XOPEN2K/ndbm.h/linknamespace
XPASS: conform/XOPEN2K8/ndbm.h/linknamespace
XPASS: conform/XPG42/ndbm.h/linknamespace
FAIL: dirent/tst-readdir64-compat
FAIL: elf/tst-audit2
UNSUPPORTED: io/tst-fallocate
UNSUPPORTED: io/tst-fallocate64
UNSUPPORTED: misc/tst-adjtimex
UNSUPPORTED: misc/tst-adjtimex-time64
UNSUPPORTED: misc/tst-clock_adjtime
UNSUPPORTED: misc/tst-clock_adjtime-time64
UNSUPPORTED: misc/tst-ntp_adjtime
UNSUPPORTED: misc/tst-ntp_adjtime-time64
UNSUPPORTED: misc/tst-pkey
UNSUPPORTED: nptl/test-cond-printers
UNSUPPORTED: nptl/test-condattr-printers
UNSUPPORTED: nptl/test-mutex-printers
UNSUPPORTED: nptl/test-mutexattr-printers
UNSUPPORTED: nptl/test-rwlock-printers
UNSUPPORTED: nptl/test-rwlockattr-printers
FAIL: nptl/tst-cleanupx4
FAIL: nptl/tst-cond25
FAIL: signal/tst-minsigstksz-5
UNSUPPORTED: stdlib/test-bz22786
FAIL: stdlib/tst-setcontext2
FAIL: stdlib/tst-setcontext7
UNSUPPORTED: stdlib/tst-strtod-overflow
UNSUPPORTED: string/tst-memmove-overflow
UNSUPPORTED: time/tst-clock_settime
UNSUPPORTED: time/tst-clock_settime-time64
UNSUPPORTED: time/tst-settimeofday
UNSUPPORTED: time/tst-settimeofday-time64
UNSUPPORTED: time/tst-y2039
Summary of test results:
       7 FAIL
    4173 PASS
      23 UNSUPPORTED
      16 XFAIL
       4 XPASS

Probably, Iecho $? should investigate why nptl/tst-cleanupx4 fails.  Might be another issue with signal delivery.

Dave
On 2021-11-16 8:54 a.m., Helge Deller wrote:
> Dave,
>
> Thanks for tracking this down!
>
> I wonder what's wrong with it.
> Based on the outcome I should either completely revert that patch or fix it.
>
> Helge
>
> On 11/15/21 23:25, John David Anglin wrote:
>> This change breaks signal delivery and causes various glibc tests to fail.
>>
>> commit 3e4a1aff2a97cb4fd7f0268e4b69e8c9d3641277:
>> dave@atlas:~/gnu/glibc/objdir$ env GCONV_PATH=/home/dave/gnu/glibc/objdir/iconvdata LOCPATH=/home/dave/gnu/glibc/objdir/localedata LC_ALL=C /home/dave/gnu/glibc/objdir/elf/ld.so.1 --library-path /home/dave/gnu/glibc/objdir:/home/dave/gnu/glibc/objdir/math:/home/dave/gnu/glibc/objdir/elf:/home/dave/gnu/glibc/objdir/dlfcn:/home/dave/gnu/glibc/objdir/nss:/home/dave/gnu/glibc/objdir/nis:/home/dave/gnu/glibc/objdir/rt:/home/dave/gnu/glibc/objdir/resolv:/home/dave/gnu/glibc/objdir/mathvec:/home/dave/gnu/glibc/objdir/support:/home/dave/gnu/glibc/objdir/crypt:/home/dave/gnu/glibc/objdir/nptl /home/dave/gnu/glibc/objdir/nptl/tst-cancelx4
>> got size 4512
>> in-time cancel test of 'read' successful
>> in-time cancel test of 'readv' successful
>> in-time cancel test of 'select' successful
>> in-time cancel test of 'pselect' successful
>> in-time cancel test of 'poll' successful
>> in-time cancel test of 'ppoll' successful
>> in-time cancel test of 'write' successful
>> in-time cancel test of 'writev' successful
>> in-time cancel test of 'sleep' successful
>> in-time cancel test of 'usleep' successful
>> in-time cancel test of 'nanosleep' successful
>> in-time cancel test of 'wait' successful
>> in-time cancel test of 'waitid' successful
>> in-time cancel test of 'waitpid' successful
>> in-time cancel test of 'sigpause' successful
>> in-time cancel test of 'sigsuspend' successful
>> in-time cancel test of 'sigwait' successful
>> in-time cancel test of 'sigwaitinfo' successful
>> in-time cancel test of 'sigtimedwait' successful
>> in-time cancel test of 'pause' successful
>> in-time cancel test of 'accept' successful
>> got size 4512
>> in-time cancel test of 'send' successful
>> in-time cancel test of 'recv' successful
>> in-time cancel test of 'recvfrom' successful
>> in-time cancel test of 'recvmsg' successful
>> in-time cancel test of 'msgrcv' successful
>> early cancel test of 'read' successful
>> early cancel test of 'readv' successful
>> early cancel test of 'select' successful
>> early cancel test of 'pselect' successful
>> early cancel test of 'poll' successful
>> early cancel test of 'ppoll' successful
>> early cancel test of 'write' successful
>> early cancel test of 'writev' successful
>> early cancel test of 'sleep' successful
>> early cancel test of 'usleep' successful
>> early cancel test of 'nanosleep' successful
>> early cancel test of 'wait' successful
>> early cancel test of 'waitid' successful
>> early cancel test of 'waitpid' successful
>> early cancel test of 'sigpause' successful
>> early cancel test of 'sigsuspend' successful
>> early cancel test of 'sigwait' successful
>> early cancel test of 'sigwaitinfo' successful
>> early cancel test of 'sigtimedwait' successful
>> early cancel test of 'pause' successful
>> early cancel test of 'accept' successful
>> got size 4512
>> early cancel test of 'send' successful
>> early cancel test of 'recv' successful
>> early cancel test of 'recvfrom' successful
>> early cancel test of 'recvmsg' successful
>> early cancel test of 'preadv' successful
>> early cancel test of 'preadv2' successful
>> early cancel test of 'pwritev' successful
>> early cancel test of 'pwritev2' successful
>> early cancel test of 'open' successful
>> early cancel test of 'close' successful
>> early cancel test of 'pread' successful
>> early cancel test of 'pwrite' successful
>> early cancel test of 'fsync' successful
>> early cancel test of 'fdatasync' successful
>> early cancel test of 'msync' successful
>> got size 4512
>> early cancel test of 'sendto' successful
>> early cancel test of 'sendmsg' successful
>> early cancel test of 'creat' successful
>> early cancel test of 'connect' successful
>> early cancel test of 'tcdrain' successful
>> early cancel test of 'msgrcv' successful
>> early cancel test of 'msgsnd' successful
>> dave@atlas:~/gnu/glibc/objdir$ echo $?
>> 0
>>
>> commit e4f2006f1287e7ea17660490569cff323772dac4:
>> dave@atlas:~/gnu/glibc/objdir$ env GCONV_PATH=/home/dave/gnu/glibc/objdir/iconvdata LOCPATH=/home/dave/gnu/glibc/objdir/localedata LC_ALL=C /home/dave/gnu/glibc/objdir/elf/ld.so.1 --library-path /home/dave/gnu/glibc/objdir:/home/dave/gnu/glibc/objdir/math:/home/dave/gnu/glibc/objdir/elf:/home/dave/gnu/glibc/objdir/dlfcn:/home/dave/gnu/glibc/objdir/nss:/home/dave/gnu/glibc/objdir/nis:/home/dave/gnu/glibc/objdir/rt:/home/dave/gnu/glibc/objdir/resolv:/home/dave/gnu/glibc/objdir/mathvec:/home/dave/gnu/glibc/objdir/support:/home/dave/gnu/glibc/objdir/crypt:/home/dave/gnu/glibc/objdir/nptl /home/dave/gnu/glibc/objdir/nptl/tst-cancelx4
>> got size 4512
>> cleanup handler not called for 'read'
>> cleanup handler not called for 'readv'
>> cleanup handler not called for 'select'
>> cleanup handler not called for 'pselect'
>> cleanup handler not called for 'poll'
>> cleanup handler not called for 'ppoll'
>> cleanup handler not called for 'write'
>> cleanup handler not called for 'writev'
>> cleanup handler not called for 'sleep'
>> cleanup handler not called for 'usleep'
>> cleanup handler not called for 'nanosleep'
>> cleanup handler not called for 'wait'
>> cleanup handler not called for 'waitid'
>> cleanup handler not called for 'waitpid'
>> cleanup handler not called for 'sigpause'
>> cleanup handler not called for 'sigsuspend'
>> cleanup handler not called for 'sigwait'
>> cleanup handler not called for 'sigwaitinfo'
>> cleanup handler not called for 'sigtimedwait'
>> cleanup handler not called for 'pause'
>> cleanup handler not called for 'accept'
>> got size 4512
>> cleanup handler not called for 'send'
>> cleanup handler not called for 'recv'
>> cleanup handler not called for 'recvfrom'
>> cleanup handler not called for 'recvmsg'
>> cleanup handler not called for 'msgrcv'
>> early cancel test of 'read' successful
>> early cancel test of 'readv' successful
>> early cancel test of 'select' successful
>> early cancel test of 'pselect' successful
>> early cancel test of 'poll' successful
>> early cancel test of 'ppoll' successful
>> early cancel test of 'write' successful
>> early cancel test of 'writev' successful
>> early cancel test of 'sleep' successful
>> early cancel test of 'usleep' successful
>> early cancel test of 'nanosleep' successful
>> early cancel test of 'wait' successful
>> early cancel test of 'waitid' successful
>> early cancel test of 'waitpid' successful
>> early cancel test of 'sigpause' successful
>> early cancel test of 'sigsuspend' successful
>> early cancel test of 'sigwait' successful
>> early cancel test of 'sigwaitinfo' successful
>> early cancel test of 'sigtimedwait' successful
>> early cancel test of 'pause' successful
>> early cancel test of 'accept' successful
>> got size 4512
>> early cancel test of 'send' successful
>> early cancel test of 'recv' successful
>> early cancel test of 'recvfrom' successful
>> early cancel test of 'recvmsg' successful
>> early cancel test of 'preadv' successful
>> early cancel test of 'preadv2' successful
>> early cancel test of 'pwritev' successful
>> early cancel test of 'pwritev2' successful
>> early cancel test of 'open' successful
>> early cancel test of 'close' successful
>> early cancel test of 'pread' successful
>> early cancel test of 'pwrite' successful
>> early cancel test of 'fsync' successful
>> early cancel test of 'fdatasync' successful
>> early cancel test of 'msync' successful
>> got size 4512
>> early cancel test of 'sendto' successful
>> early cancel test of 'sendmsg' successful
>> early cancel test of 'creat' successful
>> early cancel test of 'connect' successful
>> early cancel test of 'tcdrain' successful
>> early cancel test of 'msgrcv' successful
>> early cancel test of 'msgsnd' successful
>> dave@atlas:~/gnu/glibc/objdir$ echo $?
>> 1
>>
>> Dave
>>
>> On 2021-09-08 4:44 p.m., Helge Deller wrote:
>>> We can move the INSN_LDI_R20 instruction into the branch delay slot.
>>>
>>> Signed-off-by: Helge Deller <deller@gmx.de>
>>> ---
>>>    arch/parisc/include/asm/rt_sigframe.h |  2 +-
>>>    arch/parisc/kernel/signal.c           | 13 ++++++-------
>>>    arch/parisc/kernel/signal32.h         |  2 +-
>>>    3 files changed, 8 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/arch/parisc/include/asm/rt_sigframe.h b/arch/parisc/include/asm/rt_sigframe.h
>>> index 2b3010ade00e..4b9e3d707571 100644
>>> --- a/arch/parisc/include/asm/rt_sigframe.h
>>> +++ b/arch/parisc/include/asm/rt_sigframe.h
>>> @@ -2,7 +2,7 @@
>>>    #ifndef _ASM_PARISC_RT_SIGFRAME_H
>>>    #define _ASM_PARISC_RT_SIGFRAME_H
>>>
>>> -#define SIGRETURN_TRAMP 4
>>> +#define SIGRETURN_TRAMP 3
>>>    #define SIGRESTARTBLOCK_TRAMP 5
>>>    #define TRAMP_SIZE (SIGRETURN_TRAMP + SIGRESTARTBLOCK_TRAMP)
>>>
>>> diff --git a/arch/parisc/kernel/signal.c b/arch/parisc/kernel/signal.c
>>> index 46b1050640b8..bbfe23c40c01 100644
>>> --- a/arch/parisc/kernel/signal.c
>>> +++ b/arch/parisc/kernel/signal.c
>>> @@ -288,22 +288,21 @@ setup_rt_frame(struct ksignal *ksig, sigset_t *set, struct pt_regs *regs,
>>>           already in userspace. The first words of tramp are used to
>>>           save the previous sigrestartblock trampoline that might be
>>>           on the stack. We start the sigreturn trampoline at
>>> -       SIGRESTARTBLOCK_TRAMP+X. */
>>> +       SIGRESTARTBLOCK_TRAMP. */
>>>        err |= __put_user(in_syscall ? INSN_LDI_R25_1 : INSN_LDI_R25_0,
>>>                &frame->tramp[SIGRESTARTBLOCK_TRAMP+0]);
>>> -    err |= __put_user(INSN_LDI_R20,
>>> -            &frame->tramp[SIGRESTARTBLOCK_TRAMP+1]);
>>>        err |= __put_user(INSN_BLE_SR2_R0,
>>> +            &frame->tramp[SIGRESTARTBLOCK_TRAMP+1]);
>>> +    err |= __put_user(INSN_LDI_R20,
>>>                &frame->tramp[SIGRESTARTBLOCK_TRAMP+2]);
>>> -    err |= __put_user(INSN_NOP, &frame->tramp[SIGRESTARTBLOCK_TRAMP+3]);
>>>
>>> -    start = (unsigned long) &frame->tramp[0];
>>> -    end = (unsigned long) &frame->tramp[TRAMP_SIZE];
>>> +    start = (unsigned long) &frame->tramp[SIGRESTARTBLOCK_TRAMP+0];
>>> +    end = (unsigned long) &frame->tramp[SIGRESTARTBLOCK_TRAMP+3];
>>>        flush_user_dcache_range_asm(start, end);
>>>        flush_user_icache_range_asm(start, end);
>>>
>>>        /* TRAMP Words 0-4, Length 5 = SIGRESTARTBLOCK_TRAMP
>>> -     * TRAMP Words 5-9, Length 4 = SIGRETURN_TRAMP
>>> +     * TRAMP Words 5-7, Length 3 = SIGRETURN_TRAMP
>>>         * So the SIGRETURN_TRAMP is at the end of SIGRESTARTBLOCK_TRAMP
>>>         */
>>>        rp = (unsigned long) &frame->tramp[SIGRESTARTBLOCK_TRAMP];
>>> diff --git a/arch/parisc/kernel/signal32.h b/arch/parisc/kernel/signal32.h
>>> index f166250f2d06..a5bdbb5678b7 100644
>>> --- a/arch/parisc/kernel/signal32.h
>>> +++ b/arch/parisc/kernel/signal32.h
>>> @@ -36,7 +36,7 @@ struct compat_regfile {
>>>            compat_int_t rf_sar;
>>>    };
>>>
>>> -#define COMPAT_SIGRETURN_TRAMP 4
>>> +#define COMPAT_SIGRETURN_TRAMP 3
>>>    #define COMPAT_SIGRESTARTBLOCK_TRAMP 5
>>>    #define COMPAT_TRAMP_SIZE (COMPAT_SIGRETURN_TRAMP + \
>>>                    COMPAT_SIGRESTARTBLOCK_TRAMP)
>>> --
>>> 2.31.1
>>>
>>


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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/4] parisc: Reduce sigreturn trampoline to 3 instructions
  2021-11-16 15:31       ` John David Anglin
@ 2021-11-16 17:08         ` John David Anglin
  2021-11-16 19:02           ` Helge Deller
  0 siblings, 1 reply; 15+ messages in thread
From: John David Anglin @ 2021-11-16 17:08 UTC (permalink / raw)
  To: Helge Deller, linux-parisc; +Cc: James Bottomley

On 2021-11-16 10:31 a.m., John David Anglin wrote:
> I don't know what's wrong with it.  I have reverted it for now.
I think problem must be with cache flush range.

Dave

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


^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/4] parisc: Reduce sigreturn trampoline to 3 instructions
  2021-11-16 17:08         ` John David Anglin
@ 2021-11-16 19:02           ` Helge Deller
  2021-11-16 19:12             ` John David Anglin
  0 siblings, 1 reply; 15+ messages in thread
From: Helge Deller @ 2021-11-16 19:02 UTC (permalink / raw)
  To: John David Anglin, linux-parisc; +Cc: James Bottomley

On 11/16/21 18:08, John David Anglin wrote:
> On 2021-11-16 10:31 a.m., John David Anglin wrote:
>> I don't know what's wrong with it.  I have reverted it for now.
> I think problem must be with cache flush range.

Not sure. Seems like reverting SIGRETURN_TRAMP back to 4 fixes it.

Anyway, I'll revert the whole patch for now.
Maybe we can fix it in v5.17.

Helge

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: [PATCH 4/4] parisc: Reduce sigreturn trampoline to 3 instructions
  2021-11-16 19:02           ` Helge Deller
@ 2021-11-16 19:12             ` John David Anglin
  0 siblings, 0 replies; 15+ messages in thread
From: John David Anglin @ 2021-11-16 19:12 UTC (permalink / raw)
  To: Helge Deller, linux-parisc; +Cc: James Bottomley

On 2021-11-16 2:02 p.m., Helge Deller wrote:
> On 11/16/21 18:08, John David Anglin wrote:
>> On 2021-11-16 10:31 a.m., John David Anglin wrote:
>>> I don't know what's wrong with it.  I have reverted it for now.
>> I think problem must be with cache flush range.
Didn't work 🙁
> Not sure. Seems like reverting SIGRETURN_TRAMP back to 4 fixes it.
>
> Anyway, I'll revert the whole patch for now.
Reverting whole patch fixed issue with 5.15.2 and 5.16.0-rc1.
> Maybe we can fix it in v5.17.
Dave

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


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2021-11-16 19:12 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-08 20:44 [PATCH 1/4] parisc: Drop strnlen_user() in favour of generic version Helge Deller
2021-09-08 20:44 ` [PATCH 2/4] parisc: Drop useless debug info and comments from signal.c Helge Deller
2021-09-08 20:44 ` [PATCH 3/4] parisc: Check user signal stack trampoline is inside TASK_SIZE Helge Deller
2021-09-08 20:44 ` [PATCH 4/4] parisc: Reduce sigreturn trampoline to 3 instructions Helge Deller
2021-11-15 22:25   ` John David Anglin
2021-11-16 13:54     ` Helge Deller
2021-11-16 15:31       ` John David Anglin
2021-11-16 17:08         ` John David Anglin
2021-11-16 19:02           ` Helge Deller
2021-11-16 19:12             ` John David Anglin
2021-09-08 21:26 ` [PATCH 1/4] parisc: Drop strnlen_user() in favour of generic version Arnd Bergmann
2021-09-08 21:40   ` Helge Deller
2021-09-08 22:08     ` Arnd Bergmann
2021-09-09  2:03       ` Helge Deller
2021-09-09  6:51         ` Arnd Bergmann

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.