Skip to contentSkip to conversation

fix floppy.c to store correct ro/rw status in underlying gendisk

PATCH In review Jon Masters 4 participants 8 messages Oct 29, 2005 – Nov 1, 2005 1 message not in the archive

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 fix floppy.c to store correct ro/rw status in underlying gendisk

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Evgeny Stambulchik found that doing the following always worked:

# mount /dev/fd0 /mnt/floppy/
mount: block device /dev/fd0 is write-protected, mounting read-only
# mount -o remount,rw /mnt/floppy
# echo $?
0

This is the case because the block device /dev/fd0 is writeable but the floppy disk is marked protected. A fix is to simply have floppy_open mark the underlying gendisk policy according to reality (since the VFS doesn't provide a way for do_remount_sb to inquire as to the current device status).

- --- linux-2.6.14/drivers/block/floppy.c 2005-10-28 01:02:08.000000000 +0100
+++ linux-2.6.14_new/drivers/block/floppy.c 2005-10-29
18:14:47.000000000 +0100
@@ -3714,6 +3714,13 @@

                USETF(FD_VERIFY);
        }

+ /* set underlying gendisk policy to reflect real ro/rw status */
+ if (UTESTF(FD_DISK_WRITABLE)) {
+ inode->i_bdev->bd_disk->policy = 0;
+ } else {
+ inode->i_bdev->bd_disk->policy = 1;
+ }
+

        if (UDRS->fd_ref == -1 || (UDRS->fd_ref && (filp->f_flags &
O_EXCL)))
                goto out2;

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.2 (GNU/Linux)
Comment: Using GnuPG with Thunderbird - http://enigmail.mozdev.org

iD8DBQFDY7CAeTyyexZHHxERAvwzAJ0cnuIhiufkEwRK/Kyj0p8URvLAEgCdF38+ k8hBPhPYvtIt3XGKDfkQbeY=
=P0sF
-----END PGP SIGNATURE-----

A message that is not in the archive (sent off-list or not yet received): [email protected]

#2 lore raw
Quoted 2 lines · Jon Masters

Let me know if this fixes it for you - should bomb out now if you try. The error isn't the cleanest (blame mount), but it does fail.

This works fine, thanks! For what it worth, though, mount -o remount,rw says remounting read-only yet still returns success. (Opposite to busybox, which now says "Permission denied" - rather misleading, but at least it fails).

My question is, shouldn't it be implemented at a more generic level? Floppy is just one example. Others are all kind of USB storage, ZIP/Jazz drives, and even normal SCSI disks (which have a RO jumper - at least some of them do).

I got an ancient USB disk on key with a write-protection switch. When I plug it in in the RO mode, everything goes exactly as it was (before your patch) with floppy. Now something interesting:
1. The thingy is plugged in RW and mounted
2. While connected, I switch it to RO
dmesg says:
-> SCSI device sda: 129024 512-byte hdwr sectors (66 MB)
-> sda: Write Protect is on
-> sda: Mode Sense: 03 00 80 00
-> sda: assuming drive cache: write through
3.

# mount -o remount,rw  /mnt
mount: block device /dev/sda1 is write-protected, mounting read-only
# echo $?
0

So it seems there is some layer in bd which does know about RO status (and furthermore it's set by hot events)?

Regards,

Evgeny

#3 lore raw
Quoted 6 lines · Evgeny Stambulchik
Quoted 2 lines · Jon Masters

Let me know if this fixes it for you - should bomb out now if you try. The error isn't the cleanest (blame mount), but it does fail.

This works fine, thanks!

Good. This raises the general problem of devices knowing about their underlying media status.

Quoted 4 lines

For what it worth, though, mount -o remount,rw
says remounting read-only yet still returns success. (Opposite to busybox, which now says "Permission denied" - rather misleading, but at least it fails).

Mount needs some fixing :-) It just tries calling sys_mount and retries ro if that fails, then returns success because the last call worked ok. Or so it would seem from the strace output.

Quoted 1 line

My question is, shouldn't it be implemented at a more generic level?

Yes. Right now, I think the gendisk policy is the best place to store this - but having some generic "check the media status" VFS callback might be a good thing.

Quoted 3 lines

Floppy is just one example. Others are all kind of USB storage, ZIP/Jazz drives, and even normal SCSI disks (which have a RO jumper - at least some of them do).

