Re: [PATCH v2 1/2] fs: Improve and simplify copy_mount_options

Andy Lutomirski
2016-06-16T00:02:20Z

On Wed, Jun 15, 2016 at 4:50 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Mon, Jun 13, 2016 at 07:36:04PM -0700, Andy Lutomirski wrote:
>> copy_mount_options always tries to copy a full page even if the
>> string is shorter than a page. If the string starts part-way into a
>> page and ends on the same page it started on, this means that
>> copy_mount_options can overrun the supplied buffer and read into the
>> next page.
>
> Have you considered the possibility that there might be a reason for
> having separate copy_mount_option() and copy_mount_string()? Such as
> options not being a string, perhaps?
>
> In some filesystems (including older NFS variants) it is not a string
> at all - a binary data structure, with quite a few zero bytes in it.
> And no, we fucking *can't* break mount.nfs(8), no matter how we'd like
> to get rid of that wart of an ABI.
>
> IOW, NAK with prejudice - don't bring that thing back, it's hard no-go.

Well, that sucks. I suppose we could make it conditional on the fs
type being "nfs", but yuck.

If we don't fix this, though, then we have other problems:

devtmpfsd does:

*err = sys_mount("devtmpfs", "/", "devtmpfs", MS_SILENT, options);

where options points to the kernel stack. This is bad. do_mount_root
is similarly broken.

Is there any reason that these things use sys_mount instead of do_mount?

--Andy