[Toybox] [PATCH] cpio: Implement -u (copy unconditionally)

Rob Landley rob at landley.net
Fri Feb 26 23:17:26 PST 2021


On 2/20/21 8:34 AM, Yi-yo Chiang via Toybox wrote:
> *ping* (in case this got categorized as spam by gmail)

It did, and so did this reply. :)

> On Sun, Feb 14, 2021 at 9:13 PM Yi-Yo Chiang <yochiang at google.com
> <mailto:yochiang at google.com>> wrote:
> 
>     If -u is specified, then replace any existing files or directories.

When does it replace directories? Do we replace a directory with a file, or a
file with a directory? (What if the directory to be replaced with a file isn't
empty, ala rm -r vs rmdir?)

Let's see...

  $ mkdir walrus
  $ echo hello > walrus/file
  $ echo walrus/file | cpio -H newc -o > blah.cpio
  $ cpio -H newc -u -i < blah.cpio
  1 block
  $ mv walrus walrus2
  $ touch walrus
  $ cpio -H newc -i < blah.cpio
  cpio: walrus/file: Cannot open: Not a directory
  1 block
  $ cpio -H newc -u -i < blah.cpio
  cpio: walrus/file: Cannot open: Not a directory
  1 block
  $ cpio -H newc -dui < blah.cpio
  cpio: `walrus' exists but is not a directory
  cpio: walrus/file: Cannot open: Not a directory
  1 block

Nope, doesn't look like it. Let's see, will it replace a /dev node...? Yes it
does (rather than writing the contents to it. What's the default tar behavior
here... replaces the dev node. Which is not what solaris did back in college, I
note. Why do we want cpio's default behavior to differ from tar's here?)

In any case, it looks like the expected behavior is to open with O_EXCL and
unlink if it failed. Under what circumstances DOES it replace an existing file
with a directory, or an existing directory with a file? (The tests you submitted
only replace files with other files.)

I'd like to avoid the separate "stat" if necessary not just because it's extra
code but because it's a race condition between two non-atomic operations waiting
to happen. (I have no idea if the gap is exploitable, but I don't want to worry
about it if just not doing that is an option? And not doing that would be open
O_EXCL and on failure unlink and O_EXCL a second time.)

(You'll notice the existing stat was an fstat() on an already open file
descriptor, and you're adding an lstat on a path which could get swapped out
afterwards. In general I try not to do that because it seems racy with programs
dropping symlinks based on inotify triggers. Yeah you've sprayed the system down
with selinux but I don't want to worry about it at a design level...)

Unfortunately I haven't had the spare focus this week to properly wrap my brain
around what I want to happen here. I finally got a chance to look at it the end
of the day friday, and just wanted to say why I haven't applied it as-is yet.
I'll try to redo it this weekend, but first I need to confirm that open(O_EXCL)
is sufficient error detection and what we do about conflicting directory
creation (is there a case where -u DOES apply to -d?) and adding tests for the
corner cases limning in the design decisions.

I need to properly wrap my head around this, but haven't been at my best the
past couple weeks. Slightly overstretched. Sorry 'bout that.

Rob


More information about the Toybox mailing list