[PATCH] MIPS: Provide correct siginfo_t.si_stime

[PATCH] MIPS: Provide correct siginfo_t.si_stime

[Petr Malat]

From: Petr Malat <oss@malat.biz>

Provide correct siginfo_t.si_stime on MIPS64

Bug description:
MIPS version of copy_siginfo() is not aware of alignment on platforms with
64-bit long integers, which leads to an incorrect si_stime passed to signal
handlers, because the last element (si_stime) of _sifields._sigchld is not
copied. If _MIPS_SZLONG is 64, then the _sifields starts at the offset of 
4 * sizeof(int).

Patch description:
Use the generic copy_siginfo, which doesn't have this problem.

Signed-off-by: Petr Malat <oss@malat.biz>
---
Please put me on CC, I'm not signed into the mailing list.

diff -Naurp linux-3.18/arch/mips/include/asm/siginfo.h linux-3.18-new/arch/mips/include/asm/siginfo.h
--- linux-3.18/arch/mips/include/asm/siginfo.h	2014-12-07 23:21:05.000000000 +0100
+++ linux-3.18-new/arch/mips/include/asm/siginfo.h	1970-01-01 01:00:00.000000000 +0100
@@ -1,29 +0,0 @@
-/*
- * This file is subject to the terms and conditions of the GNU General Public
- * License.  See the file "COPYING" in the main directory of this archive
- * for more details.
- *
- * Copyright (C) 1998, 1999, 2001, 2003 Ralf Baechle
- * Copyright (C) 2000, 2001 Silicon Graphics, Inc.
- */
-#ifndef _ASM_SIGINFO_H
-#define _ASM_SIGINFO_H
-
-#include <uapi/asm/siginfo.h>
-
-
-/*
- * Duplicated here because of <asm-generic/siginfo.h> braindamage ...
- */
-#include <linux/string.h>
-
-static inline void copy_siginfo(struct siginfo *to, struct siginfo *from)
-{
-	if (from->si_code < 0)
-		memcpy(to, from, sizeof(*to));
-	else
-		/* _sigchld is currently the largest know union member */
-		memcpy(to, from, 3*sizeof(int) + sizeof(from->_sifields._sigchld));
-}
-
-#endif /* _ASM_SIGINFO_H */
diff -Naurp linux-3.18/arch/mips/include/uapi/asm/siginfo.h linux-3.18-new/arch/mips/include/uapi/asm/siginfo.h
--- linux-3.18/arch/mips/include/uapi/asm/siginfo.h	2014-12-07 23:21:05.000000000 +0100
+++ linux-3.18-new/arch/mips/include/uapi/asm/siginfo.h	2014-12-11 17:11:36.698056810 +0100
@@ -16,13 +16,6 @@
 #define HAVE_ARCH_SIGINFO_T
 
 /*
- * We duplicate the generic versions - <asm-generic/siginfo.h> is just borked
- * by design ...
- */
-#define HAVE_ARCH_COPY_SIGINFO
-struct siginfo;
-
-/*
  * Careful to keep union _sifields from shifting ...
  */
 #if _MIPS_SZLONG == 32
@@ -35,8 +28,9 @@ struct siginfo;
 
 #define __ARCH_SIGSYS
 
-#include <asm-generic/siginfo.h>
+#include <uapi/asm-generic/siginfo.h>
 
