Subject: bug#15926: RFE: unlink command already uses 'unlink' call; make 'rm' use 'remove' call

From: Pádraig Brady <[email-masked]>
Date: Sat, 23 Nov 2013 11:49:38 +0000

On 11/23/2013 10:55 AM, Pádraig Brady wrote:
> On 11/23/2013 02:32 AM, Eric Blake wrote:
>> On 11/22/2013 06:30 PM, Pádraig Brady wrote:
>>>> +
>>>> +# Pull rm(1) the teeth by intercepting the unlinkat() system call via the
>>>
>>> s/the// ?
>>
>> Pull the teeth from rm(1) by...
>>
>>>
>>> So the real case where this could not be handled (and what might
>>> actually catch users out) is when various synonyms of '/' are specified.
>>> i.e. // //. /./ /bin/..
>>> It would be good to have tests for those.

as explained below, the names containing . or .. would not
contain this particular diagnostic, though we'd probably
want to test anyway and ensure ignored to protect against
future reordering of the checks in code.

Also a symlink test as described below would be good to add.

>> Careful on the // case, for systems like cygwin where // is a distinct
>> root from / (it has a separate inode, but should likewise be forbidden
>> to recursively delete from // - hmm, I don't know that I've ever been
>> brave enough to actually test whether --preserve-root preserves // on
>> cygwin).
> 
> I would expect // to be treated the same on all systems in this regard
> 
>> Also, if a user does 'ln -s / root' and does 'rm -r root/.'
>> (technically, this should also be done for 'rm -r root/', but Linux
>> doesn't obey POSIX with regards to 'rm symlink-to-dir/').
> 
> If rm is going to do this at all it might as well
> handle all corner cases (that are much more likely to
> catch users out). I.E. it should be working on the
> canonicalized name.

To be clear, I think rm is working as expected already,
since it checks the dev,inode identity of the command argument
against that of '/'.  So symlinks are handled as expected:

$ rm -ri ~/root/
src/rm: it is dangerous to operate recursively on ‘/home/padraig/root/’ (same as ‘/’)
src/rm: use --no-preserve-root to override this failsafe

Note the trailing '.' for the 'root/.' case will trigger that check first.

cheers,
Pádraig.