I'll check, but they probably do something like this in their implementation.

Quoted 11 lines

I got an ancient USB disk on key with a write-protection switch. When I plug it in in the RO mode, everything goes exactly as it was (before your patch) with floppy. Now something interesting:
1. The thingy is plugged in RW and mounted
2. While connected, I switch it to RO
dmesg says:
-> SCSI device sda: 129024 512-byte hdwr sectors (66 MB) -> sda: Write Protect is on
-> sda: Mode Sense: 03 00 80 00
-> sda: assuming drive cache: write through
3.

Quoted 7 lines
# mount -o remount,rw  /mnt
mount: block device /dev/sda1 is write-protected, mounting read-only
# echo $?
0

So it seems there is some layer in bd which does know about RO status (and furthermore it's set by hot events)?

There's a callback for remount requests, perhaps they use that. Bear in mind that USB devices tend to be more communicative than ancient floppy disk drives. I'll have a look into this though - anyone else reading who thinks we need a generic way of checking media read/write status in addition to media change detect?

Jon.

#4 lore raw
In reply to Evgeny Stambulchik
Quoted 8 lines · Evgeny Stambulchik
Quoted 2 lines · Jon Masters

Let me know if this fixes it for you - should bomb out now if you try. The error isn't the cleanest (blame mount), but it does fail.

This works fine, thanks! For what it worth, though, mount -o remount,rw says remounting read-only yet still returns success. (Opposite to busybox, which now says "Permission denied" - rather misleading, but at least it fails).

That sounds like the string translation of EPERM returned by libc's strerror(). (At busybox we're frugal bastards; we don't include text messages when we can get the C library to provide them for us. :)

But yeah, we're sticklers for correct behavior, and only attempt to remount readonly if we get EACCES or EROFS, not _just_ because we attempted a read/write mount and it failed. (And yes, I personally tested this corner case. We haven't started on an automated regression test script for mount yet because running it would require root access, but it's on the todo list as we upgrade the test suite in our Copious Free Time...)

Rob

#5 lore raw
Quoted 5 lines · Rob Landley
Quoted 4 lines · Evgeny Stambulchik
Quoted 2 lines · Jon Masters

Let me know if this fixes it for you - should bomb out now if you try. The error isn't the cleanest (blame mount), but it does fail.

Quoted 8 lines
Quoted 4 lines

This works fine, thanks! For what it worth, though, mount -o remount,rw says remounting read-only yet still returns success. (Opposite to busybox, which now says "Permission denied" - rather misleading, but at least it fails).

That sounds like the string translation of EPERM returned by libc's strerror(). (At busybox we're frugal bastards; we don't include text messages when we can get the C library to provide them for us. :)

Indeed. Reminds me that I should clean up and send a form parser I wrote for busybox to handle multi-part mime form posts in its webserver while I'm at it. That's something it could do with even if it's being frugal.

Quoted 6 lines

But yeah, we're sticklers for correct behavior, and only attempt to remount readonly if we get EACCES or EROFS, not _just_ because we attempted a read/write mount and it failed. (And yes, I personally tested this corner case. We haven't started on an automated regression test script for mount yet because running it would require root access, but it's on the todo list as we upgrade the test suite in our Copious Free Time...)

You looked at running this inside a qemu environment (scratchbox), huh?

Jon.

#6 lore raw
Quoted 11 lines · Jon Masters
Quoted 10 lines · Rob Landley
Quoted 8 lines · Evgeny Stambulchik
Quoted 2 lines · Jon Masters

Let me know if this fixes it for you - should bomb out now if you try. The error isn't the cleanest (blame mount), but it does fail.

This works fine, thanks! For what it worth, though, mount -o remount,rw says remounting read-only yet still returns success. (Opposite to busybox, which now says "Permission denied" - rather misleading, but at least it fails).

Quoted 8 lines
Quoted 3 lines

That sounds like the string translation of EPERM returned by libc's strerror(). (At busybox we're frugal bastards; we don't include text messages when we can get the C library to provide them for us. :)

Indeed. Reminds me that I should clean up and send a form parser I wrote for busybox to handle multi-part mime form posts in its webserver while I'm at it. That's something it could do with even if it's being frugal.

Oh we'll accept a suprising amount of functionality as long as it's the smallest possible implementation and you can easily configure it out if you don't want it.

We're starting to draw the line at servers, though. I personally think our existing httpd/dhcpd/telnetd servers should probably be broken off into a separate package. (We've wandered a touch beyond the mandate of standard command line utilities, in places. An agenda item for version 1.2...)

