[git commit] e2fsprogs/*: remove ioctl calling obfuscation

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


commit: https://git.busybox.net/busybox/commit/?id=96436fb36a5fa0ac8e993fb093b4788fb5448afe
branch: https://git.busybox.net/busybox/commit/?id=refs/heads/master

function                                             old     new   delta
change_attributes                                    326     416     +90
list_attributes                                      222     248     +26
close_silently                                        22       -     -22
.rodata                                           103722  103692     -30
fgetsetversion                                        74       -     -74
fgetsetprojid                                        107       -    -107
fgetsetflags                                         148       -    -148
------------------------------------------------------------------------------
(add/remove: 0/4 grow/shrink: 2/1 up/down: 116/-381)         Total: -265 bytes

Signed-off-by: Denys Vlasenko <vda.linux at googlemail.com>
---
 e2fsprogs/chattr.c   |  70 +++++++++++++++++++-----------
 e2fsprogs/e2fs_lib.c | 119 +--------------------------------------------------
 e2fsprogs/e2fs_lib.h |  17 +-------
 e2fsprogs/lsattr.c   |  29 +++++++++----
 shell/ash.c          |   9 ++--
 5 files changed, 69 insertions(+), 175 deletions(-)

diff --git a/e2fsprogs/chattr.c b/e2fsprogs/chattr.c
index 4dfe803d2..f6a9fcf30 100644
--- a/e2fsprogs/chattr.c
+++ b/e2fsprogs/chattr.c
@@ -61,9 +61,9 @@
 #define OPT_SET_PROJ (1 << 4)
 
 struct globals {
-	unsigned long version;
-	unsigned long af;
-	unsigned long rf;
+	unsigned version;
+	unsigned af;
+	unsigned rf;
 	int flags;
 	uint32_t projid;
 	smallint recursive;
@@ -79,7 +79,7 @@ static unsigned long get_flag(char c)
 
 static char** decode_arg(char **argv, struct globals *gp)
 {
-	unsigned long *fl;
+	unsigned *fl;
 	const char *arg = *argv;
 	char opt = *arg;
 
@@ -149,7 +149,8 @@ static int FAST_FUNC chattr_dir_proc(const char *dir_name, struct dirent *de, vo
 
 static void change_attributes(const char *name, struct globals *gp)
 {
-	unsigned long fsflags;
+	unsigned fsflags;
+	int fd;
 	struct stat st;
 
 	if (lstat(name, &st) != 0) {
@@ -166,33 +167,50 @@ 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;
 
-	if (gp->flags & OPT_SET_VER)
-		if (fsetversion(name, gp->version) != 0)
-			bb_perror_msg("setting %s on %s", "version", name);
+	fd = open_or_warn(name, O_RDONLY | O_NONBLOCK); /* neither read nor write asked for */
+	if (fd >= 0) {
+		int r;
 
-	if (gp->flags & OPT_SET_PROJ)
-		if (fsetprojid(name, gp->projid) != 0)
-			bb_perror_msg("setting %s on %s", "project ID", name);
+		if (gp->flags & OPT_SET_VER) {
+			r = ioctl(fd, EXT2_IOC_SETVERSION, &gp->version);
+			if (r != 0)
+				bb_perror_msg("setting %s on %s", "version", name);
+		}
 
-	if (gp->flags & OPT_SET) {
-		fsflags = gp->af;
-	} else {
-		if (fgetflags(name, &fsflags) != 0) {
-			bb_perror_msg("reading flags on %s", name);
-			goto skip_setflags;
+		if (gp->flags & OPT_SET_PROJ) {
+			struct ext2_fsxattr fsxattr;
+			r = ioctl(fd, EXT2_IOC_FSGETXATTR, &fsxattr);
+			if (r != 0)
+				bb_perror_msg("getting %s on %s", "project ID", name);
+			fsxattr.fsx_projid = gp->projid;
+			r = ioctl(fd, EXT2_IOC_FSSETXATTR, &fsxattr);
+			if (r != 0)
+				bb_perror_msg("setting %s on %s", "project ID", name);
 		}
-		/*if (gp->flags & OPT_REM) - not needed, rf is zero otherwise */
-			fsflags &= ~gp->rf;
-		/*if (gp->flags & OPT_ADD) - not needed, af is zero otherwise */
-			fsflags |= gp->af;
+
+		if (gp->flags & OPT_SET) {
+			fsflags = gp->af;
+		} else {
+			r = ioctl(fd, EXT2_IOC_GETFLAGS, &fsflags);
+			if (r != 0) {
+				bb_perror_msg("getting %s on %s", "flags", name);
+				goto skip_setflags;
+			}
+			/*if (gp->flags & OPT_REM) - not needed, rf is zero otherwise */
+				fsflags &= ~gp->rf;
+			/*if (gp->flags & OPT_ADD) - not needed, af is zero otherwise */
+				fsflags |= gp->af;
 // What is this? And why it's not done for SET case?
-		if (!S_ISDIR(st.st_mode))
-			fsflags &= ~EXT2_DIRSYNC_FL;
+			if (!S_ISDIR(st.st_mode))
+				fsflags &= ~EXT2_DIRSYNC_FL;
+		}
+		r = ioctl(fd, EXT2_IOC_SETFLAGS, &fsflags);
+		if (r != 0)
+			bb_perror_msg("setting %s on %s", "flags", name);
+ skip_setflags:
+		close(fd);
 	}
-	if (fsetflags(name, fsflags) != 0)
-		bb_perror_msg("setting flags on %s", name);
 
- skip_setflags:
 	if (gp->recursive && S_ISDIR(st.st_mode))
 		iterate_on_dir(name, chattr_dir_proc, gp);
 }
diff --git a/e2fsprogs/e2fs_lib.c b/e2fsprogs/e2fs_lib.c
index 8d56add2d..0ec4eb2f2 100644
--- a/e2fsprogs/e2fs_lib.c
+++ b/e2fsprogs/e2fs_lib.c
@@ -8,8 +8,6 @@
 #include "libbb.h"
 #include "e2fs_lib.h"
 
-#define HAVE_EXT2_IOCTLS 1
-
 #if INT_MAX == LONG_MAX
 #define IF_LONG_IS_SAME(...) __VA_ARGS__
 #define IF_LONG_IS_WIDER(...)
@@ -18,14 +16,6 @@
 #define IF_LONG_IS_WIDER(...) __VA_ARGS__
 #endif
 
-static void close_silently(int fd)
-{
-	int e = errno;
-	close(fd);
-	errno = e;
-}
-
-
 /* Iterate a function on each entry of a directory */
 int iterate_on_dir(const char *dir_name,
 		int FAST_FUNC (*func)(const char *, struct dirent *, void *),
@@ -45,113 +35,6 @@ int iterate_on_dir(const char *dir_name,
 	return 0;
 }
 
-
-/* Get/set a file version on an ext2 file system */
-int fgetsetversion(const char *name, unsigned long *get_version, unsigned long set_version)
-{
-#if HAVE_EXT2_IOCTLS
-	int fd, r;
-	IF_LONG_IS_WIDER(unsigned ver;)
-
-	fd = open(name, O_RDONLY | O_NONBLOCK);
-	if (fd == -1)
-		return -1;
-	if (!get_version) {
-		IF_LONG_IS_WIDER(
-			ver = (unsigned) set_version;
-			r = ioctl(fd, EXT2_IOC_SETVERSION, &ver);
-		)
-		IF_LONG_IS_SAME(
-			r = ioctl(fd, EXT2_IOC_SETVERSION, (void*)&set_version);
-		)
-	} else {
-		IF_LONG_IS_WIDER(
-			r = ioctl(fd, EXT2_IOC_GETVERSION, &ver);
-			*get_version = ver;
-		)
-		IF_LONG_IS_SAME(
-			r = ioctl(fd, EXT2_IOC_GETVERSION, (void*)get_version);
-		)
-	}
-	close_silently(fd);
-	return r;
-#else /* ! HAVE_EXT2_IOCTLS */
-	errno = EOPNOTSUPP;
-	return -1;
-#endif /* ! HAVE_EXT2_IOCTLS */
-}
-
-int fgetsetprojid(const char *name, uint32_t *get, uint32_t set)
-{
-#if HAVE_EXT2_IOCTLS
-	struct ext2_fsxattr fsxattr;
-	int fd, r;
-
-	fd = open(name, O_RDONLY | O_NONBLOCK);
-	if (fd == -1)
-		return -1;
-	r = ioctl(fd, EXT2_IOC_FSGETXATTR, &fsxattr);
-	/* note: ^^^ may fail in 32-bit userspace on 64-bit kernel (seen on 4.12.0) */
-	if (r == 0) {
-		if (get) {
-			*get = fsxattr.fsx_projid;
-		} else {
-			fsxattr.fsx_projid = set;
-			r = ioctl(fd, EXT2_IOC_FSSETXATTR, &fsxattr);
-		}
-	}
-	close_silently(fd);
-	return r;
-#else /* ! HAVE_EXT2_IOCTLS */
-	errno = EOPNOTSUPP;
-	return -1;
-#endif /* ! HAVE_EXT2_IOCTLS */
-}
-
-/* Get/set a file flags on an ext2 file system */
-int fgetsetflags(const char *name, unsigned long *get_flags, unsigned long set_flags)
-{
-#if HAVE_EXT2_IOCTLS
-	struct stat buf;
-	int fd, r;
-	IF_LONG_IS_WIDER(unsigned f;)
-
-	if (stat(name, &buf) == 0 /* stat is ok */
-	 && !S_ISREG(buf.st_mode) && !S_ISDIR(buf.st_mode)
-	) {
-		goto notsupp;
-	}
-	fd = open(name, O_RDONLY | O_NONBLOCK); /* neither read nor write asked for */
-	if (fd == -1)
-		return -1;
-
-	if (!get_flags) {
-		IF_LONG_IS_WIDER(
-			f = (unsigned) set_flags;
-			r = ioctl(fd, EXT2_IOC_SETFLAGS, &f);
-		)
-		IF_LONG_IS_SAME(
-			r = ioctl(fd, EXT2_IOC_SETFLAGS, (void*)&set_flags);
-		)
-	} else {
-		IF_LONG_IS_WIDER(
-			r = ioctl(fd, EXT2_IOC_GETFLAGS, &f);
-			*get_flags = f;
-		)
-		IF_LONG_IS_SAME(
-			r = ioctl(fd, EXT2_IOC_GETFLAGS, (void*)get_flags);
-		)
-	}
-
-	close_silently(fd);
-	return r;
- notsupp:
-#endif /* HAVE_EXT2_IOCTLS */
-	errno = EOPNOTSUPP;
-	return -1;
-}
-
-
 /* Print file attributes on an ext2 file system */
 const uint32_t e2attr_flags_value[] ALIGN4 = {
 #ifdef ENABLE_COMPRESSION
@@ -215,7 +98,7 @@ static const char e2attr_flags_lname[] ALIGN1 =
 	"Verity" "\0"
 	/* Another trailing NUL is added by compiler */;
 
-void print_e2flags(FILE *f, unsigned long flags, unsigned options)
+void print_e2flags(FILE *f, unsigned flags, unsigned options)
 {
 	const uint32_t *fv;
 	const char *fn;
diff --git a/e2fsprogs/e2fs_lib.h b/e2fsprogs/e2fs_lib.h
index 82a1581cb..1a5d092c0 100644
--- a/e2fsprogs/e2fs_lib.h
+++ b/e2fsprogs/e2fs_lib.h
@@ -16,25 +16,10 @@ int iterate_on_dir(const char *dir_name,
 		int FAST_FUNC (*func)(const char *, struct dirent *, void *),
 		void *private);
 
-/* Get/set a file version on an ext2 file system */
-int fgetsetversion(const char *name, unsigned long *get_version, unsigned long set_version);
-#define fgetversion(name, version) fgetsetversion(name, version, 0)
-#define fsetversion(name, version) fgetsetversion(name, NULL, version)
-
-/* Get/set a file project ID on an ext2 file system */
-int fgetsetprojid(const char *name, uint32_t *get, uint32_t set);
-#define fgetprojid(name, projid) fgetsetprojid(name, projid, 0)
-#define fsetprojid(name, projid) fgetsetprojid(name, NULL, projid)
-
-/* Get/set a file flags on an ext2 file system */
-int fgetsetflags(const char *name, unsigned long *get_flags, unsigned long set_flags);
-#define fgetflags(name, flags) fgetsetflags(name, flags, 0)
-#define fsetflags(name, flags) fgetsetflags(name, NULL, flags)
-
 /* Must be 1 for compatibility with 'int long_format'. */
 #define PFOPT_LONG  1
 /* Print file attributes on an ext2 file system */
-void print_e2flags(FILE *f, unsigned long flags, unsigned options);
+void print_e2flags(FILE *f, unsigned flags, unsigned options);
 
 extern const uint32_t e2attr_flags_value[];
 extern const char e2attr_flags_sname[];
diff --git a/e2fsprogs/lsattr.c b/e2fsprogs/lsattr.c
index 9b035f584..3972ce8a7 100644
--- a/e2fsprogs/lsattr.c
+++ b/e2fsprogs/lsattr.c
@@ -46,25 +46,35 @@ enum {
 
 static void list_attributes(const char *name)
 {
-	unsigned long fsflags;
+	unsigned fsflags;
+	int fd, r;
 
-	if (fgetflags(name, &fsflags) != 0)
-		goto read_err;
+	fd = open_or_warn(name, O_RDONLY | O_NONBLOCK); /* neither read nor write asked for */
+	if (fd < 0) /* for example, dangling links */
+		return;
 
 	if (option_mask32 & OPT_PROJID) {
-		uint32_t p;
-		if (fgetprojid(name, &p) != 0)
+		struct ext2_fsxattr fsxattr;
+		r = ioctl(fd, EXT2_IOC_FSGETXATTR, &fsxattr);
+		if (r != 0)
 			goto read_err;
-		printf("%5lu ", (unsigned long)p);
+		printf("%5u ", (unsigned)fsxattr.fsx_projid);
 	}
 
 	if (option_mask32 & OPT_GENERATION) {
-		unsigned long generation;
-		if (fgetversion(name, &generation) != 0)
+		unsigned generation;
+		r = ioctl(fd, EXT2_IOC_GETVERSION, &generation);
+		if (r != 0)
 			goto read_err;
-		printf("%-10lu ", generation);
+		printf("%-10u ", generation);
 	}
 
+	r = ioctl(fd, EXT2_IOC_GETFLAGS, &fsflags);
+	if (r != 0)
+		goto read_err;
+
+	close(fd);
+
 	if (option_mask32 & OPT_PF_LONG) {
 		printf("%-28s ", name);
 		print_e2flags(stdout, fsflags, PFOPT_LONG);
@@ -77,6 +87,7 @@ static void list_attributes(const char *name)
 	return;
  read_err:
 	bb_perror_msg("reading %s", name);
+	close(fd);
 }
 
 static int FAST_FUNC lsattr_dir_proc(const char *dir_name,
diff --git a/shell/ash.c b/shell/ash.c
index bee81920a..2eac6e113 100644
--- a/shell/ash.c
+++ b/shell/ash.c
@@ -12751,7 +12751,7 @@ parsesub: {
 			do {
 				STPUTC(c, out);
 				c = pgetc_eatbnl();
-			} while (!subtype && isdigit(c));
+			} while ((subtype == 0 || subtype == VSLENGTH) && isdigit(c));
 		} else if (c != '}') {
 			/* $[{[#]]<specialchar>[}] */
 			int cc = c;
@@ -12781,11 +12781,6 @@ parsesub: {
 		} else
 			goto badsub;
 
-		if (c != '}' && subtype == VSLENGTH) {
-			/* ${#VAR didn't end with } */
-			goto badsub;
-		}
-
 		if (subtype == 0) {
 			static const char types[] ALIGN1 = "}-+?=";
 			/* ${VAR...} but not $VAR or ${#VAR} */
@@ -12842,6 +12837,8 @@ parsesub: {
 #endif
 			}
 		} else {
+			if (subtype == VSLENGTH && c != '}')
+				subtype = 0;
  badsub:
 			pungetc();
 		}


More information about the busybox-cvs mailing list