Re: [PATCH v9 2/4] seccomp: switch system call argument type to void *
On Mon, Dec 03, 2018 at 07:17:26PM -0700, Tycho Andersen wrote: > On Tue, Dec 04, 2018 at 10:07:38AM +0800, kbuild test robot wrote: > > Hi Tycho, > > > > I love your patch! Yet something to improve: > > > > [auto build test ERROR on linus/master] > > [also build test ERROR on v4.20-rc5 next-20181203] > > [if your patch is applied to the wrong git tree, please drop us a note to > > help improve the system] > > > > url: > > https://github.com/0day-ci/linux/commits/Tycho-Andersen/seccomp-hoist-struct-seccomp_data-recalculation-higher/20181204-013450 > > config: i386-randconfig-x005-201848 (attached as .config) > > compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 > > reproduce: > > # save the attached .config to linux build tree > > make ARCH=i386 > > > > All errors (new ones prefixed by >>): > > > >In file included from kernel/seccomp.c:28:0: > > >> include/linux/syscalls.h:239:18: error: conflicting types for > > >> 'sys_seccomp' > > asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \ > > ^ > >include/linux/syscalls.h:225:2: note: in expansion of macro > > '__SYSCALL_DEFINEx' > > __SYSCALL_DEFINEx(x, sname, __VA_ARGS__) > > ^ > >include/linux/syscalls.h:216:36: note: in expansion of macro > > 'SYSCALL_DEFINEx' > > #define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, > > __VA_ARGS__) > >^~~ > >kernel/seccomp.c:946:1: note: in expansion of macro 'SYSCALL_DEFINE3' > > SYSCALL_DEFINE3(seccomp, unsigned int, op, unsigned int, flags, > > ^~~ > >In file included from kernel/seccomp.c:28:0: > >include/linux/syscalls.h:881:17: note: previous declaration of > > 'sys_seccomp' was here > > asmlinkage long sys_seccomp(unsigned int op, unsigned int flags, > > ^~~ > > Huh, I have no idea why I don't see this, but even with the attached > config it still doesn't cause a problem for me. Anyway, I'll fix it up > and do some more investigating... Oh, because it's "make ARCH=i386". Whoosh :) Anyway, it's fixed for v10. Tycho
Re: [PATCH v9 2/4] seccomp: switch system call argument type to void *
On Mon, Dec 03, 2018 at 07:17:26PM -0700, Tycho Andersen wrote: > On Tue, Dec 04, 2018 at 10:07:38AM +0800, kbuild test robot wrote: > > Hi Tycho, > > > > I love your patch! Yet something to improve: > > > > [auto build test ERROR on linus/master] > > [also build test ERROR on v4.20-rc5 next-20181203] > > [if your patch is applied to the wrong git tree, please drop us a note to > > help improve the system] > > > > url: > > https://github.com/0day-ci/linux/commits/Tycho-Andersen/seccomp-hoist-struct-seccomp_data-recalculation-higher/20181204-013450 > > config: i386-randconfig-x005-201848 (attached as .config) > > compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 > > reproduce: > > # save the attached .config to linux build tree > > make ARCH=i386 > > > > All errors (new ones prefixed by >>): > > > >In file included from kernel/seccomp.c:28:0: > > >> include/linux/syscalls.h:239:18: error: conflicting types for > > >> 'sys_seccomp' > > asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \ > > ^ > >include/linux/syscalls.h:225:2: note: in expansion of macro > > '__SYSCALL_DEFINEx' > > __SYSCALL_DEFINEx(x, sname, __VA_ARGS__) > > ^ > >include/linux/syscalls.h:216:36: note: in expansion of macro > > 'SYSCALL_DEFINEx' > > #define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, > > __VA_ARGS__) > >^~~ > >kernel/seccomp.c:946:1: note: in expansion of macro 'SYSCALL_DEFINE3' > > SYSCALL_DEFINE3(seccomp, unsigned int, op, unsigned int, flags, > > ^~~ > >In file included from kernel/seccomp.c:28:0: > >include/linux/syscalls.h:881:17: note: previous declaration of > > 'sys_seccomp' was here > > asmlinkage long sys_seccomp(unsigned int op, unsigned int flags, > > ^~~ > > Huh, I have no idea why I don't see this, but even with the attached > config it still doesn't cause a problem for me. Anyway, I'll fix it up > and do some more investigating... Oh, because it's "make ARCH=i386". Whoosh :) Anyway, it's fixed for v10. Tycho
Re: [PATCH v9 2/4] seccomp: switch system call argument type to void *
On Tue, Dec 04, 2018 at 10:07:38AM +0800, kbuild test robot wrote: > Hi Tycho, > > I love your patch! Yet something to improve: > > [auto build test ERROR on linus/master] > [also build test ERROR on v4.20-rc5 next-20181203] > [if your patch is applied to the wrong git tree, please drop us a note to > help improve the system] > > url: > https://github.com/0day-ci/linux/commits/Tycho-Andersen/seccomp-hoist-struct-seccomp_data-recalculation-higher/20181204-013450 > config: i386-randconfig-x005-201848 (attached as .config) > compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 > reproduce: > # save the attached .config to linux build tree > make ARCH=i386 > > All errors (new ones prefixed by >>): > >In file included from kernel/seccomp.c:28:0: > >> include/linux/syscalls.h:239:18: error: conflicting types for 'sys_seccomp' > asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \ > ^ >include/linux/syscalls.h:225:2: note: in expansion of macro > '__SYSCALL_DEFINEx' > __SYSCALL_DEFINEx(x, sname, __VA_ARGS__) > ^ >include/linux/syscalls.h:216:36: note: in expansion of macro > 'SYSCALL_DEFINEx' > #define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, > __VA_ARGS__) >^~~ >kernel/seccomp.c:946:1: note: in expansion of macro 'SYSCALL_DEFINE3' > SYSCALL_DEFINE3(seccomp, unsigned int, op, unsigned int, flags, > ^~~ >In file included from kernel/seccomp.c:28:0: >include/linux/syscalls.h:881:17: note: previous declaration of > 'sys_seccomp' was here > asmlinkage long sys_seccomp(unsigned int op, unsigned int flags, > ^~~ Huh, I have no idea why I don't see this, but even with the attached config it still doesn't cause a problem for me. Anyway, I'll fix it up and do some more investigating... Tycho
Re: [PATCH v9 2/4] seccomp: switch system call argument type to void *
On Tue, Dec 04, 2018 at 10:07:38AM +0800, kbuild test robot wrote: > Hi Tycho, > > I love your patch! Yet something to improve: > > [auto build test ERROR on linus/master] > [also build test ERROR on v4.20-rc5 next-20181203] > [if your patch is applied to the wrong git tree, please drop us a note to > help improve the system] > > url: > https://github.com/0day-ci/linux/commits/Tycho-Andersen/seccomp-hoist-struct-seccomp_data-recalculation-higher/20181204-013450 > config: i386-randconfig-x005-201848 (attached as .config) > compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 > reproduce: > # save the attached .config to linux build tree > make ARCH=i386 > > All errors (new ones prefixed by >>): > >In file included from kernel/seccomp.c:28:0: > >> include/linux/syscalls.h:239:18: error: conflicting types for 'sys_seccomp' > asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \ > ^ >include/linux/syscalls.h:225:2: note: in expansion of macro > '__SYSCALL_DEFINEx' > __SYSCALL_DEFINEx(x, sname, __VA_ARGS__) > ^ >include/linux/syscalls.h:216:36: note: in expansion of macro > 'SYSCALL_DEFINEx' > #define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, > __VA_ARGS__) >^~~ >kernel/seccomp.c:946:1: note: in expansion of macro 'SYSCALL_DEFINE3' > SYSCALL_DEFINE3(seccomp, unsigned int, op, unsigned int, flags, > ^~~ >In file included from kernel/seccomp.c:28:0: >include/linux/syscalls.h:881:17: note: previous declaration of > 'sys_seccomp' was here > asmlinkage long sys_seccomp(unsigned int op, unsigned int flags, > ^~~ Huh, I have no idea why I don't see this, but even with the attached config it still doesn't cause a problem for me. Anyway, I'll fix it up and do some more investigating... Tycho
Re: [PATCH v9 2/4] seccomp: switch system call argument type to void *
Hi Tycho,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.20-rc5 next-20181203]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Tycho-Andersen/seccomp-hoist-struct-seccomp_data-recalculation-higher/20181204-013450
config: i386-randconfig-x005-201848 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
In file included from kernel/seccomp.c:28:0:
>> include/linux/syscalls.h:239:18: error: conflicting types for 'sys_seccomp'
asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \
^
include/linux/syscalls.h:225:2: note: in expansion of macro
'__SYSCALL_DEFINEx'
__SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
^
include/linux/syscalls.h:216:36: note: in expansion of macro
'SYSCALL_DEFINEx'
#define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, __VA_ARGS__)
^~~
kernel/seccomp.c:946:1: note: in expansion of macro 'SYSCALL_DEFINE3'
SYSCALL_DEFINE3(seccomp, unsigned int, op, unsigned int, flags,
^~~
In file included from kernel/seccomp.c:28:0:
include/linux/syscalls.h:881:17: note: previous declaration of 'sys_seccomp'
was here
asmlinkage long sys_seccomp(unsigned int op, unsigned int flags,
^~~
vim +/sys_seccomp +239 include/linux/syscalls.h
1bd21c6c2 Dominik Brodowski 2018-04-05 228
e145242ea Dominik Brodowski 2018-04-09 229 /*
e145242ea Dominik Brodowski 2018-04-09 230 * The asmlinkage stub is
aliased to a function named __se_sys_*() which
e145242ea Dominik Brodowski 2018-04-09 231 * sign-extends 32-bit ints to
longs whenever needed. The actual work is
e145242ea Dominik Brodowski 2018-04-09 232 * done within __do_sys_*().
e145242ea Dominik Brodowski 2018-04-09 233 */
1bd21c6c2 Dominik Brodowski 2018-04-05 234 #ifndef __SYSCALL_DEFINEx
bed1ffca0 Frederic Weisbecker 2009-03-13 235 #define __SYSCALL_DEFINEx(x,
name, ...) \
bee200317 Arnd Bergmann 2018-06-19 236 __diag_push();
\
bee200317 Arnd Bergmann 2018-06-19 237 __diag_ignore(GCC, 8,
"-Wattribute-alias", \
bee200317 Arnd Bergmann 2018-06-19 238 "Type aliasing is
used to sanitize syscall arguments");\
83460ec8d Andi Kleen 2013-11-12 @239 asmlinkage long
sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \
e145242ea Dominik Brodowski 2018-04-09 240
__attribute__((alias(__stringify(__se_sys##name;\
c9a211951 Howard McLauchlan 2018-03-21 241
ALLOW_ERROR_INJECTION(sys##name, ERRNO);\
e145242ea Dominik Brodowski 2018-04-09 242 static inline long
__do_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__));\
e145242ea Dominik Brodowski 2018-04-09 243 asmlinkage long
__se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__)); \
e145242ea Dominik Brodowski 2018-04-09 244 asmlinkage long
__se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__)) \
1a94bc347 Heiko Carstens 2009-01-14 245 {
\
e145242ea Dominik Brodowski 2018-04-09 246 long ret =
__do_sys##name(__MAP(x,__SC_CAST,__VA_ARGS__));\
07fe6e00f Al Viro 2013-01-21 247
__MAP(x,__SC_TEST,__VA_ARGS__); \
2cf096668 Al Viro 2013-01-21 248 __PROTECT(x,
ret,__MAP(x,__SC_ARGS,__VA_ARGS__)); \
2cf096668 Al Viro 2013-01-21 249 return ret;
\
1a94bc347 Heiko Carstens 2009-01-14 250 }
\
bee200317 Arnd Bergmann 2018-06-19 251 __diag_pop();
\
e145242ea Dominik Brodowski 2018-04-09 252 static inline long
__do_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))
1bd21c6c2 Dominik Brodowski 2018-04-05 253 #endif /* __SYSCALL_DEFINEx */
1a94bc347 Heiko Carstens 2009-01-14 254
:: The code at line 239 was first introduced by commit
:: 83460ec8dcac14142e7860a01fa59c267ac4657c syscalls.h: use gcc alias
instead of assembler aliases for syscalls
:: TO: Andi Kleen
:: CC: Linus Torvalds
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: [PATCH v9 2/4] seccomp: switch system call argument type to void *
Hi Tycho,
I love your patch! Yet something to improve:
[auto build test ERROR on linus/master]
[also build test ERROR on v4.20-rc5 next-20181203]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system]
url:
https://github.com/0day-ci/linux/commits/Tycho-Andersen/seccomp-hoist-struct-seccomp_data-recalculation-higher/20181204-013450
config: i386-randconfig-x005-201848 (attached as .config)
compiler: gcc-7 (Debian 7.3.0-1) 7.3.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
All errors (new ones prefixed by >>):
In file included from kernel/seccomp.c:28:0:
>> include/linux/syscalls.h:239:18: error: conflicting types for 'sys_seccomp'
asmlinkage long sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \
^
include/linux/syscalls.h:225:2: note: in expansion of macro
'__SYSCALL_DEFINEx'
__SYSCALL_DEFINEx(x, sname, __VA_ARGS__)
^
include/linux/syscalls.h:216:36: note: in expansion of macro
'SYSCALL_DEFINEx'
#define SYSCALL_DEFINE3(name, ...) SYSCALL_DEFINEx(3, _##name, __VA_ARGS__)
^~~
kernel/seccomp.c:946:1: note: in expansion of macro 'SYSCALL_DEFINE3'
SYSCALL_DEFINE3(seccomp, unsigned int, op, unsigned int, flags,
^~~
In file included from kernel/seccomp.c:28:0:
include/linux/syscalls.h:881:17: note: previous declaration of 'sys_seccomp'
was here
asmlinkage long sys_seccomp(unsigned int op, unsigned int flags,
^~~
vim +/sys_seccomp +239 include/linux/syscalls.h
1bd21c6c2 Dominik Brodowski 2018-04-05 228
e145242ea Dominik Brodowski 2018-04-09 229 /*
e145242ea Dominik Brodowski 2018-04-09 230 * The asmlinkage stub is
aliased to a function named __se_sys_*() which
e145242ea Dominik Brodowski 2018-04-09 231 * sign-extends 32-bit ints to
longs whenever needed. The actual work is
e145242ea Dominik Brodowski 2018-04-09 232 * done within __do_sys_*().
e145242ea Dominik Brodowski 2018-04-09 233 */
1bd21c6c2 Dominik Brodowski 2018-04-05 234 #ifndef __SYSCALL_DEFINEx
bed1ffca0 Frederic Weisbecker 2009-03-13 235 #define __SYSCALL_DEFINEx(x,
name, ...) \
bee200317 Arnd Bergmann 2018-06-19 236 __diag_push();
\
bee200317 Arnd Bergmann 2018-06-19 237 __diag_ignore(GCC, 8,
"-Wattribute-alias", \
bee200317 Arnd Bergmann 2018-06-19 238 "Type aliasing is
used to sanitize syscall arguments");\
83460ec8d Andi Kleen 2013-11-12 @239 asmlinkage long
sys##name(__MAP(x,__SC_DECL,__VA_ARGS__)) \
e145242ea Dominik Brodowski 2018-04-09 240
__attribute__((alias(__stringify(__se_sys##name;\
c9a211951 Howard McLauchlan 2018-03-21 241
ALLOW_ERROR_INJECTION(sys##name, ERRNO);\
e145242ea Dominik Brodowski 2018-04-09 242 static inline long
__do_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__));\
e145242ea Dominik Brodowski 2018-04-09 243 asmlinkage long
__se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__)); \
e145242ea Dominik Brodowski 2018-04-09 244 asmlinkage long
__se_sys##name(__MAP(x,__SC_LONG,__VA_ARGS__)) \
1a94bc347 Heiko Carstens 2009-01-14 245 {
\
e145242ea Dominik Brodowski 2018-04-09 246 long ret =
__do_sys##name(__MAP(x,__SC_CAST,__VA_ARGS__));\
07fe6e00f Al Viro 2013-01-21 247
__MAP(x,__SC_TEST,__VA_ARGS__); \
2cf096668 Al Viro 2013-01-21 248 __PROTECT(x,
ret,__MAP(x,__SC_ARGS,__VA_ARGS__)); \
2cf096668 Al Viro 2013-01-21 249 return ret;
\
1a94bc347 Heiko Carstens 2009-01-14 250 }
\
bee200317 Arnd Bergmann 2018-06-19 251 __diag_pop();
\
e145242ea Dominik Brodowski 2018-04-09 252 static inline long
__do_sys##name(__MAP(x,__SC_DECL,__VA_ARGS__))
1bd21c6c2 Dominik Brodowski 2018-04-05 253 #endif /* __SYSCALL_DEFINEx */
1a94bc347 Heiko Carstens 2009-01-14 254
:: The code at line 239 was first introduced by commit
:: 83460ec8dcac14142e7860a01fa59c267ac4657c syscalls.h: use gcc alias
instead of assembler aliases for syscalls
:: TO: Andi Kleen
:: CC: Linus Torvalds
---
0-DAY kernel test infrastructureOpen Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
.config.gz
Description: application/gzip
Re: [PATCH v9 2/4] seccomp: switch system call argument type to void *
On Mon, Dec 3, 2018 at 12:01 AM Serge E. Hallyn wrote:
> On Sun, Dec 02, 2018 at 08:28:25PM -0700, Tycho Andersen wrote:
> > The const qualifier causes problems for any code that wants to write to the
> > third argument of the seccomp syscall, as we will do in a future patch in
> > this series.
> >
> > The third argument to the seccomp syscall is documented as void *, so
> > rather than just dropping the const, let's switch everything to use void *
> > as well.
> >
> > I believe this is safe because of 1. the documentation above, 2. there's no
> > real type information exported about syscalls anywhere besides the man
> > pages.
> >
> > Signed-off-by: Tycho Andersen
> > CC: Kees Cook
> > CC: Andy Lutomirski
> > CC: Oleg Nesterov
> > CC: Eric W. Biederman
> > CC: "Serge E. Hallyn"
>
> Acked-by: Serge Hallyn
>
> Though I'm not entirely convinced there will be no ill effects of changing
> the argument type. I'll feel comfortable when Michael and Paul say it's
> fine :)
Well, looking at the seccomp(2) manpage on my system (dated
2018-02-02) the third argument is already shown as a "void *args":
SYNOPSIS
#include
#include
#include
#include
#include
int seccomp(unsigned int operation, unsigned int flags, void *args);
... so I think we're safe :)
>From a libseccomp perspective, we always call seccomp(2) via
syscall(2) so it is unlikely we would ever run into problems, not too
mention that we are just talking about the pointer type used in the
kernel; from a syscall ABI perspective it is still a pointer value and
that is the important part.
> > CC: Christian Brauner
> > CC: Tyler Hicks
> > CC: Akihiro Suda
> > ---
> > include/linux/seccomp.h | 2 +-
> > kernel/seccomp.c| 8
> > 2 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/linux/seccomp.h b/include/linux/seccomp.h
> > index e5320f6c8654..b5103c019cf4 100644
> > --- a/include/linux/seccomp.h
> > +++ b/include/linux/seccomp.h
> > @@ -43,7 +43,7 @@ extern void secure_computing_strict(int this_syscall);
> > #endif
> >
> > extern long prctl_get_seccomp(void);
> > -extern long prctl_set_seccomp(unsigned long, char __user *);
> > +extern long prctl_set_seccomp(unsigned long, void __user *);
> >
> > static inline int seccomp_mode(struct seccomp *s)
> > {
> > diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> > index 96afc32e041d..393e029f778a 100644
> > --- a/kernel/seccomp.c
> > +++ b/kernel/seccomp.c
> > @@ -924,7 +924,7 @@ static long seccomp_get_action_avail(const char __user
> > *uaction)
> >
> > /* Common entry point for both prctl and syscall. */
> > static long do_seccomp(unsigned int op, unsigned int flags,
> > -const char __user *uargs)
> > +void __user *uargs)
> > {
> > switch (op) {
> > case SECCOMP_SET_MODE_STRICT:
> > @@ -944,7 +944,7 @@ static long do_seccomp(unsigned int op, unsigned int
> > flags,
> > }
> >
> > SYSCALL_DEFINE3(seccomp, unsigned int, op, unsigned int, flags,
> > - const char __user *, uargs)
> > + void __user *, uargs)
> > {
> > return do_seccomp(op, flags, uargs);
> > }
> > @@ -956,10 +956,10 @@ SYSCALL_DEFINE3(seccomp, unsigned int, op, unsigned
> > int, flags,
> > *
> > * Returns 0 on success or -EINVAL on failure.
> > */
> > -long prctl_set_seccomp(unsigned long seccomp_mode, char __user *filter)
> > +long prctl_set_seccomp(unsigned long seccomp_mode, void __user *filter)
> > {
> > unsigned int op;
> > - char __user *uargs;
> > + void __user *uargs;
> >
> > switch (seccomp_mode) {
> > case SECCOMP_MODE_STRICT:
> > --
> > 2.19.1
--
paul moore
www.paul-moore.com
Re: [PATCH v9 2/4] seccomp: switch system call argument type to void *
On Mon, Dec 3, 2018 at 12:01 AM Serge E. Hallyn wrote:
> On Sun, Dec 02, 2018 at 08:28:25PM -0700, Tycho Andersen wrote:
> > The const qualifier causes problems for any code that wants to write to the
> > third argument of the seccomp syscall, as we will do in a future patch in
> > this series.
> >
> > The third argument to the seccomp syscall is documented as void *, so
> > rather than just dropping the const, let's switch everything to use void *
> > as well.
> >
> > I believe this is safe because of 1. the documentation above, 2. there's no
> > real type information exported about syscalls anywhere besides the man
> > pages.
> >
> > Signed-off-by: Tycho Andersen
> > CC: Kees Cook
> > CC: Andy Lutomirski
> > CC: Oleg Nesterov
> > CC: Eric W. Biederman
> > CC: "Serge E. Hallyn"
>
> Acked-by: Serge Hallyn
>
> Though I'm not entirely convinced there will be no ill effects of changing
> the argument type. I'll feel comfortable when Michael and Paul say it's
> fine :)
Well, looking at the seccomp(2) manpage on my system (dated
2018-02-02) the third argument is already shown as a "void *args":
SYNOPSIS
#include
#include
#include
#include
#include
int seccomp(unsigned int operation, unsigned int flags, void *args);
... so I think we're safe :)
>From a libseccomp perspective, we always call seccomp(2) via
syscall(2) so it is unlikely we would ever run into problems, not too
mention that we are just talking about the pointer type used in the
kernel; from a syscall ABI perspective it is still a pointer value and
that is the important part.
> > CC: Christian Brauner
> > CC: Tyler Hicks
> > CC: Akihiro Suda
> > ---
> > include/linux/seccomp.h | 2 +-
> > kernel/seccomp.c| 8
> > 2 files changed, 5 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/linux/seccomp.h b/include/linux/seccomp.h
> > index e5320f6c8654..b5103c019cf4 100644
> > --- a/include/linux/seccomp.h
> > +++ b/include/linux/seccomp.h
> > @@ -43,7 +43,7 @@ extern void secure_computing_strict(int this_syscall);
> > #endif
> >
> > extern long prctl_get_seccomp(void);
> > -extern long prctl_set_seccomp(unsigned long, char __user *);
> > +extern long prctl_set_seccomp(unsigned long, void __user *);
> >
> > static inline int seccomp_mode(struct seccomp *s)
> > {
> > diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> > index 96afc32e041d..393e029f778a 100644
> > --- a/kernel/seccomp.c
> > +++ b/kernel/seccomp.c
> > @@ -924,7 +924,7 @@ static long seccomp_get_action_avail(const char __user
> > *uaction)
> >
> > /* Common entry point for both prctl and syscall. */
> > static long do_seccomp(unsigned int op, unsigned int flags,
> > -const char __user *uargs)
> > +void __user *uargs)
> > {
> > switch (op) {
> > case SECCOMP_SET_MODE_STRICT:
> > @@ -944,7 +944,7 @@ static long do_seccomp(unsigned int op, unsigned int
> > flags,
> > }
> >
> > SYSCALL_DEFINE3(seccomp, unsigned int, op, unsigned int, flags,
> > - const char __user *, uargs)
> > + void __user *, uargs)
> > {
> > return do_seccomp(op, flags, uargs);
> > }
> > @@ -956,10 +956,10 @@ SYSCALL_DEFINE3(seccomp, unsigned int, op, unsigned
> > int, flags,
> > *
> > * Returns 0 on success or -EINVAL on failure.
> > */
> > -long prctl_set_seccomp(unsigned long seccomp_mode, char __user *filter)
> > +long prctl_set_seccomp(unsigned long seccomp_mode, void __user *filter)
> > {
> > unsigned int op;
> > - char __user *uargs;
> > + void __user *uargs;
> >
> > switch (seccomp_mode) {
> > case SECCOMP_MODE_STRICT:
> > --
> > 2.19.1
--
paul moore
www.paul-moore.com
Re: [PATCH v9 2/4] seccomp: switch system call argument type to void *
On Sun, Dec 02, 2018 at 08:28:25PM -0700, Tycho Andersen wrote:
> The const qualifier causes problems for any code that wants to write to the
> third argument of the seccomp syscall, as we will do in a future patch in
> this series.
>
> The third argument to the seccomp syscall is documented as void *, so
> rather than just dropping the const, let's switch everything to use void *
> as well.
>
> I believe this is safe because of 1. the documentation above, 2. there's no
> real type information exported about syscalls anywhere besides the man
> pages.
>
> Signed-off-by: Tycho Andersen
> CC: Kees Cook
> CC: Andy Lutomirski
> CC: Oleg Nesterov
> CC: Eric W. Biederman
> CC: "Serge E. Hallyn"
Acked-by: Serge Hallyn
Though I'm not entirely convinced there will be no ill effects of changing
the argument type. I'll feel comfortable when Michael and Paul say it's
fine :)
> CC: Christian Brauner
> CC: Tyler Hicks
> CC: Akihiro Suda
> ---
> include/linux/seccomp.h | 2 +-
> kernel/seccomp.c| 8
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/seccomp.h b/include/linux/seccomp.h
> index e5320f6c8654..b5103c019cf4 100644
> --- a/include/linux/seccomp.h
> +++ b/include/linux/seccomp.h
> @@ -43,7 +43,7 @@ extern void secure_computing_strict(int this_syscall);
> #endif
>
> extern long prctl_get_seccomp(void);
> -extern long prctl_set_seccomp(unsigned long, char __user *);
> +extern long prctl_set_seccomp(unsigned long, void __user *);
>
> static inline int seccomp_mode(struct seccomp *s)
> {
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index 96afc32e041d..393e029f778a 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -924,7 +924,7 @@ static long seccomp_get_action_avail(const char __user
> *uaction)
>
> /* Common entry point for both prctl and syscall. */
> static long do_seccomp(unsigned int op, unsigned int flags,
> -const char __user *uargs)
> +void __user *uargs)
> {
> switch (op) {
> case SECCOMP_SET_MODE_STRICT:
> @@ -944,7 +944,7 @@ static long do_seccomp(unsigned int op, unsigned int
> flags,
> }
>
> SYSCALL_DEFINE3(seccomp, unsigned int, op, unsigned int, flags,
> - const char __user *, uargs)
> + void __user *, uargs)
> {
> return do_seccomp(op, flags, uargs);
> }
> @@ -956,10 +956,10 @@ SYSCALL_DEFINE3(seccomp, unsigned int, op, unsigned
> int, flags,
> *
> * Returns 0 on success or -EINVAL on failure.
> */
> -long prctl_set_seccomp(unsigned long seccomp_mode, char __user *filter)
> +long prctl_set_seccomp(unsigned long seccomp_mode, void __user *filter)
> {
> unsigned int op;
> - char __user *uargs;
> + void __user *uargs;
>
> switch (seccomp_mode) {
> case SECCOMP_MODE_STRICT:
> --
> 2.19.1
Re: [PATCH v9 2/4] seccomp: switch system call argument type to void *
On Sun, Dec 02, 2018 at 08:28:25PM -0700, Tycho Andersen wrote:
> The const qualifier causes problems for any code that wants to write to the
> third argument of the seccomp syscall, as we will do in a future patch in
> this series.
>
> The third argument to the seccomp syscall is documented as void *, so
> rather than just dropping the const, let's switch everything to use void *
> as well.
>
> I believe this is safe because of 1. the documentation above, 2. there's no
> real type information exported about syscalls anywhere besides the man
> pages.
>
> Signed-off-by: Tycho Andersen
> CC: Kees Cook
> CC: Andy Lutomirski
> CC: Oleg Nesterov
> CC: Eric W. Biederman
> CC: "Serge E. Hallyn"
Acked-by: Serge Hallyn
Though I'm not entirely convinced there will be no ill effects of changing
the argument type. I'll feel comfortable when Michael and Paul say it's
fine :)
> CC: Christian Brauner
> CC: Tyler Hicks
> CC: Akihiro Suda
> ---
> include/linux/seccomp.h | 2 +-
> kernel/seccomp.c| 8
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/seccomp.h b/include/linux/seccomp.h
> index e5320f6c8654..b5103c019cf4 100644
> --- a/include/linux/seccomp.h
> +++ b/include/linux/seccomp.h
> @@ -43,7 +43,7 @@ extern void secure_computing_strict(int this_syscall);
> #endif
>
> extern long prctl_get_seccomp(void);
> -extern long prctl_set_seccomp(unsigned long, char __user *);
> +extern long prctl_set_seccomp(unsigned long, void __user *);
>
> static inline int seccomp_mode(struct seccomp *s)
> {
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index 96afc32e041d..393e029f778a 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -924,7 +924,7 @@ static long seccomp_get_action_avail(const char __user
> *uaction)
>
> /* Common entry point for both prctl and syscall. */
> static long do_seccomp(unsigned int op, unsigned int flags,
> -const char __user *uargs)
> +void __user *uargs)
> {
> switch (op) {
> case SECCOMP_SET_MODE_STRICT:
> @@ -944,7 +944,7 @@ static long do_seccomp(unsigned int op, unsigned int
> flags,
> }
>
> SYSCALL_DEFINE3(seccomp, unsigned int, op, unsigned int, flags,
> - const char __user *, uargs)
> + void __user *, uargs)
> {
> return do_seccomp(op, flags, uargs);
> }
> @@ -956,10 +956,10 @@ SYSCALL_DEFINE3(seccomp, unsigned int, op, unsigned
> int, flags,
> *
> * Returns 0 on success or -EINVAL on failure.
> */
> -long prctl_set_seccomp(unsigned long seccomp_mode, char __user *filter)
> +long prctl_set_seccomp(unsigned long seccomp_mode, void __user *filter)
> {
> unsigned int op;
> - char __user *uargs;
> + void __user *uargs;
>
> switch (seccomp_mode) {
> case SECCOMP_MODE_STRICT:
> --
> 2.19.1
