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: <20191127170234.GA26180@redhat.com>
Date:   Wed, 27 Nov 2019 18:02:35 +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/26, Linus Torvalds wrote:
>
> On Tue, Nov 26, 2019 at 3:08 AM Oleg Nesterov <oleg@...hat.com> wrote:
> >
> > Alternatively we could add ->compat_restart into struct restart_block,
> > logically this is the same thing.
>
> That sounds like the better model to me. That's what the restart_block
> is about: it's supposed to contain the restart information.

I knew ;) OK, I won't argue, I'll send V2.

> I'd much rather see the system call number added into the restart
> block (or just the "compat bit" - but we have that X32 case too, so
> why not put it all there).

apart from x86, who else can use it? after the quick grep I think nobody,
even arm64 and mips which have compat nr_restart's do not need it.

restart_block.arch_restart_block_infp makes more sense, but that would be
even more painful, I do not want to add asm/restart_block.h or
HAVE_ARCH_RESTART_INFO, or use CONFIG_IA32_EMULATION.

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

but somehow I do not see a good place for

	#ifndef arch_get_nr_restart_syscall()
	#define arch_get_nr_restart_syscall()	0
	#endif

Can you suggest a simple solution?

Hmm. Or may be HAVE_ARCH_RESTART is better after all? Say, just

	--- a/include/linux/restart_block.h
	+++ b/include/linux/restart_block.h
	@@ -24,6 +24,9 @@ enum timespec_type {
	  */
	 struct restart_block {
		long (*fn)(struct restart_block *);
	+#ifdef	CONFIG_HAVE_ARCH_RESTART_XXX
	+	int	nr_restart_syscall;
	+#endif
		union {
			/* For futex_wait and futex_wait_requeue_pi */
			struct {
	@@ -55,6 +58,15 @@ struct restart_block {
		};
	 };
	 
	+static inline void set_restart_block_fn(restart, fn)
	+{
	+#ifdef	CONFIG_HAVE_ARCH_RESTART_XXX
	+	extern int arch_get_nr_restart_syscall();
	+	restart->nr_restart_syscall = arch_get_nr_restart_syscall();
	+#endif
	+	restart->fn = fn;
	+}
	+
	 extern long do_no_restart_syscall(struct restart_block *parm);
	 
	 #endif /* __LINUX_RESTART_BLOCK_H */

?

Oleg.