Messages in this thread |  | | Date | Wed, 8 Jul 2020 23:12:02 -0700 | From | Kees Cook <> | Subject | Re: [PATCH v6 6/7] seccomp: Introduce addfd ioctl to seccomp user notifier |
| |
On Tue, Jul 07, 2020 at 03:30:49PM +0200, Christian Brauner wrote: > Hm, maybe change that description to sm like: > > [...]
Cool, yeah. Thanks! I've tweaked it a little more
> > + /* 24 is original sizeof(struct seccomp_notif_addfd) */ > > + if (size < 24 || size >= PAGE_SIZE) > > + return -EINVAL; > > Hm, so maybe add the following: > > #define SECCOMP_NOTIFY_ADDFD_VER0 24 > #define SECCOMP_NOTIFY_ADDFD_LATEST SECCOMP_NOTIFY_ADDFD_VER0 > > and then place: > > BUILD_BUG_ON(sizeof(struct seccomp_notify_addfd) < SECCOMP_NOTIFY_ADDFD_VER0); > BUILD_BUG_ON(sizeof(struct open_how) != SECCOMP_NOTIFY_ADDFD_LATEST);
Yes, good idea (BTW, did the EA syscall docs land?)
I've made these SECCOMP_NOTIFY_ADDFD_SIZE_* to match your examples below (i.e. I added "SIZE" to what you suggested above).
> somewhere which is what we do for clone3(), openat2() and others to > catch build-time nonsense. > > include/uapi/linux/perf_event.h:#define PERF_ATTR_SIZE_VER0 64 /* sizeof first published struct */ > include/uapi/linux/sched.h:#define CLONE_ARGS_SIZE_VER0 64 /* sizeof first published struct */ > include/uapi/linux/sched/types.h:#define SCHED_ATTR_SIZE_VER0 48 /* sizeof first published struct */ > include/linux/fcntl.h:#define OPEN_HOW_SIZE_VER0 24 /* sizeof first published struct */ > include/linux/fcntl.h:#define OPEN_HOW_SIZE_LATEST OPEN_HOW_SIZE_VER0
The ..._SIZE_VER0 and ...LATEST stuff doesn't seem useful to export via UAPI. Above, 2 of the 3 export to uapi. Is there a specific rationale for which should and which shouldn't?
> > +#undef EA_IOCTL > > Why is this undefed? :)
It was defined "in" a function, so I like to mimic function visibility. But you're right; there's no reason to undef it.
-- Kees Cook
|  |