On Tue, May 16, 2017 at 10:54:39AM -0700, Darrick J. Wong wrote:
> On Fri, May 12, 2017 at 10:53:57AM +0200, Carlos Maiolino wrote:
> > Hi,
> >
> > > There were a number of handwaving complaints that one could "possibly"
> > > use inode numbers and extent maps to fingerprint a filesystem hosting
> > > multiple containers and somehow use the information to guess at the
> > > contents of other containers and attack them. Despite the total lack of
> > > any demonstration that this is actually possible, it's easier to
> > > restrict access now and broaden it later, so use the rmapbt fsmap
> > > backends only if the caller has CAP_SYS_ADMIN. Unprivileged users will
> > > just have to make do with only getting the free space information.
> > >
> > > Signed-off-by: Darrick J. Wong <darrick.wong@xxxxxxxxxx>
> > > ---
> > > fs/xfs/xfs_fsmap.c | 5 ++++-
> > > 1 file changed, 4 insertions(+), 1 deletion(-)
> > >
> > > diff --git a/fs/xfs/xfs_fsmap.c b/fs/xfs/xfs_fsmap.c
> > > index 3683819..814ed729 100644
> > > --- a/fs/xfs/xfs_fsmap.c
> > > +++ b/fs/xfs/xfs_fsmap.c
> > > @@ -828,6 +828,7 @@ xfs_getfsmap(
> > > struct xfs_fsmap dkeys[2]; /* per-dev keys */
> > > struct xfs_getfsmap_dev handlers[XFS_GETFSMAP_DEVS];
> > > struct xfs_getfsmap_info info = { NULL };
> > > + bool use_rmap;
> > > int i;
> > > int error = 0;
> > >
> > > @@ -837,12 +838,14 @@ xfs_getfsmap(
> > > !xfs_getfsmap_is_valid_device(mp, &head->fmh_keys[1]))
> > > return -EINVAL;
> > >
> > > + use_rmap = capable(CAP_SYS_ADMIN) &&
> > > + xfs_sb_version_hasrmapbt(&mp->m_sb);
> > > head->fmh_entries = 0;
> > >
> > > /* Set up our device handlers. */
> > > memset(handlers, 0, sizeof(handlers));
> > > handlers[0].dev = new_encode_dev(mp->m_ddev_targp->bt_dev);
> > > - if (xfs_sb_version_hasrmapbt(&mp->m_sb))
> > > + if (use_rmap)
> > > handlers[0].fn = xfs_getfsmap_datadev_rmapbt;
> > > else
> > > handlers[0].fn = xfs_getfsmap_datadev_bnobt;
> > >
> >
> > I've followed the discussion too, and alhtough it just look as a very remote
> > theoretical problem, restricting this is simple.
> >
> > Overall this looks fine to me, but, I just wonder if somebody in the future will
> > complain to be getting freespace only, instead of the fsmap itself, without
> > actually getting a permission denied error, or something like that.
> >
> > This is a minor concern from my side though, so, unless somebody agrees with my
> > point here, you can add:
>
> I think I'll just add a blurb to the manpage explicitly stating that
> filesystems have discretion to reveal as much or as little detail as
> they like. Therefore, it's perfectly fine not to reveal inode numbers,
> which is what you'd get with a pre-rmap xfs (or ext4) today.
>
This sounds perfectly fine to me, you can keep my review tag.
Cheers
> --D
>
> >
> > Reviewed-by: Carlos Maiolino <cmaiolino@xxxxxxxxxx>
> >
> > Cheers.
> >
> > --
> > > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > > More majordomo info at http://vger.kernel.org/majordomo-info.html
> >
> > --
> > Carlos
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> > the body of a message to majordomo@xxxxxxxxxxxxxxx
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@xxxxxxxxxxxxxxx
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Carlos
--
To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
