[BusyBox 0001412]: "cp -a" allows copy of a directory into itself
bugs at busybox.net
bugs at busybox.net
Thu Aug 23 21:23:06 UTC 2007
A NOTE has been added to this issue.
======================================================================
http://busybox.net/bugs/view.php?id=1412
======================================================================
Reported By: kiltedknight
Assigned To: BusyBox
======================================================================
Project: BusyBox
Issue ID: 1412
Category: Other
Reproducibility: always
Severity: minor
Priority: normal
Status: assigned
======================================================================
Date Submitted: 06-29-2007 09:16 PDT
Last Modified: 08-23-2007 14:23 PDT
======================================================================
Summary: "cp -a" allows copy of a directory into itself
Description:
execute the following series of commands on GNU linux:
mkdir /tmp/dir1
touch /tmp/dir1/stuff
cp -a /tmp/dir1 /tmp/dir1
The following error is returned: cp: cannot copy a directory, `/tmp/dir1',
into itself, `/tmp/dir1/dir1'
Now run these:
mkdir /tmp/dir1
touch /tmp/dir1/stuff
busybox cp -a /tmp/dir1 /tmp/dir1
Now you get the following:
cp: cannot stat
'/tmp/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1/dir1':
File name too long
======================================================================
----------------------------------------------------------------------
vapier - 06-29-07 09:22
----------------------------------------------------------------------
i wouldnt really call this a bug ... i'd say "dont do it"
----------------------------------------------------------------------
kiltedknight - 06-29-07 09:28
----------------------------------------------------------------------
Yeah, but you never know what a script will attempt at some point.
----------------------------------------------------------------------
vapier - 06-29-07 09:36
----------------------------------------------------------------------
regardless of what a script does, you're going to end up with errors that
someone needs to go in and clean/fix ... what exact form that error takes
i think is irrelevant
----------------------------------------------------------------------
kiltedknight - 06-29-07 09:46
----------------------------------------------------------------------
Except that this isn't just an error. It actually recursively copies the
directories. GNU Linux cp does not. It does it one time and exits with
the error.
----------------------------------------------------------------------
vapier - 06-29-07 10:36
----------------------------------------------------------------------
... which is still an error that needs to be cleaned up
----------------------------------------------------------------------
vda - 06-30-07 10:21
----------------------------------------------------------------------
I know about this behaviour. It's not strictly a bug, more like "user
definitely made a typo in dir name, warn him and stop instead of filling
entire filesystem with recursive copy".
I think it makes sense for busybox cp to have such code too, just make it
conditional on some CONFIG_FEATURE_CP_xxx.
----------------------------------------------------------------------
kiltedknight - 08-08-07 11:05
----------------------------------------------------------------------
Why should it be conditional? A recursive copy of a directory into itself
may have started as a typo, but it should never continue to execute until
hitting the longest path name allowed or a disk full error, whichever
comes first.
----------------------------------------------------------------------
sh4d0wstr1f3 - 08-21-07 04:58
----------------------------------------------------------------------
coreutils cp handles: copy of dir to dir, copy of dir into dir/sub1/ ...
subN/dir, and copy of dir into dir/ ... /link->dir ... the same code is
also used for move, so it detects a move of dir into
dir/sub1/.../subN/dir.
This is actually a battle that y'all fought long ago in a code base far
far away...
http://www.busybox.net/cgi-bin/viewcvs.cgi?rev=8516&view=rev
http://www.busybox.net/cgi-bin/viewcvs.cgi?rev=8517&view=rev
http://www.busybox.net/cgi-bin/viewcvs.cgi?rev=8547&view=rev
What situation the code in 8517 didn't handle that led Erik to revert it?
Maybe it wasn't pretty -- but it was miles cleaner than coreutils cp.
----------------------------------------------------------------------
kiltedknight - 08-22-07 12:15
----------------------------------------------------------------------
Trying this against 1.4.2 produces two identical error messages:
cp: cannot copy a directory, 'stuff', into itself, 'stuff/stuff'
However, it does not behave the same way as coreutils because with
coreutils, I would find any file within "stuff" also present when doing
"ls stuff/stuff"... with this patch, stuff/stuff is empty.
----------------------------------------------------------------------
kiltedknight - 08-23-07 11:45
----------------------------------------------------------------------
One of my co-workers said something about there being memory leaks in the
old patch.
----------------------------------------------------------------------
sh4d0wstr1f3 - 08-23-07 14:23
----------------------------------------------------------------------
Ok, so there are problems with r8517; for a directory "sub/dir", doing 'cp
-R sub dir' is detected as a recursive copy. Probably not so helpful. It
can also use unitialized values of dest_stat and throws away the pointer
to new_source (which should probably be free()'d on the early return).
I think that the coreutils behavior is helpful (despite the fact that it
leaves residual directories around); accidental trees of the same
directory is not so nice.
I'm going to throw up a small patch. This will detect a recursive copy in
the following cases:
cp sub sub
cp sub sub/dir
cp sub link_to_sub/dir
In each case, it detects the recursion an iteration later than coreutils,
leading to one level deeper residual than coreutils; but I think the
behavior as better than not doing anything and better than r8517.
You're not obligated to agree -- and if you have a better way I'm
certainly happy to hear about it.
The patch works by recording the source directories which are visited;
because no parent of dest can occur in source. This will only happen with
directories; and since there can't be hardlinks to directories, this
should never cause problems with the other code that uses the
ino_dev_hashtable.
The patch is against 1.4.2; if constructive feedback is provided I'm happy
to port the same for svn.
Issue History
Date Modified Username Field Change
======================================================================
06-29-07 09:16 kiltedknight New Issue
06-29-07 09:16 kiltedknight Status new => assigned
06-29-07 09:16 kiltedknight Assigned To => BusyBox
06-29-07 09:22 vapier Note Added: 0002534
06-29-07 09:28 kiltedknight Note Added: 0002535
06-29-07 09:28 kiltedknight Issue Monitored: kiltedknight
06-29-07 09:36 vapier Note Added: 0002536
06-29-07 09:46 kiltedknight Note Added: 0002537
06-29-07 10:36 vapier Note Added: 0002538
06-30-07 10:21 vda Note Added: 0002540
08-08-07 11:05 kiltedknight Note Added: 0002651
08-21-07 04:58 sh4d0wstr1f3 Note Added: 0002674
08-21-07 05:00 sh4d0wstr1f3 Issue Monitored: sh4d0wstr1f3
08-22-07 12:15 kiltedknight Note Added: 0002676
08-23-07 11:45 kiltedknight Note Added: 0002678
08-23-07 14:23 sh4d0wstr1f3 Note Added: 0002679
======================================================================
More information about the busybox-cvs
mailing list