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

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

[Jon Masters]

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

Signed-off-by: Jon Masters <jcm@jonmasters.org>

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

Re: [PATCH] fix floppy.c to store correct ro/rw status in underlying gendisk

[Evgeny Stambulchik]

Jon Masters wrote:

> 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

Re: [PATCH] fix floppy.c to store correct ro/rw status in underlying gendisk

[Jon Masters]

On 10/31/05, Evgeny Stambulchik <Evgeny.Stambulchik@weizmann.ac.il> wrote:
> Jon Masters wrote:

> > 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.

> 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.

> 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.

> 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.

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

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.

Re: [PATCH] fix floppy.c to store correct ro/rw status in underlying gendisk

[Rob Landley]

On Monday 31 October 2005 05:57, Evgeny Stambulchik wrote:
> Jon Masters wrote:
> > 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

Re: [PATCH] fix floppy.c to store correct ro/rw status in underlying gendisk

[Jon Masters]

On 10/31/05, Rob Landley <rob@landley.net> wrote:
> On Monday 31 October 2005 05:57, Evgeny Stambulchik wrote:
> > Jon Masters wrote:
> > > 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. :)

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.

> 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.

Re: [PATCH] fix floppy.c to store correct ro/rw status in underlying gendisk

[Rob Landley]

On Monday 31 October 2005 20:36, Jon Masters wrote:
> On 10/31/05, Rob Landley <rob@landley.net> wrote:
> > On Monday 31 October 2005 05:57, Evgeny Stambulchik wrote:
> > > Jon Masters wrote:
> > > > 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. :)
>
> 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...)

> > 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...

> Jon.

Rob

Re: [PATCH] fix floppy.c to store correct ro/rw status in underlying gendisk

[Evgeny Stambulchik]

Rob Landley wrote:

> 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

Re: [PATCH] fix floppy.c to store correct ro/rw status in underlying gendisk

[Rob Landley]

On Tuesday 01 November 2005 07:53, Evgeny Stambulchik wrote:
> Rob Landley wrote:
> > 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