recursive_action: add depth param

chmod: match coreutils versus following links


git-svn-id: svn://busybox.net/trunk/busybox@16462 69ca8d6d-28ef-0310-b511-8ec308f3f277

9 files changed, 123 insertions, 58 deletions

diff --git a/archival/tar.c b/archival/tar.c
index d96d20227..db812e0f4 100644
--- a/archival/tar.c
+++ b/archival/tar.c
@@ -301,7 +301,7 @@ static int exclude_file(const llist_t *excluded_files, const char *file)
 # endif
 
 static int writeFileToTarball(const char *fileName, struct stat *statbuf,
-                                                          void *userData)
+                        void *userData, int depth)
 {
         struct TarBallInfo *tbInfo = (struct TarBallInfo *) userData;
         const char *header_name;
@@ -473,7 +473,7 @@ static int writeTarFile(const int tar_fd, const int verboseFlag,
         /* Read the directory/files and iterate over them one at a time */
         while (include) {
                 if (!recursive_action(include->data, TRUE, dereferenceFlag,
-                                FALSE, writeFileToTarball, writeFileToTarball, &tbInfo))
+                                FALSE, writeFileToTarball, writeFileToTarball, &tbInfo, 0))
                 {
                         errorFlag = TRUE;
                 }
diff --git a/coreutils/chmod.c b/coreutils/chmod.c
index c4f8fa0b2..b601504f8 100644
--- a/coreutils/chmod.c
+++ b/coreutils/chmod.c
@@ -20,9 +20,9 @@
 #define OPT_VERBOSE (USE_DESKTOP(option_mask32 & 2) SKIP_DESKTOP(0))
 #define OPT_CHANGED (USE_DESKTOP(option_mask32 & 4) SKIP_DESKTOP(0))
 #define OPT_QUIET   (USE_DESKTOP(option_mask32 & 8) SKIP_DESKTOP(0))
-#define OPT_STR     ("-R" USE_DESKTOP("vcf"))
+#define OPT_STR     "R" USE_DESKTOP("vcf")
 
-/* TODO:
+/* coreutils:
  * chmod never changes the permissions of symbolic links; the chmod
  * system call cannot change their permissions. This is not a problem
  * since the permissions of symbolic links are never used.
@@ -31,19 +31,26 @@
  * symbolic links encountered during recursive directory traversals.
  */
 
-static int fileAction(const char *fileName, struct stat *statbuf, void* junk)
+static int fileAction(const char *fileName, struct stat *statbuf, void* junk, int depth)
 {
-        mode_t newmode = statbuf->st_mode;
+        mode_t newmode;
 
-        // TODO: match GNU behavior:
-        // if (depth > 0 && S_ISLNK(statbuf->st_mode)) return TRUE;
-        // if (depth == 0) follow link
+        /* match coreutils behavior */
+        if (depth == 0) {
+                /* statbuf holds lstat result, but we need stat (follow link) */
+                if (stat(fileName, statbuf))
+                        goto err;
+        } else { /* depth > 0: skip links */
+                if (S_ISLNK(statbuf->st_mode))
+                        return TRUE;
+        }
+        newmode = statbuf->st_mode;
 
         if (!bb_parse_mode((char *)junk, &newmode))
                 bb_error_msg_and_die("invalid mode: %s", (char *)junk);
 
-        if (chmod(fileName, statbuf->st_mode) == 0) {
-                if (OPT_VERBOSE /* -v verbose? or -c changed? */
+        if (chmod(fileName, newmode) == 0) {
+                if (OPT_VERBOSE
                  || (OPT_CHANGED && statbuf->st_mode != newmode)
                 ) {
                         printf("mode of '%s' changed to %04o (%s)\n", fileName,
@@ -51,7 +58,8 @@ static int fileAction(const char *fileName, struct stat *statbuf, void* junk)
                 }
                 return TRUE;
         }
-        if (!OPT_QUIET) /* not silent (-f)? */
+ err:
+        if (!OPT_QUIET)
                 bb_perror_msg("%s", fileName);
         return FALSE;
 }
@@ -62,30 +70,33 @@ int chmod_main(int argc, char **argv)
         char *arg, **argp;
         char *smode;
 
-        /* Convert first encountered -r into a-r, -w into a-w etc */
-        argp = argv + 1;
-        while ((arg = *argp)) {
+        /* Convert first encountered -r into ar, -w into aw etc
+         * so that getopt would not eat it */
+        argp = argv;
+        while ((arg = *++argp)) {
                 /* Mode spec must be the first arg (sans -R etc) */
                 /* (protect against mishandling e.g. "chmod 644 -r") */
-                if (arg[0] != '-')
+                if (arg[0] != '-') {
+                        arg = NULL;
                         break;
+                }
                 /* An option. Not a -- or valid option? */
-                if (arg[1] && !strchr(OPT_STR, arg[1])) {
-                        argp[0] = xasprintf("a%s", arg);
+                if (arg[1] && !strchr("-"OPT_STR, arg[1])) {
+                        arg[0] = 'a';
                         break;
                 }
-                argp++;
         }
-        /* "chmod -rzzz abc" will say "invalid mode: a-rzzz"!
-         * It is easily fixable, but deemed not worth the code */
 
+        /* Paerse options */
         opt_complementary = "-2";
-        getopt32(argc, argv, OPT_STR + 1); /* Reuse string */
+        getopt32(argc, argv, ("-"OPT_STR) + 1); /* Reuse string */
         argv += optind;
 
-        smode = *argv++;
+        /* Restore option-like mode if needed */
+        if (arg) arg[0] = '-';
 
         /* Ok, ready to do the deed now */
+        smode = *argv++;
         do {
                 if (!recursive_action(*argv,
                                 OPT_RECURSE,    // recurse
@@ -93,7 +104,8 @@ int chmod_main(int argc, char **argv)
                                 FALSE,          // depth first
                                 fileAction,     // file action
                                 fileAction,     // dir action
-                                smode)          // user data
+                                smode,          // user data
+                                0)              // depth
                 ) {
                         retval = EXIT_FAILURE;
                 }
@@ -101,3 +113,45 @@ int chmod_main(int argc, char **argv)
 
         return retval;
 }
+
+/*
+Security: chmod is too important and too subtle.
+This is a test script (busybox chmod versus coreutils).
+Run it in empty dir. Probably requires bash.
+
+#!/bin/sh
+function create() {
+    rm -rf $1; mkdir $1
+    (
+    cd $1 || exit 1
+    mkdir dir
+    >up
+    >file
+    >dir/file
+    ln -s dir linkdir
+    ln -s file linkfile
+    ln -s ../up dir/up
+    )
+}
+function test() {
+    (cd test1; $t1 $1)
+    (cd test2; $t2 $1)
+    (cd test1; ls -lR) >out1
+    (cd test2; ls -lR) >out2
+    echo "chmod $1" >out.diff
+    if ! diff -u out1 out2 >>out.diff; then exit 1; fi
+    mv out.diff out1.diff
+}
+t1="/tmp/busybox chmod"
+t2="/usr/bin/chmod"
+create test1; create test2
+test "a+w file"
+test "a-w dir"
+test "a+w linkfile"
+test "a-w linkdir"
+test "-R a+w file"
+test "-R a-w dir"
+test "-R a+w linkfile"
+test "-R a-w linkdir"
+test "a-r,a+x linkfile"
+*/
diff --git a/coreutils/chown.c b/coreutils/chown.c
index bef89ce86..fddce7cf1 100644
--- a/coreutils/chown.c
+++ b/coreutils/chown.c
@@ -32,7 +32,7 @@ static int (*chown_func)(const char *, uid_t, gid_t) = chown;
  */
 
 static int fileAction(const char *fileName, struct stat *statbuf,
-                void ATTRIBUTE_UNUSED *junk)
+                void ATTRIBUTE_UNUSED *junk, int depth)
 {
         // TODO: -H/-L/-P
         // if (depth ... && S_ISLNK(statbuf->st_mode)) ....
@@ -75,7 +75,8 @@ int chown_main(int argc, char **argv)
                 *groupName++ = '\0';
                 gid = get_ug_id(groupName, bb_xgetgrnam);
         }
-        if (--groupName != *argv) uid = get_ug_id(*argv, bb_xgetpwnam);
+        if (--groupName != *argv)
+                uid = get_ug_id(*argv, bb_xgetpwnam);
         ++argv;
 
         /* Ok, ready to do the deed now */
@@ -86,7 +87,8 @@ int chown_main(int argc, char **argv)
                                 FALSE,          // depth first
                                 fileAction,     // file action
                                 fileAction,     // dir action
-                                NULL)           // user data
+                                NULL,           // user data
+                                0)              // depth 
                 ) {
                         retval = EXIT_FAILURE;
                 }
diff --git a/coreutils/diff.c b/coreutils/diff.c
index 2915d4009..f26bcca86 100644
--- a/coreutils/diff.c
+++ b/coreutils/diff.c
@@ -1030,7 +1030,7 @@ static int dir_strcmp(const void *p1, const void *p2)
 /* This function adds a filename to dl, the directory listing. */
 
 static int add_to_dirlist(const char *filename,
-                                                  struct stat ATTRIBUTE_UNUSED * sb, void *userdata)
+                struct stat ATTRIBUTE_UNUSED * sb, void *userdata, int depth)
 {
         dl_count++;
         dl = xrealloc(dl, dl_count * sizeof(char *));
@@ -1067,7 +1067,7 @@ static char **get_dir(char *path)
         /* Now fill dl with a listing. */
         if (cmd_flags & FLAG_r)
                 recursive_action(path, TRUE, TRUE, FALSE, add_to_dirlist, NULL,
-                                                 userdata);
+                                                 userdata, 0);
         else {
                 DIR *dp;
                 struct dirent *ep;
@@ -1076,7 +1076,7 @@ static char **get_dir(char *path)
                 while ((ep = readdir(dp))) {
                         if ((!strcmp(ep->d_name, "..")) || (!strcmp(ep->d_name, ".")))
                                 continue;
-                        add_to_dirlist(ep->d_name, NULL, NULL);
+                        add_to_dirlist(ep->d_name, NULL, NULL, 0);
                 }
                 closedir(dp);
         }