Quoted 9 lines
Quoted 7 lines

But yeah, we're sticklers for correct behavior, and only attempt to remount readonly if we get EACCES or EROFS, not _just_ because we attempted a read/write mount and it failed. (And yes, I personally tested this corner case. We haven't started on an automated regression test script for mount yet because running it would require root access, but it's on the todo list as we upgrade the test suite in our Copious Free Time...)

You looked at running this inside a qemu environment (scratchbox), huh?

I use User Mode Linux, actually. (I finally downloaded qemu a couple days ago, to test non-x86 builds. But I could only build a crippled version at the time, because ubuntu doesn't have the SDL headers installed either. It's sort of development whack-a-mole...)

No, I'm building my own distro from source code, based on busybox and uClibc. (And when I mean "based on busybox" I mean I'm using it to completely replace bzip2, coreutils, e2fsprogs, file, findutils, gawk, grep, gzip, inetutils, less, modutils, net-tools, patch, procps, sed, shadow, sysklogd, sysvinit, tar, util-linux, and vim.) And yes, this is a complete self-hosting development environment. (By self-hosting I mean capable of rebuilding itself from source.)

You can download the build script here:
http://www.landley.net/code/firmware

Untar it and run build.sh (as a normal user, not as root), and it'll download all the software packages it needs, build User Mode Linux, and run the rest of the build under UML. The result is a self-contained "firmware-uml" you can run to try it out. It's a User Mode Linux kernel with appended squashfs root partion, which automatically loopback mounts itself during initramfs. A bootable version is in the works. (I already have a patched lilo that'll boot it, the hard part is making an installer.)

I've only got two gnu packages left other than development tools: I still need to untangle busybox's command shell mess so I can stop using bash, and I need to add "diff -u" so I can drop diffutils. (And while I'm at it, I need to find another compiler to replace gcc and binutils, and find another make implementation, at least as an _option_. Then I can ask Richard Stallman if the resulting system is still "GNU/Linux", despite not using any GNU packages or libraries even to build it. Ooh, software development to advance a political agenda, a _totally_ unfair thing to pull on RMS...)

And yes, I've found (and fixed) eight gazillion bugs over the past two years working on this system. You want to shake out all the bugs and weird corner cases an implementation of sed/awk/sort? I mean after you've read through the relevant Open Group base standard, line by line taking copious notes, and implemented everything you can see? Try to get it to handle the ./configure of binutils and gcc. Those suckers are _evil_.

And by evil I mean I had to add this to busybox sed to get the darn thing to actually use it:

#define LIE_TO_AUTOCONF

#ifdef LIE_TO_AUTOCONF
    if(argc==2 && !strcmp(argv[1],"--version")) {
        printf("This is not GNU sed version 4.0\n");
        exit(0);
    }

#endif

You guessed it: the configure regexp checks for _GNU_ sed. Grrr...

Quoted 1 line

Jon.

Rob

#7 lore raw
In reply to Rob Landley
Quoted 1 line · Rob Landley

But yeah, we're sticklers for correct behavior, and only attempt to remount readonly if we get EACCES or EROFS, not _just_ because we attempted a read/write mount and it failed.

BTW, busybox doesn't agree to do just mount -o remount,... /mountpoint, it wants also either the device name passed or the entry be present in fstab.

Regards,

Evgeny

#8 lore raw
Quoted 8 lines · Evgeny Stambulchik
Quoted 3 lines · Rob Landley

But yeah, we're sticklers for correct behavior, and only attempt to remount readonly if we get EACCES or EROFS, not _just_ because we attempted a read/write mount and it failed.

BTW, busybox doesn't agree to do just mount -o remount,... /mountpoint, it wants also either the device name passed or the entry be present in fstab.

Did you try the -devel version or 1.01? (I did about an 80% rewrite of this sucker after 1.0, but it hasn't shipped yet. I bounced a 1.1.0-pre1 tarball off of Erik and he said he'd put it up today, meanwhile grab the subversion snapshot.)

Busybox -devel is way ahead of 1.01. Hopefully we'll have a new -stable release by the end of the year...

Rob