Resend: PATCH: Stop 2.6.12rc rmmod from being able to destroy CD hardware

Resend: PATCH: Stop 2.6.12rc rmmod from being able to destroy CD hardware

[Alan Cox]

On an rmmod the cdrom layer when used with ide-cd issues a cache flush
atapi command to devices including those that do not support it.
According to Jens earlier discussion this isn't merely a minor glitch
but can destroy some CD hardware due to firmware bugs in the drive (as
per the Mandrake incident)

The IDE CD layer uses a mask of unsupported features, this means that
because ide-cd doesn't know about MRW writables it doesn't set the
relevant bit for non writables and harm can occur.

The simple fix is attached, making the driver start from ~0 and mask
bits the other direction would longer term be safer.

diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.12rc3/drivers/ide/ide-cd.c linux-2.6.12rc3-minoride/drivers/ide/ide-cd.c
--- linux.vanilla-2.6.12rc3/drivers/ide/ide-cd.c	2005-04-27 16:01:29.000000000 +0100
+++ linux-2.6.12rc3-minoride/drivers/ide/ide-cd.c	2005-05-01 14:09:35.000000000 +0100
@@ -2860,6 +2922,9 @@
 		devinfo->mask |= CDC_CLOSE_TRAY;
 	if (!CDROM_CONFIG_FLAGS(drive)->mo_drive)
 		devinfo->mask |= CDC_MO_DRIVE;
+		
+	/* We must have this masked unless a drive definitely handles it */
+	devinfo->mask |= CDC_MRW_W;
 
 	devinfo->disk = info->disk;
 	return register_cdrom(devinfo);

Re: Resend: PATCH: Stop 2.6.12rc rmmod from being able to destroy CD hardware

[Bartlomiej Zolnierkiewicz]

I believe that this was fixed differently almost moth ago by Jens:
http://kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commitdiff;h=7da21a02b3587157bd43910ea6d4c76661228ebb;hp=76530da1a9e8ce05963b1f49a098eddc6ec6c534

It would be nice to have features mask in ide-cd.c done right
but this sounds like 2.6.13 material.

On 5/27/05, Alan Cox <alan@lxorguk.ukuu.org.uk> wrote:
> On an rmmod the cdrom layer when used with ide-cd issues a cache flush
> atapi command to devices including those that do not support it.
> According to Jens earlier discussion this isn't merely a minor glitch
> but can destroy some CD hardware due to firmware bugs in the drive (as
> per the Mandrake incident)
> 
> The IDE CD layer uses a mask of unsupported features, this means that
> because ide-cd doesn't know about MRW writables it doesn't set the
> relevant bit for non writables and harm can occur.
> 
> The simple fix is attached, making the driver start from ~0 and mask
> bits the other direction would longer term be safer.
> 
> diff -u --new-file --recursive --exclude-from /usr/src/exclude linux.vanilla-2.6.12rc3/drivers/ide/ide-cd.c linux-2.6.12rc3-minoride/drivers/ide/ide-cd.c
> --- linux.vanilla-2.6.12rc3/drivers/ide/ide-cd.c        2005-04-27 16:01:29.000000000 +0100
> +++ linux-2.6.12rc3-minoride/drivers/ide/ide-cd.c       2005-05-01 14:09:35.000000000 +0100
> @@ -2860,6 +2922,9 @@
>                 devinfo->mask |= CDC_CLOSE_TRAY;
>         if (!CDROM_CONFIG_FLAGS(drive)->mo_drive)
>                 devinfo->mask |= CDC_MO_DRIVE;
> +
> +       /* We must have this masked unless a drive definitely handles it */
> +       devinfo->mask |= CDC_MRW_W;
> 
>         devinfo->disk = info->disk;
>         return register_cdrom(devinfo);

Re: Resend: PATCH: Stop 2.6.12rc rmmod from being able to destroy CD hardware

[Linus Torvalds]

On Fri, 27 May 2005, Alan Cox wrote:
> 
> The simple fix is attached, making the driver start from ~0 and mask
> bits the other direction would longer term be safer.

As per Bartlomiej, I've not applied this, since the "don't do it unless
you've written to it" fix from Jens went in. I assume CD-RW's know how to
cache flush..

That said, it does sound like the capabilities bitmap should be changed
around to be the other way (either by starting at ~0, or, if the changes
aren't too invasive, just change the semantics of it to be the sane way
around ("drive can do this" rather than "drive can _not_ do this").

		Linus

Re: Resend: PATCH: Stop 2.6.12rc rmmod from being able to destroy CD hardware

[Jens Axboe]

On Fri, May 27 2005, Linus Torvalds wrote:
> 
> 
> On Fri, 27 May 2005, Alan Cox wrote:
> > 
> > The simple fix is attached, making the driver start from ~0 and mask
> > bits the other direction would longer term be safer.
> 
> As per Bartlomiej, I've not applied this, since the "don't do it unless
> you've written to it" fix from Jens went in. I assume CD-RW's know how to
> cache flush..
> 
> That said, it does sound like the capabilities bitmap should be changed
> around to be the other way (either by starting at ~0, or, if the changes
> aren't too invasive, just change the semantics of it to be the sane way
> around ("drive can do this" rather than "drive can _not_ do this").

They should, it's always annoyed me because it's counter intuitive.

-- 
Jens Axboe