On Sat, Feb 23, 2019 at 12:03 AM Dave Hansen <dave.hansen@xxxxxxxxx> wrote: > > On 2/22/19 4:53 AM, Andrey Konovalov wrote: > > --- a/fs/namespace.c > > +++ b/fs/namespace.c > > @@ -2730,7 +2730,7 @@ void *copy_mount_options(const void __user * data) > > * the remainder of the page. > > */ > > /* copy_from_user cannot cross TASK_SIZE ! */ > > - size = TASK_SIZE - (unsigned long)data; > > + size = TASK_SIZE - (unsigned long)untagged_addr(data); > > if (size > PAGE_SIZE) > > size = PAGE_SIZE; > > I would have thought that copy_from_user() *is* entirely capable of > detecting and returning an error in the case that its arguments cross > TASK_SIZE. It will fail and return an error, but that's what it's > supposed to do. > > I'd question why this code needs to be doing its own checking in the > first place. Is there something subtle going on? The comment above exact_copy_from_user() states: Some copy_from_user() implementations do not return the exact number of bytes remaining to copy on a fault. But copy_mount_options() requires that. Note that this function differs from copy_from_user() in that it will oops on bad values of `to', rather than returning a short copy.
