[prev in list] [next in list] [prev in thread] [next in thread] 

List:       busybox
Subject:    Re: rm -r fails to delete entire hierarchy when path goes in and out of it
From:       Gian Ntzik <gian.ntzik08 () imperial ! ac ! uk>
Date:       2014-09-18 19:17:58
Message-ID: 87ha04ohzt.fsf () imperial ! ac ! uk
[Download RAW message or body]

Denys Vlasenko <vda.linux@googlemail.com> writes:

> This would work:
>
>
>                 int curdir = xopen(".", O_RDONLY);
>
>                 do {
>                         /* This removes any trailing slashes from *argv.
>                          * It returns "/" _only_ for root directory.
>                          */
>                         char *base = bb_get_last_path_component_strip(*argv);
>
>                         if (DOT_OR_DOTDOT(base) || LONE_CHAR(base, '/')) {
>                                 bb_error_msg("can't remove '.', '..' or '/'");
>                                 status = 1;
>                                 goto next;
>                         }
>                         if (*argv != base) { /* if *args contains slash(es) */
>                                 base[-1] = '\0';
>                                 if (chdir(*argv) != 0) {
>                                         status = 1;
>                                         goto next;
>                                 }
>                         }
>                         if (remove_file(base, flags) < 0) {
>                                 status = 1;
>                         }
>                         if (fchdir(curdir) != 0) {
>                                 bb_error_msg_and_die("can't return to
> current directory");
>                         }
>  next: ;
>                 } while (*++argv);
>

I'm not sure if showing a truncated path in verbose or interactive mode
is the right thing to do. At least to me it seems a bit
counter-intuitive when you are removing something which is not located
within your current directory.

Here is an alternative which uses realpath():

.
.
.
	struct stat path_stat;
	char *path;
	int free_path = 0;
.
.
.
		do {
			/* This removes any trailing slashes from *argv.
                         * It returns "/" _only_ for root directory.
                         */
			const char *base = bb_get_last_path_component_strip(*argv);

                        if (DOT_OR_DOTDOT(base) || LONE_CHAR(base, '/')) {
				bb_error_msg("can't remove '.' or '..' or '/'");
				status = 1;
				continue;
			}

			path = *argv;
			
			/* if *argv is not a symlink canonicallize it */
			if (lstat(*argv, &path_stat) < 0) {
				if (errno != ENOENT || errno != ENOTDIR) {
					bb_perror_msg("can't stat '%s'", *argv);
				} else {
					bb_perror_msg("can't remove '%s'", *argv);
				}
				status = 1;
				continue;
			}
			if (!S_ISLNK(path_stat.st_mode)) {
				if (!(path = realpath(*argv, NULL))) {
					bb_perror_msg("can't remove '%s'", *argv);
					status = 1;
					continue;
				}
				free_path = 1;
			}

			if (remove_file(path, flags) >= 0) {
				continue;
			}
			status = 1;
			if (free_path) free(path);
		} while (*++argv);


In verbose/interactive mode this prints correct absolute canonical
paths.

The downside is the extra lstat() to check if *argv is a symlink to
decide if realpath() should be used or not. But this could be improved if
remove_file was changed to accept an extra optional struct stat
*argument. If NULL, remove_file would perform it's own lstat, otherwise
use the passed argument.

_______________________________________________
busybox mailing list
busybox@busybox.net
http://lists.busybox.net/mailman/listinfo/busybox
[prev in list] [next in list] [prev in thread] [next in thread]