linux-kernel - Re: [PATCH] ptrace/x86: introduce TS_COMPAT_RESTART to fix get_nr_restart

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20191128153644.GA5508@redhat.com>
Date:   Thu, 28 Nov 2019 16:36:44 +0100
From:   Oleg Nesterov <oleg@...hat.com>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Andy Lutomirski <luto@...nel.org>,
        Borislav Petkov <bp@...en8.de>, Ingo Molnar <mingo@...nel.org>,
        Jan Kratochvil <jan.kratochvil@...hat.com>,
        Pedro Alves <palves@...hat.com>, Peter Anvin <hpa@...or.com>,
        Thomas Gleixner <tglx@...utronix.de>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        the arch/x86 maintainers <x86@...nel.org>
Subject: Re: [PATCH] ptrace/x86: introduce TS_COMPAT_RESTART to fix
 get_nr_restart_syscall()

On 11/27, Linus Torvalds wrote:
>
> On Wed, Nov 27, 2019 at 9:02 AM Oleg Nesterov <oleg@...hat.com> wrote:
> >
> > OK, lets add the new restart_block.nr_restart_syscall field, then we need
> >
> >         void set_restart_block_fn(restart, fn)
> >         {
> >                 restart->nr_restart_syscall = arch_get_nr_restart_syscall()
> >                 restart->fn = fn;
> >         }
>
> No, I'd suggest just adding an arch-specific "unsigned long" to the
> restart data (and not force the naming to something like the system
> call number - that's just an x86 detail), and then something like this
> on x86:
>
>    void arch_set_restart_data(restart)
>    {
>       restart->arch_data = x86_get_restart_syscall();
>   }
>   #define arch_set_restart_data arch_set_restart_data
>
> and then we'd have in generic code something like
>
>   #ifndef arch_set_restart_data
>   #define arch_set_restart_data(block) do { } while (0)
>   #endif

OK, let it be arch_data/arch_set_restart_data, the same thing.

You misunderstood my question, I do not see a good place for the code
above. So I am going to uglify */signal.[ch] files.

See the incomplete patch below, everything else is trivial.

Please tell me if you think I should move this code somewhere else.

Oleg.


diff --git a/arch/x86/include/asm/signal.h b/arch/x86/include/asm/signal.h
index 33d3c88..f536bcb 100644
--- a/arch/x86/include/asm/signal.h
+++ b/arch/x86/include/asm/signal.h
@@ -5,6 +5,10 @@
 #ifndef __ASSEMBLY__
 #include <linux/linkage.h>
 
+struct restart_block;
+extern void arch_set_restart_data(struct restart_block *);
+#define arch_set_restart_data	arch_set_restart_data
+
 /* Most things should be clean enough to redefine this at will, if care
    is taken to make libc match.  */
 
diff --git a/arch/x86/kernel/signal.c b/arch/x86/kernel/signal.c
index 8eb7193..ede5443 100644
--- a/arch/x86/kernel/signal.c
+++ b/arch/x86/kernel/signal.c
@@ -768,6 +768,11 @@ handle_signal(struct ksignal *ksig, struct pt_regs *regs)
 	signal_setup_done(failed, ksig, stepping);
 }
 
+void arch_set_restart_data(struct restart_block *restart)
+{
+	// TODO
+}
+
 static inline unsigned long get_nr_restart_syscall(const struct pt_regs *regs)
 {
 	/*
diff --git a/include/linux/restart_block.h b/include/linux/restart_block.h
index bba2920..d39f836 100644
--- a/include/linux/restart_block.h
+++ b/include/linux/restart_block.h
@@ -55,6 +55,8 @@ struct restart_block {
 	};
 };
 
+extern long set_restart_fn(struct restart_block *restart,
+				long (*fn)(struct restart_block *));
 extern long do_no_restart_syscall(struct restart_block *parm);
 
 #endif /* __LINUX_RESTART_BLOCK_H */
diff --git a/include/linux/signal.h b/include/linux/signal.h
index 1a5f883..542499f 100644
--- a/include/linux/signal.h
+++ b/include/linux/signal.h
@@ -5,12 +5,17 @@
 #include <linux/bug.h>
 #include <linux/signal_types.h>
 #include <linux/string.h>
+#include <linux/restart_block.h>
 
 struct task_struct;
 
 /* for sysctl */
 extern int print_fatal_signals;
 
+#ifndef arch_set_restart_data
+#define arch_set_restart_data(restart) do { } while (0)
+#endif
+
 static inline void copy_siginfo(kernel_siginfo_t *to,
 				const kernel_siginfo_t *from)
 {
diff --git a/kernel/signal.c b/kernel/signal.c
index bcd46f5..d6402e6 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -4493,6 +4493,14 @@ SYSCALL_DEFINE3(sigsuspend, int, unused1, int, unused2, old_sigset_t, mask)
 }
 #endif
 
+long set_restart_fn(struct restart_block *restart,
+			long (*fn)(struct restart_block *))
+{
+	restart->fn = fn;
+	arch_set_restart_data(restart);
+	return -ERESTART_RESTARTBLOCK;
+}
+
 __weak const char *arch_vma_name(struct vm_area_struct *vma)
 {
 	return NULL;