| 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
| ||
|
Message-Id: <1213648258.3420.12.camel@localhost.localdomain>
Date: Mon, 16 Jun 2008 15:30:58 -0500
From: James Bottomley <James.Bottomley@...senPartnership.com>
To: Geert Uytterhoeven <Geert.Uytterhoeven@...ycom.com>
Cc: linux-scsi@...r.kernel.org,
Linux Kernel Development <linux-kernel@...r.kernel.org>
Subject: Re: [regression] CD-DA delay needed after insertion
On Mon, 2008-06-16 at 17:05 +0200, Geert Uytterhoeven wrote:
> Hi James,
>
> On Sun, 15 Jun 2008, James Bottomley wrote:
> > On Fri, 2008-06-13 at 13:57 +0200, Geert Uytterhoeven wrote:
> > > We've found another regression in 2.6.25 w.r.t. CD media change on PS3.
> > >
> > > It can easily be reproduced by:
> > >
> > > 1. Inserting an audio CD
> > > 2. Running the following command as soon as the blue CD/DVD/BD drive LED
> > > stops blinking and is lit continuously:
> > >
> > > cdparanoia -Z -q 1-1[:1] /dev/null || echo failed
> > >
> > > On 2.6.25 (and current mainline), you have to wait ca. 10 seconds after
> > > insertion, or it will fail.
> > > On 2.6.24 and older, it just works immediately.
> > >
> > > It does not matter whether
> > > http://git.kernel.org/?p=linux%2Fkernel%2Fgit%2Fjejb%2Fscsi-rc-fixes-2.6.git;a=commitdiff_plain;h=d1daeabf0da5bfa1943272ce508e2ba785730bf0
> > > is applied or not.
> > >
> > > We haven't bisected it yet.
> >
> > There aren't that many commits affecting sr between 2.6.24 and 2.6.25,
> > so I'd bet on the previous culprits.
> >
> > This time, the taxonomy looks like NOT_READY isn't being waited for
> > properly. I'd still tend to blame
> > 210ba1d1724f5c4ed87a2ab1a21ca861a915f734 it's just that this time I
>
> Reverting that commit doesn't fix the problem.
That's a bit of a problem. Particularly as:
> > suspect this to be the problem line:
> >
> > if (sshdr.sense_key == NOT_READY && sshdr.asc == 0x04)
> > return CDS_DISC_OK;
> >
> > Previously the code would have returned CDS_NO_DISC for the not ready
> > case. The CD would have tried to close the door but (and this is the
> > key) it would have waited for the door to close before trying again.
>
> I tried the patch below.
>
> Now the kernel spits out messages every 2 seconds:
>
> | G: CDS_NO_DISC
> | G: CDS_NO_DISC
>
> Insert CD-DA media
>
> Reading from CD using cdparanioa fails
>
> | F: CDS_DISC_OK
> | F: CDS_DISC_OK
> | F: CDS_DISC_OK
> | F: CDS_DISC_OK
> | F: CDS_DISC_OK
That's in the two lines I fingered as the source of the problem ...
however, if reversing the apparently bad commit (that introduces those
lines) doesn't work there's something else wrong.
Could you try altering your F: case to return CDS_DRIVE_NOT_READY, but
if that doesn't work, it's going to take a bit of bisection, I'm
afraid ...
Thanks,
James
> Reading from CD starts to work here
>
> | B: CDS_DISC_OK
> | B: CDS_DISC_OK
>
> I tried changing the return value in the `F' case to CDS_NO_DISC, but that
> doesn't make a difference.
>
> --- a/drivers/scsi/sr_ioctl.c 2008-06-16 16:34:01.000000000 +0200
> +++ b/drivers/scsi/sr_ioctl.c 2008-06-16 16:59:28.000000000 +0200
> @@ -304,25 +304,34 @@ int sr_drive_status(struct cdrom_device_
>
> if (CDSL_CURRENT != slot) {
> /* we have no changer support */
> +printk("A: -EINVAL\n");
> return -EINVAL;
> }
> - if (0 == sr_test_unit_ready(cd->device, &sshdr))
> + if (0 == sr_test_unit_ready(cd->device, &sshdr)) {
> +printk("B: CDS_DISC_OK\n");
> return CDS_DISC_OK;
> + }
>
> if (!cdrom_get_media_event(cdi, &med)) {
> - if (med.media_present)
> + if (med.media_present) {
> +printk("C: CDS_DISC_OK\n");
> return CDS_DISC_OK;
> - else if (med.door_open)
> + } else if (med.door_open) {
> +printk("D: CDS_TRAY_OPEN\n");
> return CDS_TRAY_OPEN;
> - else
> + } else {
> +printk("E: CDS_NO_DISC\n");
> return CDS_NO_DISC;
> + }
> }
>
> /*
> * 0x04 is format in progress .. but there must be a disc present!
> */
> - if (sshdr.sense_key == NOT_READY && sshdr.asc == 0x04)
> + if (sshdr.sense_key == NOT_READY && sshdr.asc == 0x04) {
> +printk("F: CDS_DISC_OK\n");
> return CDS_DISC_OK;
> + }
>
> /*
> * If not using Mt Fuji extended media tray reports,
> @@ -331,11 +340,15 @@ int sr_drive_status(struct cdrom_device_
> */
> if (scsi_sense_valid(&sshdr) &&
> /* 0x3a is medium not present */
> - sshdr.asc == 0x3a)
> + sshdr.asc == 0x3a) {
> +printk("G: CDS_NO_DISC\n");
> return CDS_NO_DISC;
> - else
> + } else {
> +printk("H: CDS_TRAY_OPEN\n");
> return CDS_TRAY_OPEN;
> + }
>
> +printk("I: CDS_DRIVE_NOT_READY\n");
> return CDS_DRIVE_NOT_READY;
> }
>
>
> With kind regards,
>
> Geert Uytterhoeven
> Software Architect
>
> Sony Techsoft Centre
> The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium
>
> Phone: +32 (0)2 700 8453
> Fax: +32 (0)2 700 8622
> E-mail: Geert.Uytterhoeven@...ycom.com
> Internet: http://www.sony-europe.com/
>
> Sony Technology and Software Centre Europe
> A division of Sony Service Centre (Europe) N.V.
> Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
> VAT BE 0413.825.160 · RPR Brussels
> Fortis 293-0376800-10 GEBA-BE-BB
--
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/
Powered by blists - more mailing lists