diff --git a/findutils/find.c b/findutils/find.c
index 62cb5d7eb..8618f3a97 100644
--- a/findutils/find.c
+++ b/findutils/find.c
@@ -56,7 +56,7 @@ static int num_matches;
 static int exec_opt;
 #endif
 
-static int fileAction(const char *fileName, struct stat *statbuf, void* junk)
+static int fileAction(const char *fileName, struct stat *statbuf, void* junk, int depth)
 {
 #ifdef CONFIG_FEATURE_FIND_XDEV
         if (S_ISDIR(statbuf->st_mode) && xdev_count) {
@@ -307,12 +307,12 @@ int find_main(int argc, char **argv)
 
         if (firstopt == 1) {
                 if (!recursive_action(".", TRUE, dereference, FALSE, fileAction,
-                                        fileAction, NULL))
+                                        fileAction, NULL, 0))
                         status = EXIT_FAILURE;
         } else {
                 for (i = 1; i < firstopt; i++) {
                         if (!recursive_action(argv[i], TRUE, dereference, FALSE, fileAction,
-                                                fileAction, NULL))
+                                                fileAction, NULL, 0))
                                 status = EXIT_FAILURE;
                 }
         }
diff --git a/findutils/grep.c b/findutils/grep.c
index b76a17a41..8bb38f95c 100644
--- a/findutils/grep.c
+++ b/findutils/grep.c
@@ -290,7 +290,7 @@ static void load_regexes_from_file(llist_t *fopt)
 }
 
 
