Bug 2084 - (int-65896) using /bin/cp file /media/mmc1 (where the user is user and the destination is readonly owned by root) gives a strange error message
(int-65896)
: using /bin/cp file /media/mmc1 (where the user is user and the destination is...
Status: RESOLVED FIXED
Product: Core
Busybox
: 4.0
: N800 Maemo
: Low normal (vote)
: 5.0-alpha
Assigned To: unassigned
: busybox-bugs
: http://bugs.busybox.net/view.php?id=3674
:
:
:
  Show dependency tree
 
Reported: 2007-10-09 14:48 UTC by timeless
Modified: 2009-03-02 13:36 UTC (History)
2 users (show)

See Also:


Attachments


Note

You need to log in before you can comment on or make changes to this bug.


Description timeless (reporter) 2007-10-09 14:48:47 UTC
EXPECTED OUTCOME:
an error indicating that the target destination is read only would be greatly
appreciated

ACTUAL OUTCOME:
busybox 1.4.1:
  16                 fprintf(stderr, "'%s' exists\n", dest);

busybox 1.6.1:
  50                 bb_perror_msg("cannot remove '%s'", dest);

STEPS TO REPRODUCE THE PROBLEM:
1. be sure there is no card in the external mmc slot
2. open xterminal
3. cp /etc/passwd /media/mmc1

OTHER COMMENTS:
the code looks something like this:

busybox: /libbb/copy_file.c

  56 /* Return:
  57  * -1 error, copy not made
  58  *  0 copy is made or user answered "no" in interactive mode
  59  *    (failures to preserve mode/owner/times are not reported in exit
code)
  60  */
  61 int copy_file(const char *source, const char *dest, int flags)
...
  80         if (lstat(dest, &dest_stat) < 0) {
  81                 if (errno != ENOENT) {

not quite sure if this is the right point, there's definitely one instance of
stat64 returning ENOENT according to strace.

...

 193         } else if (S_ISREG(source_stat.st_mode)
 194          /* Huh? DEREF uses stat, which never returns links! */
 195          /* || (FLAGS_DEREF && S_ISLNK(source_stat.st_mode)) */
 196         ) {
...

 225 #if DO_POSIX_CP  /* POSIX way (a security problem versus symlink
attacks!): */
 226                 dst_fd = open(dest, (flags & FILEUTILS_INTERACTIVE)
 227                                 ? O_WRONLY|O_CREAT|O_EXCL
 228                                 : O_WRONLY|O_CREAT|O_TRUNC,
source_stat.st_mode);
 229 #else  /* safe way: */
 230                 dst_fd = open(dest, O_WRONLY|O_CREAT|O_EXCL,
source_stat.st_mode);
 231 #endif

one of these two opens returns -1 EACCES

 232                 if (dst_fd == -1) {

at this point, we know the file doesn't exist and can't be written to

 233                         ovr = ask_and_unlink(dest, flags);

so we try to delete it!

 234                         if (ovr <= 0) {
 235                                 close(src_fd);
 236                                 return ovr;

busybox: libbb/copy_file.c

  26 static int ask_and_unlink(const char *dest, int flags)
...
  49         if (unlink(dest) < 0) {
  50                 bb_perror_msg("cannot remove '%s'", dest);
  51                 return -1; // error

it'd be greatly appreciated if the error message was something indicating that
the destination is readonly instead of a message talking about not being able
to delete the file.

note, on the version of busybox i have on my n800, i actually get:

/tmp $ cp x.png /media/mmc1/
'/media/mmc1/x.png' exists
/tmp $

the sources i'm reading are from busybox 1.6.1, the device has busybox 1.4.1

that code looks like this:

busybox: libbb/copy_file.c
  13 static int retry_overwrite(const char *dest, int flags)
  14 {
  15         if (!(flags & (FILEUTILS_FORCE|FILEUTILS_INTERACTIVE))) {
  16                 fprintf(stderr, "'%s' exists\n", dest);



  31 int copy_file(const char *source, const char *dest, int flags)

 179                 // POSIX: if exists and -i, ask (w/o -i assume yes).
 180                 // Then open w/o EXCL.
 181                 // If open still fails and -f, try unlink, then try open
again.
 182                 // Result: a mess:
 183                 // If dest is a softlink, we overwrite softlink's
destination!
 184                 // (or fail, if it points to dir/nonexistent
location/etc).
 185                 // This is strange, but POSIX-correct.
 186                 // coreutils cp has --remove-destination to override
this...
 187                 dst_fd = open(dest, (flags & FILEUTILS_INTERACTIVE)
 188                                 ? O_WRONLY|O_CREAT|O_TRUNC|O_EXCL
 189                                 : O_WRONLY|O_CREAT|O_TRUNC,
source_stat.st_mode);
 190                 if (dst_fd == -1) {
 191                         // We would not do POSIX insanity. -i asks,
 192                         // then _unlinks_ the offender. Presto.
 193                         // Or else we will end up having 3 open()s!
 194                         ovr = retry_overwrite(dest, flags);
Comment 1 Andre Klapper maemo.org 2008-06-04 12:38:03 UTC
the internal ticket is closed as WONTFIX:

"Moreover, since it is not a usecase for the enduser, I do not think, that we
should spend time on it. Closing as WONTFIX."
Comment 2 Andre Klapper maemo.org 2008-06-06 13:21:03 UTC
Forwarded the bug report to the upstream busybox maintainers at
http://bugs.busybox.net/view.php?id=3674 .
Comment 3 Andre Klapper maemo.org 2008-06-09 11:45:50 UTC
Forwarding from the upstream ticket at
http://busybox.net/bugs/view.php?id=3674 :

"Current busybox says:

# ./busybox cp busybox /
cp: cannot create '/busybox': Read-only file system
# ./busybox cp busybox /init
cp: cannot create '/init': Read-only file system
# ./busybox cp busybox /bin
cp: cannot create '/bin/busybox': File exists

Last message is somewhat suboptimal, but better than what we had before
("cannot remove - file doesn't exist" - doh!). "


For me it's fine to close this bug report as FIXED (I consider the upstream
code as fixed) and wait for Nokia to ship an updated busybox version (if I
remember correctly this will not happen for Diablo, but for Fremantle). Please
reopen if you disagree.
Comment 4 Andre Klapper maemo.org 2008-08-25 10:56:50 UTC
(Correcting deprecated target milestone)