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