-static int file_action_grep(const char *filename, struct stat *statbuf, void* matched)
+static int file_action_grep(const char *filename, struct stat *statbuf, void* matched, int depth)
 {
         FILE *file = fopen(filename, "r");
         if (file == NULL) {
@@ -315,7 +315,8 @@ static int grep_dir(const char *dir)
                 /* depthFirst= */ 1,
                 /* fileAction= */ file_action_grep,
                 /* dirAction= */ NULL,
-                /* userData= */ &matched);
+                /* userData= */ &matched,
+                /* depth= */ 0);
         return matched;
 }
 
diff --git a/include/libbb.h b/include/libbb.h
index 169964ba3..9f9a2b82e 100644
--- a/include/libbb.h
+++ b/include/libbb.h
@@ -194,10 +194,10 @@ extern int is_directory(const char *name, int followLinks, struct stat *statBuf)
 extern int remove_file(const char *path, int flags);
 extern int copy_file(const char *source, const char *dest, int flags);
 extern int recursive_action(const char *fileName, int recurse,
-          int followLinks, int depthFirst,
-          int (*fileAction) (const char *fileName, struct stat* statbuf, void* userData),
-          int (*dirAction) (const char *fileName, struct stat* statbuf, void* userData),
-          void* userData);
+        int followLinks, int depthFirst,
+        int (*fileAction) (const char *fileName, struct stat* statbuf, void* userData, int depth),
+        int (*dirAction) (const char *fileName, struct stat* statbuf, void* userData, int depth),
+        void* userData, int depth);
 extern int device_open(const char *device, int mode);
 extern int get_console_fd(void);
 extern char *find_block_device(char *path);
diff --git a/libbb/recursive_action.c b/libbb/recursive_action.c
index 0d6567dda..ddaf9b896 100644
--- a/libbb/recursive_action.c
+++ b/libbb/recursive_action.c
@@ -11,7 +11,6 @@
 
 #undef DEBUG_RECURS_ACTION
 
