On Thu, Aug 14, 2008 at 11:46:32PM +0200, Denys Vlasenko wrote:
> On Wednesday 13 August 2008 17:57, Doug Graham wrote:
> > In make_directory.c, bb_make_directory() is slightly broken in two ways.
> > This function messes around with mode and umask, I think in an attempt
> > to make sure that intermediate directories created with the -p option
> > are always writable. However, the umask is only changed if the mode
> > passed in is not -1, which is the usual case. So this breaks:
> >
> > $ umask 0222
> > $ ./busybox mkdir -p foo/bar
> > mkdir: cannot create directory 'foo/bar': Permission denied
> >
> > I think that bb_make_directory() needs to temporarily change the umask
> > regardless of whether or not the mode passed in is -1.
>
> Sure other applets which use this function would want it this way?
Pretty sure, but it's hard to be absolutely sure, since this affects only
a rare scenario that isn't likely to be explicitly documented for any of
the applets that use bb_make_directory(), or their full-blown equivalents.
For example, there's nothing in the mkdir(1) man page that states what the
behaviour is supposed to be if the umask is set strangely and intermediate
directories must be created. In any case, it makes no sense to me to
set the umask only in the case where bb_make_directory() is passed an
explicit mode. I don't see the connection between those two things.
The only effect of this change is that some applet that might previously
have failed because the umask was set strangely will now succeed and
leave intermediate directories with the u+wx bits set. If the applet
is run by root, it would have succeeded anyway, so the only possible
difference will be in the u+wx bits of intermediate directories.
> > Here is a patch for both problems. With this patch, the umask is *always*
> > set so that intermediate directories will be writable, but the original
>
> umask(mask & ~0300); /* ensure intermediate dirs are u rx */
>
> comment says "rx" but you actually ensured "rw". "rx" are 5 octal.
> what was intended?
Oops. Comment fixed. What is intended is that intermediate directories
be created with the u+wx bits set.
> Please rediff the patch so that it is a unified diff, applyable
> with "patch <file" or "patch -p1 <file" at the top of the source tree.
> Please use tabs for indentation.
Ok. Please apply the following with "patch -p1 < file" from the top of
the source tree.
diff -ru busybox-1.11.1.orig/libbb/make_directory.c
busybox-1.11.1/libbb/make_directory.c
--- busybox-1.11.1.orig/libbb/make_directory.c 2008-06-25 08:51:24.000000000
-0400
+++ busybox-1.11.1/libbb/make_directory.c 2008-08-14 19:49:09.000000000
-0400
@@ -35,14 +35,7 @@
struct stat st;
mask = umask(0);
- if (mode == -1) {
- umask(mask);
- mode = (S_IXUSR | S_IXGRP | S_IXOTH |
- S_IWUSR | S_IWGRP | S_IWOTH |
- S_IRUSR | S_IRGRP | S_IROTH) & ~mask;
- } else {
- umask(mask & ~0300);
- }
+ umask(mask & ~0300); /* ensure intermediate dirs are u wx */
do {
c = 0;
@@ -62,6 +55,9 @@
}
}
+ if (!c) /* last component use orig umask */
+ umask(mask);
+
if (mkdir(path, 0777) < 0) {
/* If we failed for any other reason than the directory
* already exists, output a diagnostic and return -1.*/
@@ -85,7 +81,6 @@
/* Done. If necessary, updated perms on the newly
* created directory. Failure to update here _is_
* an error.*/
- umask(mask);
if ((mode != -1) && (chmod(path, mode) < 0)){
fail_msg = "set permissions of";
break;
--Doug.
_______________________________________________
busybox mailing list
[email protected]
http://busybox.net/cgi-bin/mailman/listinfo/busybox
