[git commit] lsattr,chattr: do not open e.g. device files unless asked directly; do not follow links

Denys Vlasenko vda.linux at googlemail.com
Wed Jun 23 23:07:56 UTC 2021


commit: https://git.busybox.net/busybox/commit/?id=9468ea06d2445f774dd923fdfdea04209984fbf4
branch: https://git.busybox.net/busybox/commit/?id=refs/heads/master

Add O_NOFOLLOW (and O_NOCTTY for good measure) to open calls like e2fsprogs does.

In lsattr, when recursing, operate only on regular files, symlinks, and directories.
(Otherwise, "lsattr /dev" can e.g. open a watchdog device... not good).

At this time, looks like chattr/lsattr can't operate on symlink inodes -
ioctls do not work on open(O_PATH | O_NOFOLLOW) fds.

function                                             old     new   delta
lsattr_dir_proc                                      168     203     +35
change_attributes                                    410     408      -2
------------------------------------------------------------------------------
(add/remove: 0/0 grow/shrink: 1/1 up/down: 35/-2)              Total: 33 bytes

Signed-off-by: Denys Vlasenko <vda.linux at googlemail.com>
---
 e2fsprogs/chattr.c |  9 ++++++---
 e2fsprogs/lsattr.c | 15 ++++++++++++---
 2 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/e2fsprogs/chattr.c b/e2fsprogs/chattr.c
index e599abae7..f436cd91e 100644
--- a/e2fsprogs/chattr.c
+++ b/e2fsprogs/chattr.c
@@ -162,8 +162,6 @@ static void change_attributes(const char *name, struct globals *gp)
 		bb_perror_msg("can't stat '%s'", name);
 		return;
 	}
-	if (S_ISLNK(st.st_mode) && gp->recursive)
-		return;
 
 	/* Don't try to open device files, fifos etc.  We probably
 	 * ought to display an error if the file was explicitly given
@@ -172,7 +170,12 @@ static void change_attributes(const char *name, struct globals *gp)
 	if (!S_ISREG(st.st_mode) && !S_ISLNK(st.st_mode) && !S_ISDIR(st.st_mode))
 		return;
 
-	fd = open_or_warn(name, O_RDONLY | O_NONBLOCK); /* neither read nor write asked for */
+	/* There is no way to run needed ioctls on a symlink.
+	 * open(O_PATH | O_NOFOLLOW) _can_ be used to get a fd referring to the symlink,
+	 * but ioctls fail on such a fd (tried on 4.12.0 kernel).
+	 * e2fsprogs-1.46.2 uses open(O_NOFOLLOW), it fails on symlinks.
+	 */
+	fd = open_or_warn(name, O_RDONLY | O_NONBLOCK | O_NOCTTY | O_NOFOLLOW);
 	if (fd >= 0) {
 		int r;
 
diff --git a/e2fsprogs/lsattr.c b/e2fsprogs/lsattr.c
index b4ed12d50..545afa7f5 100644
--- a/e2fsprogs/lsattr.c
+++ b/e2fsprogs/lsattr.c
@@ -49,8 +49,13 @@ static void list_attributes(const char *name)
 	unsigned fsflags;
 	int fd, r;
 
-	fd = open_or_warn(name, O_RDONLY | O_NONBLOCK); /* neither read nor write asked for */
-	if (fd < 0) /* for example, dangling links */
+	/* There is no way to run needed ioctls on a symlink.
+	 * open(O_PATH | O_NOFOLLOW) _can_ be used to get a fd referring to the symlink,
+	 * but ioctls fail on such a fd (tried on 4.12.0 kernel).
+	 * e2fsprogs-1.46.2 uses open(O_NOFOLLOW), it fails on symlinks.
+	 */
+	fd = open_or_warn(name, O_RDONLY | O_NONBLOCK | O_NOCTTY | O_NOFOLLOW);
+	if (fd < 0)
 		return;
 
 	if (option_mask32 & OPT_PROJID) {
@@ -102,8 +107,12 @@ static int FAST_FUNC lsattr_dir_proc(const char *dir_name,
 
 	if (lstat(path, &st) != 0)
 		bb_perror_msg("can't stat '%s'", path);
+
 	else if (de->d_name[0] != '.' || (option_mask32 & OPT_ALL)) {
-		list_attributes(path);
+	        /* Don't try to open device files, fifos etc */
+		if (S_ISREG(st.st_mode) || S_ISLNK(st.st_mode) || S_ISDIR(st.st_mode))
+			list_attributes(path);
+
 		if (S_ISDIR(st.st_mode) && (option_mask32 & OPT_RECUR)
 		 && !DOT_OR_DOTDOT(de->d_name)
 		) {


More information about the busybox-cvs mailing list