Messages in this thread | | | Date | Mon, 12 Aug 2019 19:05:01 +0200 | From | Christian Brauner <> | Subject | Re: [PATCH v5 1/2] fork: extend clone3() to support CLONE_SET_TID |
| |
On Mon, Aug 12, 2019 at 06:57:34PM +0200, Oleg Nesterov wrote: > On 08/12, Christian Brauner wrote: > > On Mon, Aug 12, 2019 at 06:37:10PM +0200, Oleg Nesterov wrote: > > > On 08/11, Adrian Reber wrote: > > > > > > > > include/linux/pid.h | 2 +- > > > > include/linux/sched/task.h | 1 + > > > > include/uapi/linux/sched.h | 1 + > > > > kernel/fork.c | 22 ++++++++++++++++++++-- > > > > kernel/pid.c | 36 +++++++++++++++++++++++++++++------- > > > > 5 files changed, 52 insertions(+), 10 deletions(-) > > > > > > Looks good to me... > > > > > > A couple of nits below, but I won't insist, feel free to ignore. > > > > > > > +/* > > > > + * Different sizes of struct clone_args > > > > + */ > > > > +#define CLONE3_ARGS_SIZE_V0 64 > > > > > > I don't really understand why do we want the "size < CLONE3_ARGS_SIZE_V0" > > > check in copy_clone_args_from_user(), but I won't argue. > > > > To make sure a user can't give us a garbage sized struct that is smaller > > than the initial version of the struct. > > But why do we want to detect this case?
I have to admit I don't understand that question. Because it's a garbage sized struct that we can't do anything with. Why shouldn't we detect this case?
> > And why CLONE3_ARGS_SIZE_V0 is special?
Hm? Because that is the first-well known struct size. In fact this pattern is also used for perf and sched:
if (!access_ok(uattr, PERF_ATTR_SIZE_VER0)) return -EFAULT;
| |