On 8/22/19 2:39 PM, Eric Sandeen wrote:
> Once upon a time, xfsdump pointed at a bind-mounted subdirectory
> caused problems because we assumed the inode of the bind-mounted
> dir was the root inode of the filesystem. That got written into the
> dump header, and a subsequent restore would fail on the mismatch.
>
> An effort was made to determine the true root inode using bulkstat,
> to retrieve the first (lowest-numbered) inode in the filesystem.
>
> That approach is now known to fail in the (rare) case where inodes
> are allocated with a lower number than the root. As a result, thewrong root inode number still goes into the dump header, and restore
> will fail with:
>
> xfsrestore: tree.c:757: tree_begindir: Assertion `ino != persp->p_rootino || hardh == persp->p_rooth' failed.
>
> in this case as well.
>
> Rather than trying to work out the true root inode for the filesystem
> in question if it's bind mounted, just go the simple route and reject
> bind mounts of subdirs altogether. This probably leads to less surprise
> in any case, as dumping a bind-mounted subdir actually will dump the
> entire filesystem AFAICT.
>
> Fixes: 25195ebf107 ("xfsdump: handle bind mount targets")
> Signed-off-by: Eric Sandeen <sandeen@xxxxxxxxxx>
> ---
>
> diff --git a/dump/content.c b/dump/content.c
> index 30232d4..b9f4929 100644
> --- a/dump/content.c
> +++ b/dump/content.c
> @@ -1383,12 +1383,12 @@ baseuuidbypass:
>
> /* figure out the ino for the root directory of the fs
> * and get its struct xfs_bstat for inomap_build(). This could
> - * be a bind mount; don't ask for the mount point inode,
> - * find the actual lowest inode number in the filesystem.
> + * be a bind mount; disallow this case because we don't know the
> + * root inode.
> */
> {
> stat64_t rootstat;
> - xfs_ino_t lastino = 0;
> + xfs_ino_t lastino;
> int ocount = 0;
> struct xfs_fsop_bulkreq bulkreq;
>
> @@ -1404,7 +1404,8 @@ baseuuidbypass:
> (struct xfs_bstat *)calloc(1, sizeof(struct xfs_bstat));
> assert(sc_rootxfsstatp);
>
> - /* Get the first valid (i.e. root) inode in this fs */
> + /* Bulkstat this inode to get generation number */
> + lastino = rootstat.st_ino - 1;
> bulkreq.lastip = (__u64 *)&lastino;
> bulkreq.icount = 1;
> bulkreq.ubuffer = sc_rootxfsstatp;
actually, I forgot we have bigstat_one() which wraps this up, I'll use that
instead.
> @@ -1415,10 +1416,13 @@ baseuuidbypass:
> return BOOL_FALSE;
> }
>
> - if (sc_rootxfsstatp->bs_ino != rootstat.st_ino)
> - mlog (MLOG_NORMAL | MLOG_NOTE,
> - _("root ino %lld differs from mount dir ino %lld, bind mount?\n"),
> - sc_rootxfsstatp->bs_ino, rootstat.st_ino);
> + /* The real root inode will have a generation of zero */
> + if (sc_rootxfsstatp->bs_gen != 0) {
> + mlog (MLOG_ERROR,
> +_("Dir inode %lld at %s has non-zero generation, not a root inode, bind mount?\n"),
> + rootstat.st_ino, mntpnt);
> + return BOOL_FALSE;
> + }
> }
>
> /* alloc a file system handle, to be used with the jdm_open()
>
