Skip to contentSkip to conversation

Fix wrong memory free on check_is_root

PATCH In review Satoru Takeuchi 2 participants 2 messages Dec 18, 2014 Btrfs file system

lore
Share & tools
lore
#0 lore raw

PATCH v2 1/1 Fix wrong memory free on check_is_root

From: Satoru Takeuchi <[email protected]>

When "/" is Btrfs, "btrfs property <subcommand> /" regards it as non-root by mistake.

check_is_root() regards @object as a file system root if the following two conditions are satisfied.

a) Both @object and its parent directory are Btrfs object (file system root, subvolume, inode, and device used for Btrfs).
b) fsid of the above mentioned two objects are different.

It doesn't work if @object is "/" because, in this case, fsid of "/" and its parent (it's also "/"), are the same.

* Test environment

Two Btrfs file system (not subvolume) "/" and "/home/sat/mnt".

* How to reproduce

Submit "btrfs prop get" against the above mentioned file systems.

* Test Result

** Actual result (without my patch)

==========================

# btrfs prop get /home/sat/mnt/
ro=false
label=                 # label is displayed because it's a file system root
# btrfs prop get /
ro=false               # label is not displayed even if it's a file system root
==========================
** Expected result (with my patch)

==========================

# ./btrfs-new prop get btrfs-auto-test/
# ./btrfs-new prop get /home/sat/mnt/
ro=false
label=
# ./btrfs-new prop get /             
ro=false
label=foo            # label is displayed
===========================

---
changelog
v2: Return a correct error code when realpath() fails.
Thank you Gui Hecheng for pointing my mistake out.
https://www.mail-archive.com/[email protected]/msg40204.html

---
 cmds-property.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)
cmds-property.c +13 -1
index a764293..6501338 100644
@@ -124,7 +124,18 @@ static int check_is_root(const char *object)
 	int ret;
 	u8 fsid[BTRFS_FSID_SIZE];
 	u8 fsid2[BTRFS_FSID_SIZE];
-	char *tmp;
+	char *tmp = NULL;
+	char *rp;
+
+	rp = realpath(object, NULL);
+	if (!rp) {
+		ret = -errno;
+		goto out;
+	}
+	if (!strcmp(rp, "/")) {
+		ret = 0;
+		goto out;
+	}
 
 	tmp = malloc(strlen(object) + 5);
 	if (!tmp) {
@@ -165,6 +176,7 @@ static int check_is_root(const char *object)
 
 out:
 	free(tmp);
+	free(rp);
 	return ret;
 }
 
Signature · 1 line
1.9.3
Reviewed-by on v2 1/1
#1 lore raw
Quoted 47 lines · Satoru Takeuchi

From: Satoru Takeuchi <[email protected]>

When "/" is Btrfs, "btrfs property <subcommand> /" regards it as non-root by mistake.

check_is_root() regards @object as a file system root if the following two conditions are satisfied.

a) Both @object and its parent directory are Btrfs object (file system root, subvolume, inode, and device used for Btrfs).
b) fsid of the above mentioned two objects are different.

It doesn't work if @object is "/" because, in this case, fsid of "/" and its parent (it's also "/"), are the same.

* Test environment

Two Btrfs file system (not subvolume) "/" and "/home/sat/mnt".

* How to reproduce

Submit "btrfs prop get" against the above mentioned file systems.

* Test Result

** Actual result (without my patch)

==========================

# btrfs prop get /home/sat/mnt/
ro=false
label=                 # label is displayed because it's a file system root
# btrfs prop get /
ro=false               # label is not displayed even if it's a file system root
==========================
** Expected result (with my patch)

==========================

# ./btrfs-new prop get btrfs-auto-test/
# ./btrfs-new prop get /home/sat/mnt/
ro=false
label=
# ./btrfs-new prop get /             
ro=false
label=foo            # label is displayed
===========================
Quoted 2 lines

Looks good to me.

Thanks,
Gui

Quoted 41 lines

---
changelog
v2: Return a correct error code when realpath() fails.
Thank you Gui Hecheng for pointing my mistake out.
https://www.mail-archive.com/[email protected]/msg40204.html

---
 cmds-property.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)
cmds-property.c +13 -1
index a764293..6501338 100644
@@ -124,7 +124,18 @@ static int check_is_root(const char *object)
 	int ret;
 	u8 fsid[BTRFS_FSID_SIZE];
 	u8 fsid2[BTRFS_FSID_SIZE];
-	char *tmp;
+	char *tmp = NULL;
+	char *rp;
+
+	rp = realpath(object, NULL);
+	if (!rp) {
+		ret = -errno;
+		goto out;
+	}
+	if (!strcmp(rp, "/")) {
+		ret = 0;
+		goto out;
+	}
 
 	tmp = malloc(strlen(object) + 5);
 	if (!tmp) {
@@ -165,6 +176,7 @@ static int check_is_root(const char *object)
 
 out:
 	free(tmp);
+	free(rp);
 	return ret;
 }