[Toybox] Patch for rm toy.

[Rob Landley]

On Thu, Sep 18, 2014 at 2:02 PM, luckboy at vp.pl <luckboy at vp.pl> wrote:
> I again analyze fix http://landley.net/hg/toybox/rev/b748127e092e.
> I added checking whether path doesn't indicate to directory before unlink.
> Some systems allow to remove directory by unlink for superuser.

Which systems? You're sending a fix for a bug I cannot reproduce.
Please tell me how to reproduce this bug.

> It is dangerous.

Define "dangerous". If the failure you're worried about happens (in a
way you can't explain to me how to reproduce), it will delete an empty
directory the user is otherwise allowed to delete, and which the user
requested to have deleted by calling "rm" on it? That kind of
dangerous?

I thought of using a call to unlinkat() instead, but the flag is
defined as making it work like "unlink()" or "rmdir()", so if your
unlink() is broken your unlinkat() is also broken, according to the
spec.

I note that calling stat() or similar on the file, followed by a
separate call to unlink(), has an inherent race condition that
somebody could swap out the contents between the two calls (by camping
the sucker with inotify and doing 'mv' or a --bind mount or similar).
So if your definition of "dangerous" involves this being used in some
kind of malware exploit (how?), your fix doesn't actually solve
anything. A single call to unlink at least provides an atomic return
fro a single system call instead of a classic "check then act" race
window.

I'm happy to fix a problem I understand, but I do not understand the
problem you're trying to fix.

Rob

 1411079156.0