statx: reject unknown flags when using NULL path

From: Eric Biggers <[email protected]>

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.

Arguably, we could still permit known lookup-related flags such as AT_SYMLINK_NOFOLLOW. However, that would be inconsistent with how sys_utimensat() behaves when passed a NULL path, which seems to be the closest precedent. And given that the NULL path case is (I believe) mainly intended to be used to implement a wrapper function like fstatx() that doesn't have a path argument, I think rejecting lookup-related flags too is probably the best choice.

---
 fs/stat.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

index fa0be59340cc..df484a60846d 100644
@@ -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);

2.12.0

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

Looks good to me.

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..

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

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 :)

I will be pushing this sort of thing to LTP.

David