linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
From: heiko.carstens@de.ibm.com (Heiko Carstens)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH 1/5] all: s390: move wrapper infrastructure to generic headers
Date: Thu, 28 Jan 2016 13:16:18 +0100	[thread overview]
Message-ID: <20160128121618.GB5418@osiris> (raw)
In-Reply-To: <1453741047-5498-2-git-send-email-ynorov@caviumnetworks.com>

Hello Yury,

On Mon, Jan 25, 2016 at 07:57:23PM +0300, Yury Norov wrote:
> __SC_COMPAT_CAST for s390 is too specific due to 31-bit pointer length, so it's
> moved to arch/s390/include/asm/compat.h. Generic declaration assumes that long,
> unsigned long and pointer types are all 32-bit length.
> 
> linux/syscalls_structs.h header is introduced, because from now (see next patch)
> structure types listed there are needed for both normal and compat mode.
> 
> cond_syscall_wrapped now defined two symbols: sys_foo() and compat_sys_foo(), if
> compat wrappers are enabled.
> 
> Here __SC_WRAP() macro is introduced as well. s390 doesn't need it as it uses
> asm-generated syscall table. But architectures that generate that tables with
> C code (ARM64/ILP32) should redefine it as '#define __SC_WRAP(name) compat_##name'.
> 
> Signed-off-by: Yury Norov <ynorov@caviumnetworks.com>

...

