Re: [PATCH] xfs: correct statx's result_mask value

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 1/8/2019 1:07 PM, Darrick J. Wong wrote:
On Tue, Jan 08, 2019 at 12:58:43PM +0800, Su Yanjun <suyj.fnst@xxxxxxxxxxxxxx> wrote:

On 1/8/2019 2:04 AM, Eric Sandeen wrote:
On 1/7/19 11:52 AM, Darrick J. Wong wrote:
On Mon, Jan 07, 2019 at 04:53:10AM -0500, Su Yanjun wrote:
For statx syscall, xfs return the wrong result_mask.

Signed-off-by: Su Yanjun<suyj.fnst@xxxxxxxxxxxxxx>
---
   fs/xfs/xfs_iops.c | 3 +++
   1 file changed, 3 insertions(+)

diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c
index f48ffd7..3811457 100644
--- a/fs/xfs/xfs_iops.c
+++ b/fs/xfs/xfs_iops.c
@@ -521,6 +521,9 @@ xfs_vn_getattr(
   			stat->btime.tv_nsec = ip->i_d.di_crtime.t_nsec;
   		}
   	}
+	
+	/* Only return mask that we care */
+	stat->result_mask &= request_mask;
Why not just:

	stat->result_mask = STATX_BASIC_STATS;

at the top of the function?

I don't see the need to mask off result_mask at all, since we could some
day elect to return more than what's in request_mask...
When we run xfstests with nfs, the generic/423 case runs failed. So i review
the nfs'
nfs_getattr code it does validate the request_mask.

Then i review the xfs' getattr code, it has no such check. Whatever
request_mask
  is set, the stat's result_mask always the 0x7ff.
Yes, statx can return more data than what userspace callers ask for:

Maybe it has Unclear semantics about statx's result_mask.
"A filesystem may also fill in fields that the caller didn't ask for if
it has values for them available and the information is available at no
extra cost.  If this happens, the  corresponding  bits will be set in
stx_mask."

--D

I get it, then the testcase generic/423 may need update in xfstests.
Thanks for your reply.

...waitaminute, are you seeing garbage in the result_mask that's
returned to userspace?  I also noticed the vfs stat functions declare
"struct kstat stat;" without explicitly zeroing the structure fields,
which means (I think) that we can leak stack information if the kernel
isn't built with the stackleak plugin?
No such problem.
A clear problem statement and reproducer steps would be hugely useful
here.

-Eric
Thanks,
Su









[Index of Archives]     [XFS Filesystem Development (older mail)]     [Linux Filesystem Development]     [Linux Audio Users]     [Yosemite Trails]     [Linux Kernel]     [Linux RAID]     [Linux SCSI]


  Powered by Linux