Re: Mount options may be silently discarded
On Mon, Sep 28, 2020 at 09:00:54PM +0300, Dmitry Kasatkin wrote: > But why "we" should allow "discarding" failed part instead of failing > with EFAULT as a whole? Because there might very well be absolutely legitimate users of mount(2) passing it something smaller than 4Kb immediately followed by an unmapped area. What can mount(2) do? It can't go up to the first \0 and stop there, thanks to filesystems (NFS) that want to get struct some_shite filled by userland. It can't require the entire 4Kb from the pointer passed to mount(2) to be mapped and readable, simply because passing it a string literal for e.g. ext4 mount can violate that requirement, not to mention the result of strdup(3)/asprintf(3)/etc. And it can't even tell which semantics to use by looking at the filesystem type - NFS allows both the string and binary structure for options.
Re: Mount options may be silently discarded
On Mon, Sep 28, 2020 at 5:36 PM David Laight wrote:
>
> From: Dmitry Kasatkin
> > Sent: 28 September 2020 15:03
> >
> > "copy_mount_options" function came to my eyes.
> > It splits copy into 2 pieces - over page boundaries.
> > I wonder what is the real reason for doing this?
> > Original comment was that we need exact bytes and some user memcpy
> > functions do not return correct number on page fault.
> >
> > But how would all other cases work?
> >
> > https://elixir.bootlin.com/linux/latest/source/fs/namespace.c#L3075
> >
> > if (size != PAGE_SIZE) {
> >if (copy_from_user(copy + size, data + size, PAGE_SIZE - size))
> > memset(copy + size, 0, PAGE_SIZE - size);
> > }
> >
> > This looks like some options may be just discarded?
> > What if it is an important security option?
> >
> > Why it does not return EFAULT, but just memset?
>
> The user doesn't supply the transfer length, the max size
> is a page.
> Since the copy can only start to fail on a page boundary
> reading in two pieces is exactly the same as knowing the
> address at which the transfer started to fail.
>
> Since the actual mount options can be much smaller than
> a page (and usually are) zero-filling is best.
>
Hi David,
Ok. This is now obvious that it is done for "proper" memseting...
But why "we" should allow "discarding" failed part instead of failing
with EFAULT as a whole?
Thanks,
> David
>
> -
> Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1
> 1PT, UK
> Registration No: 1397386 (Wales)
--
Thanks,
Dmitry
RE: Mount options may be silently discarded
From: Dmitry Kasatkin
> Sent: 28 September 2020 15:03
>
> "copy_mount_options" function came to my eyes.
> It splits copy into 2 pieces - over page boundaries.
> I wonder what is the real reason for doing this?
> Original comment was that we need exact bytes and some user memcpy
> functions do not return correct number on page fault.
>
> But how would all other cases work?
>
> https://elixir.bootlin.com/linux/latest/source/fs/namespace.c#L3075
>
> if (size != PAGE_SIZE) {
>if (copy_from_user(copy + size, data + size, PAGE_SIZE - size))
> memset(copy + size, 0, PAGE_SIZE - size);
> }
>
> This looks like some options may be just discarded?
> What if it is an important security option?
>
> Why it does not return EFAULT, but just memset?
The user doesn't supply the transfer length, the max size
is a page.
Since the copy can only start to fail on a page boundary
reading in two pieces is exactly the same as knowing the
address at which the transfer started to fail.
Since the actual mount options can be much smaller than
a page (and usually are) zero-filling is best.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
Mount options may be silently discarded
Hi,
"copy_mount_options" function came to my eyes.
It splits copy into 2 pieces - over page boundaries.
I wonder what is the real reason for doing this?
Original comment was that we need exact bytes and some user memcpy
functions do not return correct number on page fault.
But how would all other cases work?
https://elixir.bootlin.com/linux/latest/source/fs/namespace.c#L3075
if (size != PAGE_SIZE) {
if (copy_from_user(copy + size, data + size, PAGE_SIZE - size))
memset(copy + size, 0, PAGE_SIZE - size);
}
This looks like some options may be just discarded?
What if it is an important security option?
Why it does not return EFAULT, but just memset?
--
Thanks,
Dmitry
