Re: [PATCH 27/29] xfs: clean up the attr flag confusion

Darrick J. Wong
2020-01-26T22:27:08Z

On Sat, Jan 25, 2020 at 03:10:47PM -0800, Christoph Hellwig wrote:
> On Fri, Jan 24, 2020 at 03:24:13PM -0800, Christoph Hellwig wrote:
> > > > + u32 ioc_flags)
> > > > +{
> > > > + unsigned int namespace = 0;
> > > > +
> > > > + if (ioc_flags & XFS_IOC_ATTR_ROOT)
> > > > + namespace |= XFS_ATTR_ROOT;
> > > > + if (ioc_flags & XFS_IOC_ATTR_SECURE)
> > > > + namespace |= XFS_ATTR_SECURE;
> > >
> > > Seeing as these are mutually exclusive options, I'm a little surprised
> > > there isn't more checking that both of these flags aren't set at the
> > > same time.
> > >
> > > (Or I've been reading this series too long and missed that it does...)
> >
> > XFS never rejected the combination. It just won't find anything in that
> > case. Let me see if I could throw in another patch to add more checks
> > there.
>
> So for the get/set ioctl this was all fixed by
>
> "xfs: reject invalid flags combinations in XFS_IOC_ATTRMULTI_BY_HANDLE"
>
> for listattr it is rather harmless, but I can throw in a patch to
> explicitly reject it.

I think that's a good idea.

--D