> diff --git a/include/linux/compat.h b/include/linux/compat.h
> index a76c917..1a761ea 100644
> --- a/include/linux/compat.h
> +++ b/include/linux/compat.h
> @@ -718,4 +718,67 @@ asmlinkage long compat_sys_fanotify_mark(int, unsigned int, __u32, __u32,
>  #define is_compat_task() (0)
>  
>  #endif /* CONFIG_COMPAT */
> +
> +#ifdef CONFIG_COMPAT_WRAPPER
> +
> +#ifndef __TYPE_IS_PTR
> +#define __TYPE_IS_PTR(t) (!__builtin_types_compatible_p(typeof(0?(t)0:0ULL), u64))
> +#endif
> +
> +#ifndef __SC_COMPAT_TYPE
> +#define __SC_COMPAT_TYPE(t, a) \
> +	__typeof(__builtin_choose_expr(sizeof(t) > 4, 0L, (t)0)) a
> +#endif
> +
> +#ifndef __SC_COMPAT_CAST
> +#define __SC_COMPAT_CAST(t, a) ({					\
> +	BUILD_BUG_ON((sizeof(t) > 4) && !__TYPE_IS_L(t) &&		\
> +		     !__TYPE_IS_UL(t) && !__TYPE_IS_PTR(t));		\
> +	((t) ((t)(-1) < 0 ? (s64)(s32)(a) : (u64)(u32)(a)));		\
> +})
> +#endif
> +
> +#ifndef SYSCALL_DEFINE_WRAPx
> +/*
> + * The SYSCALL_DEFINE_WRAP macro generates system call wrappers to be used by
> + * compat tasks. These wrappers will only be used for system calls where only
> + * the system call arguments need sign or zero extension or zeroing of upper
> + * bits of pointers.
> + * Note: since the wrapper function will afterwards call a system call which
> + * again performs zero and sign extension for all system call arguments with
> + * a size of less than eight bytes, these compat wrappers only touch those
> + * system call arguments with a size of eight bytes ((unsigned) long and
> + * pointers). Zero and sign extension for e.g. int parameters will be done by
> + * the regular system call wrappers.
> + */
> +#define SYSCALL_DEFINE_WRAPx(x, name, ...)						\
> +asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__));				\
> +asmlinkage long compat_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))			\
> +		__attribute__((alias(__stringify(compat_SyS##name))));			\
> +asmlinkage long compat_SyS##name(__MAP(x,__SC_COMPAT_TYPE,__VA_ARGS__));		\
> +asmlinkage long compat_SyS##name(__MAP(x,__SC_COMPAT_TYPE,__VA_ARGS__))			\
> +{											\
> +	return sys##name(__MAP(x,__SC_COMPAT_CAST,__VA_ARGS__));			\
> +}											\
> +SYSCALL_DEFINEx(x, name, __VA_ARGS__)
> +#endif
> +
> +#define SYSCALL_DEFINE_WRAP1(name, ...) SYSCALL_DEFINE_WRAPx(1, _##name, __VA_ARGS__)
> +#define SYSCALL_DEFINE_WRAP2(name, ...) SYSCALL_DEFINE_WRAPx(2, _##name, __VA_ARGS__)
> +#define SYSCALL_DEFINE_WRAP3(name, ...) SYSCALL_DEFINE_WRAPx(3, _##name, __VA_ARGS__)
> +#define SYSCALL_DEFINE_WRAP4(name, ...) SYSCALL_DEFINE_WRAPx(4, _##name, __VA_ARGS__)
> +#define SYSCALL_DEFINE_WRAP5(name, ...) SYSCALL_DEFINE_WRAPx(5, _##name, __VA_ARGS__)
> +#define SYSCALL_DEFINE_WRAP6(name, ...) SYSCALL_DEFINE_WRAPx(6, _##name, __VA_ARGS__)
> +
> +#else
> +
> +#define SYSCALL_DEFINE_WRAP1	SYSCALL_DEFINE1
> +#define SYSCALL_DEFINE_WRAP2    SYSCALL_DEFINE2
> +#define SYSCALL_DEFINE_WRAP3    SYSCALL_DEFINE3
> +#define SYSCALL_DEFINE_WRAP4    SYSCALL_DEFINE4
> +#define SYSCALL_DEFINE_WRAP5    SYSCALL_DEFINE5
> +#define SYSCALL_DEFINE_WRAP6    SYSCALL_DEFINE6
> +
> +#endif /* CONFIG_COMPAT_WRAPPER */

How about if you rename SYSCALL_DEFINE_WRAP to SYSCALL_COMPAT_DEFINE which
has the semantics that no dedicated compat system call exists (aka system
call is compat safe).

Then convert all existing SYSCALL_DEFINE'd system calls for which no compat
variant exists to SYSCALL_COMPAT_DEFINE.

This would allow to specify "compat_sys_<syscallname>" in the compat system
call table for _all_ system calls.

No need to look up if a compat variant (or wrapper) exists or
sys_<syscallname> should be used instead. Also no possibility for security
bugs that could creep in because SYSCALL_DEFINE has been used instead of
SYSCALL_DEFINE_WRAP.

Ideally the implementation would only generate an alias if no sign/zero
extension is necessary.

Trivially this would be true for system calls without arguments like e.g.
sys_fork() which would get a compat_sys_fork alias without any further
code.

I'm not sure how difficult it is to implement the same logic for system
calls that have parameters. That is: either generate a
compat_sys_<syscallname> wrapper function, or if the SYSCALL_COMPAT_DEFINE
macro figures out that no zero sign extension is required, only an alias
without any additional code.

I think in the long term something like this is much easier to maintain.

Does that make sense?

  parent reply	other threads:[~2016-01-28 12:16 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-01-25 16:57 [PATCH 0/5] all: s390: make compat wrappers the generic solution Yury Norov
2016-01-25 16:57 ` [PATCH 1/5] all: s390: move wrapper infrastructure to generic headers Yury Norov
2016-01-25 18:10   ` kbuild test robot
2016-01-28 12:16   ` Heiko Carstens [this message]
2016-01-28 16:31     ` Yury Norov
2016-02-01 11:42       ` Yury Norov
2016-02-02  7:39         ` Heiko Carstens
2016-02-02 15:43           ` Yury Norov
2016-02-02 16:08             ` Heiko Carstens
2016-02-02 19:54               ` Heiko Carstens
2016-02-02 20:41                 ` Yury Norov
2016-02-03  8:01                   ` Heiko Carstens
2016-02-17  8:22                   ` Heiko Carstens
2016-02-17 13:57                     ` Yury Norov
2016-01-25 16:57 ` [PATCH 2/5] all: declare new wrappers Yury Norov
2016-01-25 16:57 ` [PATCH 3/5] all: s390: redefine wrappers in generic code Yury Norov
2016-01-25 16:57 ` [PATCH 4/5] all: wrap getdents64 syscall Yury Norov
2016-01-25 16:57 ` [PATCH 5/5] all: introduce COMPAT_WRAPPER option and enable it for s390 Yury Norov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160128121618.GB5418@osiris \
    --to=heiko.carstens@de.ibm.com \
    --cc=linux-arm-kernel@lists.infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).