linux-kernel - Re: [PATCH] sr: Ensure disk is revalidated when media changes

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20110401154327.GA6593@mtj.dyndns.org>
Date:	Fri, 1 Apr 2011 17:43:27 +0200
From:	Tejun Heo <tj@...nel.org>
To:	Amit Shah <amit.shah@...hat.com>
Cc:	linux-kernel@...r.kernel.org, Jens Axboe <axboe@...nel.dk>,
	"James E.J. Bottomley" <James.Bottomley@...e.de>,
	linux-scsi@...r.kernel.org, Markus Armbruster <armbru@...hat.com>,
	Stefan Hajnoczi <stefanha@...il.com>
Subject: Re: [PATCH] sr: Ensure disk is revalidated when media changes

Hello,

On Thu, Mar 31, 2011 at 11:50:04PM +0530, Amit Shah wrote:
> After the first GET_EVENT_STATUS_NOTIFICATION command, any new media
> notification is reset by the device.  The following is then noticed:
> 
> 1. insert a CD of a particular size
> 2. mount it
> 3. note /sys/block/sr0/size
> 4. unmount cd
> 5. replace cd with a size greater than previous one
> 6. mount it
> 7. /sys/block/sr0/size isn't updated
> 8. copy all files from cd to somewhere; IO errors will pop up where the
>    files lie beyond previous CD's geometry
> 
> The cause is:
> 
>  cdrom_open()
>      open_for_data()
>          cdo->drive_status() = sr_drive_status()
>              cdrom_get_media_event()
>              --> GPCMD_GET_EVENT_STATUS_NOTIFICATION
>          --> med.media_present is true, return CDS_DISK_OK
>      (success)
>  check_disk_change()
>     ... -> 2nd call to GPCMD_GET_EVENT_STATUS_NOTIFICATION
> 
> at this point the device has already reset the new media event and the
> call to revalidate_disk() in check_disk_change() is never made.

Hmm... I see.  That's something I didn't expect.

> All of this is noticed in a qemu-kvm virtual machine where two CD images
> are created from two files different in size.
...
> diff --git a/drivers/scsi/sr_ioctl.c b/drivers/scsi/sr_ioctl.c
> index 8be3055..0651448 100644
> --- a/drivers/scsi/sr_ioctl.c
> +++ b/drivers/scsi/sr_ioctl.c
> @@ -316,12 +316,19 @@ int sr_drive_status(struct cdrom_device_info *cdi, int slot)
>  		return CDS_DRIVE_NOT_READY;
>  
>  	if (!cdrom_get_media_event(cdi, &med)) {
> -		if (med.media_present)
> +		if (med.media_present) {
> +			/*
> +			 * New media was inserted; ensure disk data is
> +			 * revalidated.
> +			 */
> +			if (cdi->disk->fops->revalidate_disk)
> +				cdi->disk->fops->revalidate_disk(cdi->disk);
>  			return CDS_DISC_OK;
> -		else if (med.door_open)
> +		} else if (med.door_open) {
>  			return CDS_TRAY_OPEN;
> -		else
> +		} else {
>  			return CDS_NO_DISC;
> +		}
>  	}

But I don't think this is the correct place to do it.  The problem
happens because block layer consumes the event but doesn't remember it
when the time for revalidation comes.  It should be done by block
layer, not sr.  Hmmm... looking at the code, the new disk event code
should handle this correctly.  Was 2.6.38 showing the problem too?

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/