0003674: copying to a read-only destination gives strange error message (Nokia N800)

BusyBox Bug and Patch Tracking

Viewing Issue Simple Details [ Jump to Notes ]

[ View Advanced ] [ Issue History ] [ Print ]

Category

Severity

Reproducibility

Date Submitted

Last Update

0003674

[BusyBox] New Features

minor

always

06-06-08 03:16

06-07-08 16:50

Reporter

Andre Klapper

View Status

public

Assigned To

BusyBox

Priority

normal

Resolution

fixed

Status

closed

Product Version

1.4.x

Summary

0003674: copying to a read-only destination gives strange error message (Nokia N800)

Description

Forwarding bug report from https://bugs.maemo.org/show_bug.cgi?id=2084 [^] to upstream:

SUMMARY:
using /bin/cp file /media/mmc1 (where user is user and destination is readonly owned by root) gives strange error message

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);

Additional Information

Attached Files

Relationships

Notes
(0008124) vda 06-07-08 16:50	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!).

Issue History
Date Modified	Username	Field	Change
06-06-08 03:16	Andre Klapper	New Issue
06-06-08 03:16	Andre Klapper	Status	new => assigned
06-06-08 03:16	Andre Klapper	Assigned To	=> BusyBox
06-07-08 16:50	vda	Status	assigned => closed
06-07-08 16:50	vda	Note Added: 0008124
06-07-08 16:50	vda	Resolution	open => fixed