Re: [PATCH] xfsprogs: be careful about what we stat in platform_check_mount

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Wed, May 02, 2018 at 02:56:22PM -0500, Eric Sandeen wrote:
> After we lost ustat(2) in commit 4e7a824, we ended up with a slightly
> bonkers method to determine if our target block device was mounted:
> it goes through every entry returned by getmntent and stats the dir
> to see if its underlying device matches ours.
> 
> Unfortunately that dir might be a hung nfs server and sadness ensues.
> 
> So do some pre-checks; can we stat the mounted "device?"  If so is
> it really a block device?  If not, skip it.
> 
> Fixes: 4e7a824 ("libxfs/linux.c: Replace use of ustat by stat")
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>

Looks ok,
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> ---
> 
> diff --git a/libxfs/linux.c b/libxfs/linux.c
> index 0bace3e..7350fa7 100644
> --- a/libxfs/linux.c
> +++ b/libxfs/linux.c
> @@ -77,7 +77,21 @@ platform_check_mount(char *name, char *block, struct stat *s, int flags)
>  		    progname, name);
>  		return 1;
>  	}
> +	/*
> +	 * This whole business is to work out if our block device is mounted
> +	 * after we lost ustat(2), see:
> +	 * 	4e7a824 libxfs/linux.c: Replace use of ustat by stat
> +	 * We don't really want to stat every single mounted directory,
> +	 * as that may include tmpfs, cgroups, procfs or - worst - hung nfs
> +	 * servers.  So we check the "device" before going after the mountpoint.
> +	 */
>  	while ((mnt = getmntent(f)) != NULL) {
> +		/* If the "fsname" is't a stat-able device, skip it */
> +		if (stat(mnt->mnt_fsname, &mst) < 0)
> +			continue;
> +		if (!S_ISBLK(mst.st_mode))
> +			continue;
> +		/* Ok, this entry does seem to be a mounted block device */
>  		if (stat(mnt->mnt_dir, &mst) < 0)
>  			continue;
>  		if (mst.st_dev != s->st_rdev)
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html