+/* We can't use generic siginfo_t, because our si_code and si_errno are swapped */
 typedef struct siginfo {
 	int si_signo;
 	int si_code;
@@ -120,5 +114,6 @@ typedef struct siginfo {
 #define SI_TIMER __SI_CODE(__SI_TIMER, -3) /* sent by timer expiration */
 #define SI_MESGQ __SI_CODE(__SI_MESGQ, -4) /* sent by real time mesq state change */
 
+#include <asm-generic/siginfo.h>
 
 #endif /* _UAPI_ASM_SIGINFO_H */

Re: [PATCH] MIPS: Provide correct siginfo_t.si_stime

[David Daney]

On 12/12/2014 06:28 AM, Petr Malat wrote:
> From: Petr Malat <oss@malat.biz>
>
> Provide correct siginfo_t.si_stime on MIPS64
>
> Bug description:
> MIPS version of copy_siginfo() is not aware of alignment on platforms with
> 64-bit long integers, which leads to an incorrect si_stime passed to signal
> handlers, because the last element (si_stime) of _sifields._sigchld is not
> copied. If _MIPS_SZLONG is 64, then the _sifields starts at the offset of
> 4 * sizeof(int).
>
> Patch description:
> Use the generic copy_siginfo, which doesn't have this problem.
>

Please state how this patch effects binary compatibility with previous 
releases of the kernel.

Thanks,
David Daney



> Signed-off-by: Petr Malat <oss@malat.biz>
> ---
> Please put me on CC, I'm not signed into the mailing list.
>
> diff -Naurp linux-3.18/arch/mips/include/asm/siginfo.h linux-3.18-new/arch/mips/include/asm/siginfo.h
> --- linux-3.18/arch/mips/include/asm/siginfo.h	2014-12-07 23:21:05.000000000 +0100
> +++ linux-3.18-new/arch/mips/include/asm/siginfo.h	1970-01-01 01:00:00.000000000 +0100
> @@ -1,29 +0,0 @@
> -/*
> - * This file is subject to the terms and conditions of the GNU General Public
> - * License.  See the file "COPYING" in the main directory of this archive
> - * for more details.
> - *
> - * Copyright (C) 1998, 1999, 2001, 2003 Ralf Baechle
> - * Copyright (C) 2000, 2001 Silicon Graphics, Inc.
> - */
> -#ifndef _ASM_SIGINFO_H
> -#define _ASM_SIGINFO_H
> -
> -#include <uapi/asm/siginfo.h>
> -
> -
> -/*
> - * Duplicated here because of <asm-generic/siginfo.h> braindamage ...
> - */
> -#include <linux/string.h>
> -
> -static inline void copy_siginfo(struct siginfo *to, struct siginfo *from)
> -{
> -	if (from->si_code < 0)
> -		memcpy(to, from, sizeof(*to));
> -	else
> -		/* _sigchld is currently the largest know union member */
> -		memcpy(to, from, 3*sizeof(int) + sizeof(from->_sifields._sigchld));
> -}
> -
> -#endif /* _ASM_SIGINFO_H */
> diff -Naurp linux-3.18/arch/mips/include/uapi/asm/siginfo.h linux-3.18-new/arch/mips/include/uapi/asm/siginfo.h
> --- linux-3.18/arch/mips/include/uapi/asm/siginfo.h	2014-12-07 23:21:05.000000000 +0100
> +++ linux-3.18-new/arch/mips/include/uapi/asm/siginfo.h	2014-12-11 17:11:36.698056810 +0100
> @@ -16,13 +16,6 @@
>   #define HAVE_ARCH_SIGINFO_T
>
>   /*
> - * We duplicate the generic versions - <asm-generic/siginfo.h> is just borked
> - * by design ...
> - */
> -#define HAVE_ARCH_COPY_SIGINFO
> -struct siginfo;
> -
> -/*
>    * Careful to keep union _sifields from shifting ...
>    */
>   #if _MIPS_SZLONG == 32
> @@ -35,8 +28,9 @@ struct siginfo;
>
>   #define __ARCH_SIGSYS
>
> -#include <asm-generic/siginfo.h>
> +#include <uapi/asm-generic/siginfo.h>
>
> +/* We can't use generic siginfo_t, because our si_code and si_errno are swapped */
>   typedef struct siginfo {
>   	int si_signo;
>   	int si_code;
> @@ -120,5 +114,6 @@ typedef struct siginfo {
>   #define SI_TIMER __SI_CODE(__SI_TIMER, -3) /* sent by timer expiration */
>   #define SI_MESGQ __SI_CODE(__SI_MESGQ, -4) /* sent by real time mesq state change */
>
> +#include <asm-generic/siginfo.h>
>
>   #endif /* _UAPI_ASM_SIGINFO_H */
>
>

Re: [PATCH] MIPS: Provide correct siginfo_t.si_stime

[Petr Malat]

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

On Fri, Dec 12, 2014 at 09:11:36AM -0800, David Daney wrote:
> Please state how this patch effects binary compatibility with
> previous releases of the kernel.

Hi David,
the kernel returns a random value in the field si_stime. With the patch
applied, the correct value is present in the field. This is the only 
change visible in userspace, because copy_siginfo() is used just for 
coping done in kernel. To the userspace data are copried by a different
function - copy_siginfo_to_user(), which copies field by field, so 
information leakage caused by this change is not possible.

Here is an output from a program (attached), which illustrates the 
issue:

X86_64:
usage.ru_stime 1000 ms
info->si_stime 1000 ms (64)

MIPS (Octeon) with the patch applied:
usage.ru_stime 1000 ms
info->si_stime 1000 ms (64)

MIPS (Octeon) without the patch (3 executions): 
usage.ru_stime 1000 ms
info->si_stime 5532471680 ms (20f9e1c0)
usage.ru_stime 1000 ms
info->si_stime 5532484000 ms (20f9e690)
usage.ru_stime 1000 ms
info->si_stime 5532484640 ms (20f9e6d0)

Regards,
  Petr  

[-- Attachment #2: burn.c --]
[-- Type: text/x-csrc, Size: 1054 bytes --]

//
// Fork a child, which spends 1 second in system and print
// stime obtained from getrusage and stime received in siginfo
// of the SIGCHLD
//

#include <sys/resource.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <sys/time.h>
#include <sys/wait.h>
#include <stdlib.h>
#include <signal.h>
#include <stdio.h>
#include <unistd.h>
#include <fcntl.h>

void handler(int sig, siginfo_t *info, void *context)
{
	printf("info->si_stime %ld ms (%lx)\n", 
			1000 * info->si_stime / sysconf(_SC_CLK_TCK),
			info->si_stime);
}

int main(int argc, char *argv[])
{
	struct sigaction act = { .sa_sigaction = handler, .sa_flags = SA_SIGINFO };
	sigaction(SIGCHLD, &act, NULL);

	if (fork()) {
		wait(NULL);
	} else {
		struct rusage usage;
		do {
			int fd = open("/proc/self/maps", O_RDONLY);
			char buf[4096];

			read(fd, buf, sizeof buf);
			close(fd);
			getrusage(RUSAGE_SELF, &usage);
		} while (usage.ru_stime.tv_sec < 1);
		printf("usage.ru_stime %ld ms\n", 
			1000 * usage.ru_stime.tv_sec + 
			usage.ru_stime.tv_usec / 1000);
	}
	return 0;
}