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?
> The second bug is that a chmod() is always done on the last component
> of the directory being created:
>
> if ((mode != -1) && (chmod(path, mode) < 0)){
> fail_msg = "set permissions of";
> break;
> }
>
> Mode can never be -1 at this point, because if it were -1 on entry to
> bb_make_directory(), it would have been changed to (0777 & ~mask) at
> the top of the function. So the chmod is always done, and will always
> clear any bits other than the 0777 bits. It our case, we want the new
> directory to inherit the setgid bit from the parent directory, but this
> chmod breaks that.
>
> 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?
> umask is restored before creating the final directory. So no chmod()
> is required in the default case.
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.
--
vda
_______________________________________________
busybox mailing list
[email protected]
http://busybox.net/cgi-bin/mailman/listinfo/busybox
