| Message ID | 20170311065823.4415-1-ebiggers3@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Mar 10, 2017 at 10:58:23PM -0800, Eric Biggers wrote: > From: Eric Biggers <ebiggers@google.com> > > The statx() system call currently accepts unknown flags when called with > a NULL path to operate on a file descriptor. Left unchanged, this could > make it hard to introduce new query flags in the future, since > applications may not be able to tell whether a given flag is supported. > > Fix this by failing the system call with EINVAL if any flags other than > KSTAT_QUERY_FLAGS are specified in combination with a NULL path. > Does anyone have comments on this patch? I really think we need to get this in before v4.11 is released, since it deals with the API. - Eric
On Mon, Mar 20, 2017 at 10:11:42AM -0700, Eric Biggers wrote: > On Fri, Mar 10, 2017 at 10:58:23PM -0800, Eric Biggers wrote: > > From: Eric Biggers <ebiggers@google.com> > > > > The statx() system call currently accepts unknown flags when called with > > a NULL path to operate on a file descriptor. Left unchanged, this could > > make it hard to introduce new query flags in the future, since > > applications may not be able to tell whether a given flag is supported. > > > > Fix this by failing the system call with EINVAL if any flags other than > > KSTAT_QUERY_FLAGS are specified in combination with a NULL path. > > > > Does anyone have comments on this patch? Looks good to me. > I really think we need to get this in before v4.11 is released, since it deals > with the API. Yes. And we really need test for all of this. Or just revert the patches. Shipping with untested syscalls is just a desaster.
Al, Linus: can we get this in please? It would be sad to grow another syscall with an unchecked flags argument..
On Fri, Mar 31, 2017 at 08:39:52AM -0700, Christoph Hellwig wrote: > Al, Linus: > > can we get this in please? It would be sad to grow another syscall > with an unchecked flags argument.. Applied, will be in tonight pull request...
Al Viro <viro@ZenIV.linux.org.uk> wrote: > > can we get this in please? It would be sad to grow another syscall > > with an unchecked flags argument.. > > Applied, will be in tonight pull request... I'm not convinced that this is right. I'm more inclined to let any flag be passed that is available to statx() with a filename and just mask off the bits before handing them on. David
On Fri, Mar 31, 2017 at 04:51:24PM +0100, David Howells wrote: > I'm not convinced that this is right. I'm more inclined to let any flag be > passed that is available to statx() with a filename and just mask off the > bits before handing them on. And what would these flags actually do? Currently we verify that the allowed flags are passed for the case where we have a filename. We need to do the same for the non-filename case, and we should also check that we don't pass flags that don't make sense for this case. Eric's patch is doing exactly that. The only thing he could do even better is to add a testcase to xfstests to verify this :)
Christoph Hellwig <hch@infradead.org> wrote: > The only thing he could do even better is to add a testcase to xfstests > to verify this :) I will be pushing this sort of thing to LTP. David
diff --git a/fs/stat.c b/fs/stat.c index fa0be59340cc..df484a60846d 100644 --- a/fs/stat.c +++ b/fs/stat.c @@ -130,9 +130,13 @@ EXPORT_SYMBOL(vfs_getattr); int vfs_statx_fd(unsigned int fd, struct kstat *stat, u32 request_mask, unsigned int query_flags) { - struct fd f = fdget_raw(fd); + struct fd f; int error = -EBADF; + if (query_flags & ~KSTAT_QUERY_FLAGS) + return -EINVAL; + + f = fdget_raw(fd); if (f.file) { error = vfs_getattr(&f.file->f_path, stat, request_mask, query_flags);