[Toybox] [PATCH] Correct Setting of uid and gid when extracting CPIO archive

[Rob Landley]

On 12/02/2015 07:39 AM, Mike Moreton wrote:
> Hi, I’ve fixed a couple of bugs in cpio.

Thanks, I applied both patches. Several good fixes here in the "I don't
know how this ever worked before" category, but:

1) git am says it doesn't have a valid email address. (I fixed that up
to get it to apply.)

2) I don't understand this bit:

-      if (!S_ISREG(mode) && !S_ISLNK(mode) && !getpid()) {
-        int fd = open(name, O_WRONLY|O_NOFOLLOW);
+      if (!S_ISREG(mode) && !S_ISLNK(mode) && !geteuid()) {
+        int fd = open(name, O_RDONLY|O_NOFOLLOW);

The purpose of the filehandle is to call fchown() on it. Can we change
ownership through a read only filehandle? (The kernel not only allows
this now but will continue to allow this in future?)

The O_WRONLY was also there because you can create a node or directory
without the read bit set, in which case attempting to open it for read
access fails.

I'd really like to do this via O_PATH, but trying to gave me errno
EBADF. And it turns out that the bionic workaround of doing:

  fd = open("file", O_PATH)
  sprintf(buf, "/proc/self/fd/%d");
  fd2 = open(buf, O_RDONLY);

Does the proper permissions checks on the second open (good!) which
means that using O_PATH to try to reliably affect the metadata of chmod
000 files STILL DOESN'T WORK. Grrr. I expect I have to create
directories and nodes chmod u+w and then chmod them again to the final
version later. I can presumably add some extra tests to make this only
happen when necessary...

Minor comments on second patch:

I try to keep the code within 80 chars. Code style thing. This goes way
off the right edge without rewrapping.

I generally put the local variable tests _before_ the tests that
potentially generate system calls, because && preserves side effects so
the left side does get evaluated even if the right will always fail.
(Fairly minor nitpick.)

Thanks,

Rob

 1449439462.0