Re: [PATCH 00/17] VFS: Filesystem information and notifications [ver #17]

[Greg Kroah-Hartman]

On Tue, Mar 03, 2020 at 02:34:42PM +0100, Miklos Szeredi wrote:
> On Tue, Mar 3, 2020 at 2:14 PM Greg Kroah-Hartman
> <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> 
> > > Unlimited beers for a 21-line kernel patch?  Sign me up!
> > >
> > > Totally untested, barely compiled patch below.
> >
> > Ok, that didn't even build, let me try this for real now...
> 
> Some comments on the interface:

Ok, hey, let's do this proper :)

> O_LARGEFILE can be unconditional, since offsets are not exposed to the caller.

Good point.

> Use the openat2 style arguments; limit the accepted flags to sane ones
> (e.g. don't let this syscall create a file).

Yeah, I just added that check to my local version:
	/* Mask off all O_ flags as we only want to read from the file */
	flags &= ~(VALID_OPEN_FLAGS);
	flags |= O_RDONLY | O_LARGEFILE;

> If buffer is too small to fit the whole file, return error.

Why?  What's wrong with just returning the bytes asked for?  If someone
only wants 5 bytes from the front of a file, it should be fine to give
that to them, right?

> Verify that the number of bytes read matches the file size, otherwise
> return error (may need to loop?).

No, we can't "match file size" as sysfs files do not really have a sane
"size".  So I don't want to loop at all here, one-shot, that's all you
get :)

Let me actually do this and try it out for real.

/me has no idea what he is getting himself into...

thanks,

greg k-h