-
 /*
  * Walk down all the directories under the specified
  * location, and do something (something specified
@@ -23,16 +22,23 @@
  * is so stinking huge.
  */
 
-static int true_action(const char *fileName, struct stat *statbuf, void* userData)
+static int true_action(const char *fileName, struct stat *statbuf, void* userData, int depth)
 {
         return TRUE;
 }
 
+/*
+ * followLinks=0/1 differs mainly in handling of links to dirs.
+ * 0: lstat(statbuf). Calls fileAction on link name even if points to dir.
+ * 1: stat(statbuf). Calls dirAction and optionally recurse on link to dir.
+ */
+
 int recursive_action(const char *fileName,
                 int recurse, int followLinks, int depthFirst,
-                int (*fileAction) (const char *fileName, struct stat * statbuf, void* userData),
-                int (*dirAction) (const char *fileName, struct stat * statbuf, void* userData),
-                void* userData)
+                int (*fileAction)(const char *fileName, struct stat *statbuf, void* userData, int depth),
+                int (*dirAction)(const char *fileName, struct stat *statbuf, void* userData, int depth),
+                void* userData,
+                int depth)
 {
         struct stat statbuf;
         int status;
@@ -53,21 +59,23 @@ int recursive_action(const char *fileName,
                 return FALSE;
         }
 
-        if (!followLinks && (S_ISLNK(statbuf.st_mode))) {
-                return fileAction(fileName, &statbuf, userData);
+        /* If S_ISLNK(m), then we know that !S_ISDIR(m).
+         * Then we can skip checking first part: if it is true, then
+         * (!dir) is also true! */
+        if ( /* (!followLinks && S_ISLNK(statbuf.st_mode)) || */
+         !S_ISDIR(statbuf.st_mode)
+        ) {
+                return fileAction(fileName, &statbuf, userData, depth);
         }
 
+        /* It's a directory (or a link to one, and followLinks is set) */
+
         if (!recurse) {
-                if (S_ISDIR(statbuf.st_mode)) {
-                        return dirAction(fileName, &statbuf, userData);
-                }
+                return dirAction(fileName, &statbuf, userData, depth);
         }
 
-        if (!S_ISDIR(statbuf.st_mode))
-                return fileAction(fileName, &statbuf, userData);
-
         if (!depthFirst) {
-                status = dirAction(fileName, &statbuf, userData);
+                status = dirAction(fileName, &statbuf, userData, depth);
                 if (!status) {
                         bb_perror_msg("%s", fileName);
                         return FALSE;
@@ -88,14 +96,14 @@ int recursive_action(const char *fileName,
                 if (nextFile == NULL)
                         continue;
                 if (!recursive_action(nextFile, TRUE, followLinks, depthFirst,
-                                        fileAction, dirAction, userData)) {
+                                fileAction, dirAction, userData, depth+1)) {
                         status = FALSE;
                 }
                 free(nextFile);
         }
         closedir(dir);
         if (depthFirst) {
-                if (!dirAction(fileName, &statbuf, userData)) {
+                if (!dirAction(fileName, &statbuf, userData, depth)) {
                         bb_perror_msg("%s", fileName);
                         return FALSE;
                 }
diff --git a/modutils/insmod.c b/modutils/insmod.c
index 00e25f5e3..49b823d0e 100644
--- a/modutils/insmod.c
+++ b/modutils/insmod.c
@@ -801,7 +801,7 @@ static char *m_fullName;
 
 
 static int check_module_name_match(const char *filename, struct stat *statbuf,
-                                void *userdata)
+                                void *userdata, int depth)
 {
         char *fullname = (char *) userdata;
 
@@ -4048,7 +4048,7 @@ int insmod_main( int argc, char **argv)
                         else
                                 module_dir = real_module_dir;
                         recursive_action(module_dir, TRUE, FALSE, FALSE,
-                                        check_module_name_match, 0, m_fullName);
+                                        check_module_name_match, 0, m_fullName, 0);
                         free(tmdn);
                 }
 
@@ -4063,7 +4063,7 @@ int insmod_main( int argc, char **argv)
                         /* No module found under /lib/modules/`uname -r`, this
                          * time cast the net a bit wider.  Search /lib/modules/ */
                         if (!recursive_action(module_dir, TRUE, FALSE, FALSE,
-                                                    check_module_name_match, 0, m_fullName)
+                                                    check_module_name_match, 0, m_fullName, 0)
                         ) {
                                 if (m_filename == 0
                                  || ((fp = fopen(m_filename, "r")) == NULL)