Btrfs devel - Oct 2013 - [PATCH] Btrfs-progs: check return value of realpath(3)

I hit a segfault when deleting a subvolume with very long name(>4096),
it''s because cmd_subvol_delete() calls strdup() and passes NULL as
argument, which is returned by realpath(3).

I used the following script to reproduce

	#!/bin/bash
	mnt=$1
	i=1
	path=$mnt/subvol_$i

	# Create very deep subvolumes
	while btrfs sub create $path;do
	      ((i++))
	      path="$path/subvol_$i"
	done
	last_vol=$(dirname $path)
	dir=$(dirname $last_vol)
	vol=$(basename $last_vol)

	# Try to delete tha last one, this would get segfault
	pushd $dir
	btrfs sub delete $vol
	popd

Fix it by checking return value of realpath(3), also fix the one in
find_mount_root().

Signed-off-by: Eryu Guan <guaneryu@gmail.com>
---
 cmds-send.c      | 8 ++++++--
 cmds-subvolume.c | 6 ++++++
 2 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/cmds-send.c b/cmds-send.c
index 0057e6b..9e4d031 100644
--- a/cmds-send.c
+++ b/cmds-send.c
@@ -62,6 +62,7 @@ int find_mount_root(const char *path, char **mount_root)
 	int fd;
 	struct mntent *ent;
 	int len;
+	int ret;
 	int longest_matchlen = 0;
 	char *longest_match = NULL;
 
@@ -91,10 +92,13 @@ int find_mount_root(const char *path, char **mount_root)
 		return -ENOENT;
 	}
 
+	ret = 0;
 	*mount_root = realpath(longest_match, NULL);
-	free(longest_match);
+	if (!mount_root)
+		ret = -errno;
 
-	return 0;
+	free(longest_match);
+	return ret;
 }
 
 static int get_root_id(struct btrfs_send *s, const char *path, u64 *root_id)
diff --git a/cmds-subvolume.c b/cmds-subvolume.c
index ccb4762..f7249f8 100644
--- a/cmds-subvolume.c
+++ b/cmds-subvolume.c
@@ -225,6 +225,12 @@ again:
 	}
 
 	cpath = realpath(path, 0);
+	if (!cpath) {
+		ret = errno;
+		fprintf(stderr, "ERROR: finding real path for ''%s'':
%s\n",
+			path, strerror(errno));
+		goto out;
+	}
 	dname = strdup(cpath);
 	dname = dirname(dname);
 	vname = strdup(cpath);
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Sat, Oct 12, 2013 at 11:47:52PM +0800, Eryu Guan
wrote:
> I hit a segfault when deleting a subvolume with very long name(>4096),
How do you get a valid pathname longer than PATH_MAX which is 4096 ?
> Fix it by checking return value of realpath(3), also fix the one in
> find_mount_root().
The error handling itself is ok.

david
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

On Tue, Oct 15, 2013 at 06:49:41PM +0200, David Sterba
wrote:
> On Sat, Oct 12, 2013 at 11:47:52PM +0800, Eryu Guan wrote:
> > I hit a segfault when deleting a subvolume with very long
name(>4096),
> 
> How do you get a valid pathname longer than PATH_MAX which is 4096 ?
Just as the steps in reproducer, you can try the following

path=/mnt/btrfs
for i in `seq 1 381`;do
	path="$path/subvol_$i"
	btrfs sub create $path
done
echo ${#path}  # len is 4093 here

((i++))
path="$path/subvol_$i"
btrfs sub create $path
echo ${#path}  # The length of absolute path of this subvolume is greater than
4096 now

Maybe just another bug of btrfs-progs?
> 
> > Fix it by checking return value of realpath(3), also fix the one in
> > find_mount_root().
> 
> The error handling itself is ok.
Thanks for the review!

Eryu
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs"
in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html