<div dir="ltr"><div dir="ltr"><div>Using different command to create the archive yields different result, I used $ find dir | cpio -o ... to create the archive for testing:</div><div><br></div><div>(testing the behavior of GNU cpio)</div><div>$ mkdir a</div><div>$ echo blah > a/bb</div><div>$ find a | cpio -H newc -o >a.cpio</div><div>$ # Archive a.cpio creates a (dir) and a/bb (file)</div><div><br></div><div>$ # Test dir from archive replaces existing file</div><div></div><div>$ rm -rf a</div><div>$ touch a</div><div>$ cpio -i <a.cpio</div><div>cpio: a not created: newer or same age version exists<br>cpio: a/bb: Cannot open: Not a directory<br>1 block<br></div><div>$ # Failed to create a (dir)</div><div>$ touch -d yesterday a</div><div>$ cpio -i <a.cpio</div>1 block</div><div>$ # Interesting, if the timestamp of a (file) is older than the archive's a (dir), then replacement happens</div><div dir="ltr">$ touch a<br><div>$ cpio -iu <a.cpio</div>1 block<br></div><div>$ # -u unlinks a (file) then mkdir(a)</div><div><br></div><div>$ # Test file from archive replaces existing empty dir</div><div>$ rm a/bb</div><div>$ mkdir a/bb</div><div>$ cpio -i <a.cpio</div>cpio: a/bb not created: newer or same age version exists<br>1 block<div>$ touch -d yesterday a/bb<br><div>$ cpio -i <a.cpio<br>1 block</div><div>$ rm a/bb<div>$ mkdir a/bb</div><div class="gmail_quote"><div dir="ltr" class="gmail_attr"><div>$ cpio -iu <a.cpio<br>1 block</div><div>$ # Overall similar result when replacing existing file.</div></div><div dir="ltr" class="gmail_attr"><br></div><div class="gmail_attr">$ # Test when a/bb is not empty</div><div class="gmail_attr">$ rm -rf a</div><div class="gmail_attr">$ mkdir -p a/bb/ccc</div><div class="gmail_attr">$ cpio -i <a.cpio</div>cpio: a/bb not created: newer or same age version exists<br>1 block<br><div dir="ltr" class="gmail_attr">$ touch -d yesterday a/bb</div><div dir="ltr" class="gmail_attr">$ cpio -i <a.cpio</div>cpio: cannot remove current a/bb: Directory not empty<br>1 block</div><div class="gmail_quote">$ cpio -iu <a.cpio</div>cpio: cannot remove current a/bb: Directory not empty<br>1 block</div><div>$ # Never replaces an existing dir</div><div><div class="gmail_quote"><br><div dir="ltr" class="gmail_attr">On Sat, Feb 27, 2021 at 3:03 PM Rob Landley <<a href="mailto:rob@landley.net">rob@landley.net</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On 2/20/21 8:34 AM, Yi-yo Chiang via Toybox wrote:<br>
> *ping* (in case this got categorized as spam by gmail)<br>
<br>
It did, and so did this reply. :)<br>
<br>
> On Sun, Feb 14, 2021 at 9:13 PM Yi-Yo Chiang <<a href="mailto:yochiang@google.com" target="_blank" class="cremed">yochiang@google.com</a><br>
> <mailto:<a href="mailto:yochiang@google.com" target="_blank" class="cremed">yochiang@google.com</a>>> wrote:<br>
> <br>
>     If -u is specified, then replace any existing files or directories.<br>
<br>
When does it replace directories? Do we replace a directory with a file, or a<br>
file with a directory? (What if the directory to be replaced with a file isn't<br>
empty, ala rm -r vs rmdir?)<br></blockquote><div><br></div><div>Honestly I'm not sure, and IIUC the spec doesn't say what to do either (<a href="https://pubs.opengroup.org/onlinepubs/7908799/xcu/cpio.html">https://pubs.opengroup.org/onlinepubs/7908799/xcu/cpio.html</a>), so I assumed it is implementation specific?</div><div>In this case I choose the behavior that is simplest to implement (IMO).</div><div><br></div><div>* If the *path* to be extracted already existed, then unlink/rmdir the path. *</div><div><br></div><div>So yes we replace a file with a directory, and replace a directory (so long as rmdir() success, which means directory must be empty) with a file.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Let's see...<br>
<br>
  $ mkdir walrus<br>
  $ echo hello > walrus/file<br>
  $ echo walrus/file | cpio -H newc -o > blah.cpio<br>
  $ cpio -H newc -u -i < blah.cpio<br>
  1 block<br>
  $ mv walrus walrus2<br>
  $ touch walrus<br>
  $ cpio -H newc -i < blah.cpio<br>
  cpio: walrus/file: Cannot open: Not a directory<br>
  1 block<br>
  $ cpio -H newc -u -i < blah.cpio<br>
  cpio: walrus/file: Cannot open: Not a directory<br>
  1 block<br>
  $ cpio -H newc -dui < blah.cpio<br>
  cpio: `walrus' exists but is not a directory<br>
  cpio: walrus/file: Cannot open: Not a directory<br>
  1 block<br>
<br>
Nope, doesn't look like it. Let's see, will it replace a /dev node...? Yes it<br>
does (rather than writing the contents to it. What's the default tar behavior<br>
here... replaces the dev node. Which is not what solaris did back in college, I<br>
note. Why do we want cpio's default behavior to differ from tar's here?)<br></blockquote><div><br></div><div>I didn't follow.. Are you saying GNU tar replaces a device node, and Solaris tar writes to a device node?</div><div>Do we want toybox cpio/tar to be feature-identical to GNU or Solaris' implementation? (I hope not, cause that feels like a bloat to code size)</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
In any case, it looks like the expected behavior is to open with O_EXCL and<br>
unlink if it failed. Under what circumstances DOES it replace an existing file<br>
with a directory, or an existing directory with a file? (The tests you submitted<br>
only replace files with other files.)<br></blockquote><div><br></div><div>See my example at the start.</div><div>Your commands didn't replace "walrus" because your archive extracts "walrus/file" but not "walrus". </div><div>If you use "find ... | cpio -o" to create the test archive, then the existing "walrus" would be replaced by cpio when cpio is trying to mkdir(walrus).</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
I'd like to avoid the separate "stat" if necessary not just because it's extra<br>
code but because it's a race condition between two non-atomic operations waiting<br>
to happen. (I have no idea if the gap is exploitable, but I don't want to worry<br>
about it if just not doing that is an option? And not doing that would be open<br>
O_EXCL and on failure unlink and O_EXCL a second time.)<br></blockquote><div><br></div><div>Yeah I totally agree. But I'd like to point out that in my first iteration of this code, I do guard the stat() with various conditions and nested if-else-ifs, hoping to only call stat() if EEXIST error happened.</div><div>However the code quickly grew too complex and unreadable, with too many combinations to consider (file replace dir, dir replace file, dir replace dir, what if dir not empty? what about symlink and special files...), so I changed the implementation to the one you're seeing now.</div><div>This is also the reason why I add the "if dir already exist, then just do chmod" shortcut, because race could happen between rmdir(a) and mkdir(a). </div><div><br></div><div>Another point is that when replacing an existing file, having a separate stat() would mean:</div><div>stat(a) & unlink(a) & open(a, O_EXCL)</div><div>but not stat()-ing beforehand would mean:</div><div>open(a, O_EXCL) [fail] & stat(a) & ("a" is a directory ? rmdir : unlink)(a) & open(a, O_EXCL)</div><div>So having a separate "stat" doesn't necessarily mean an extra system call.<br></div><div>If path doesn't exist then the stat() is an extra syscall, otherwise the stat() saves an extra syscall. </div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
(You'll notice the existing stat was an fstat() on an already open file<br>
descriptor, and you're adding an lstat on a path which could get swapped out<br>
afterwards. In general I try not to do that because it seems racy with programs<br>
dropping symlinks based on inotify triggers. Yeah you've sprayed the system down<br>
with selinux but I don't want to worry about it at a design level...)<br>
<br>
Unfortunately I haven't had the spare focus this week to properly wrap my brain<br>
around what I want to happen here. I finally got a chance to look at it the end<br>
of the day friday, and just wanted to say why I haven't applied it as-is yet.<br>
I'll try to redo it this weekend, but first I need to confirm that open(O_EXCL)<br>
is sufficient error detection and what we do about conflicting directory<br>
creation (is there a case where -u DOES apply to -d?) </blockquote><div><br></div><div>I hope not, let's keep it simple...</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">and adding tests for the<br>
corner cases limning in the design decisions.<br></blockquote><div><br></div><div>I can add more tests once we decide the desired behavior.</div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
I need to properly wrap my head around this, but haven't been at my best the<br>
past couple weeks. Slightly overstretched. Sorry 'bout that.<br></blockquote><div><br></div><div>Nah it's cool. Do take care and take your time and don't feel burned out. </div><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
Rob<br>
_______________________________________________<br>
Toybox mailing list<br>
<a href="mailto:Toybox@lists.landley.net" target="_blank" class="cremed">Toybox@lists.landley.net</a><br>
<a href="http://lists.landley.net/listinfo.cgi/toybox-landley.net" rel="noreferrer" target="_blank" class="cremed">http://lists.landley.net/listinfo.cgi/toybox-landley.net</a><br>
</blockquote></div><br clear="all"><div><br></div>-- <br><div dir="ltr" class="gmail_signature"><div dir="ltr"><table width="90%" border="0" cellspacing="0" cellpadding="0" style="margin:0px;padding:0px;font-family:"Times New Roman";max-width:348px"><tbody style="margin:0px;padding:0px"><tr style="margin:0px;padding:0px"><td style="padding:0px"><table border="0" cellspacing="0" cellpadding="0" style="margin:0px;padding:20px 0px 0px"><tbody style="margin:0px;padding:0px"><tr style="margin:0px;padding:0px"><td valign="top" style="padding:0px 20px 0px 0px;vertical-align:top;border-right:1px solid rgb(213,213,213)"><img src="https://i.imgur.com/eGpkLls.png" width="200" height="64"><br></td><td style="padding:0px 0px 0px 20px"><table border="0" cellspacing="0" cellpadding="0" style="margin:0px;padding:0px"><tbody style="margin:0px;padding:0px"><tr style="margin:0px;padding:0px"><td colspan="2" style="font-family:Arial,Helvetica,Verdana,sans-serif;padding:1px 0px 5px;font-size:13px;line-height:13px;color:rgb(56,58,53);font-weight:700">Yi-yo Chiang</td></tr><tr style="margin:0px;padding:0px"><td colspan="2" style="font-family:Arial,Helvetica,Verdana,sans-serif;padding:0px 0px 5px;font-size:11px;line-height:13px;color:rgb(56,58,53)">Software Engineer</td></tr><tr style="margin:0px;padding:0px"><td colspan="2" style="font-family:Arial,Helvetica,Verdana,sans-serif;padding:0px 0px 5px;font-size:11px;line-height:13px;color:rgb(56,58,53)"><a href="mailto:yochiang@google.com" target="_blank" class="cremed">yochiang@google.com</a></td></tr><tr style="margin:0px;padding:0px"><td colspan="2" style="font-family:Arial,Helvetica,Verdana,sans-serif;padding:0px 0px 3px;font-size:11px;line-height:13px;color:rgb(3,112,248)"></td></tr></tbody></table></td></tr></tbody></table></td></tr></tbody></table></div></div></div></div></div>