Date: Tue, 1 Jul 2008 12:55:11 +0300 (EEST) From: Jaakko Heinonen <jh@saunalahti.fi> To: FreeBSD-gnats-submit@FreeBSD.org Subject: kern/125139: [patch] [ata] bugs in ATAPI CD tray control Message-ID: <200807010955.m619tBYc036257@ws64.jh.dy.fi> Resent-Message-ID: <200807011000.m61A0BZp063107@freefall.freebsd.org>
next in thread | raw e-mail | index | archive | help
>Number: 125139 >Category: kern >Synopsis: [patch] [ata] bugs in ATAPI CD tray control >Confidential: no >Severity: non-critical >Priority: low >Responsible: freebsd-bugs >State: open >Quarter: >Keywords: >Date-Required: >Class: sw-bug >Submitter-Id: current-users >Arrival-Date: Tue Jul 01 10:00:11 UTC 2008 >Closed-Date: >Last-Modified: >Originator: Jaakko Heinonen >Release: FreeBSD 7.0-STABLE / 8.0-CURRENT >Organization: >Environment: System: FreeBSD x 7.0-STABLE FreeBSD 7.0-STABLE #4: Mon Jun 30 19:10:47 EEST 2008 x:X amd64 >Description: There are several bugs in ATAPI driver concerning CD drive tray control: 1) The ATAPI driver locks the tray when a device is opened and unlocks it when a device is closed. If there are several device files associated with a drive the tray unlocks when one of the files closes even if there are other open device files. 2) CDIOCALLOW and CDIOCPREVENT ioctls are allowed if a device is open (for example when disc is mounted) but CDIOCEJECT is not allowed. cdcontrol(1) eject command does first CDIOCALLOW ioctl followed by CDIOCEJECT. With mounted disc this unlocks drive but doesn't eject it. This could be worked around in cdcontrol(1) but possibly there should be consistency when CDIOCALLOW, CDIOCPREVENT and CDIOCEJECT ioctls are allowed. The problem is also described in the PR kern/118779 and in this message: http://lists.freebsd.org/pipermail/freebsd-stable/2007-December/039162.html . 3) CDIOCEJECT is only disallowed if the same device file is in use. However there may be other device files associated with the drive. >How-To-Repeat: (Data CD in the drive) Case 1: # mount -t cd9660 /dev/acd0 /cdrom # # head -c1 /dev/acd0t01 # (Now you can eject the tray from button.) Case 2: # mount -t cd9660 /dev/acd0 /cdrom # cdcontrol eject # (Tray doesn't open but you can eject it from button now.) Case 3: (CD is not mounted) # less -f /dev/acd0t01 (leave /dev/acd0t01 open) # cdcontrol eject # (Tray opens even /dev/acd0t01 is kept open.) >Fix: This patch adds a counter of attached geom providers to ATAPI CDROM device structure. Using that counter we can correctly detect if the drive is in use. The patch also adds the same busy check to CDIOCALLOW and CDIOCPREVENT ioctls that CDIOCEJECT ioctl has. I am not 100% sure if this is the right way but I think that those three ioctls should have consistent checks. The SCSI CD-ROM driver doesn't have any checks for those ioctls. The patch should fix all bugs described above (including kern/118779). --- atapi-cd-tray-control-fixes.diff begins here --- Index: sys/dev/ata/atapi-cd.c =================================================================== --- sys/dev/ata/atapi-cd.c (revision 180023) +++ sys/dev/ata/atapi-cd.c (working copy) @@ -246,11 +246,19 @@ acd_geom_ioctl(struct g_provider *pp, u_ break; case CDIOCALLOW: + if (cdp->gnp != 1 || pp->acr != 1) { + error = EBUSY; + break; + } error = acd_prevent_allow(dev, 0); cdp->flags &= ~F_LOCKED; break; case CDIOCPREVENT: + if (cdp->gnp != 1 || pp->acr != 1) { + error = EBUSY; + break; + } error = acd_prevent_allow(dev, 1); cdp->flags |= F_LOCKED; break; @@ -266,7 +274,7 @@ acd_geom_ioctl(struct g_provider *pp, u_ break; case CDIOCEJECT: - if (pp->acr != 1) { + if (cdp->gnp != 1 || pp->acr != 1) { error = EBUSY; break; } @@ -274,7 +282,7 @@ acd_geom_ioctl(struct g_provider *pp, u_ break; case CDIOCCLOSE: - if (pp->acr != 1) + if (cdp->gnp != 1 || pp->acr != 1) break; error = acd_tray(dev, 1); break; @@ -713,12 +721,16 @@ acd_geom_access(struct g_provider *pp, i ata_free_request(request); if (pp->acr == 0) { + cdp->gnp++; acd_prevent_allow(dev, 1); cdp->flags |= F_LOCKED; acd_read_toc(dev); } - if (dr + pp->acr == 0) { + if (dr + pp->acr == 0) + cdp->gnp--; + + if (cdp->gnp == 0) { acd_prevent_allow(dev, 0); cdp->flags &= ~F_LOCKED; } Index: sys/dev/ata/atapi-cd.h =================================================================== --- sys/dev/ata/atapi-cd.h (revision 180023) +++ sys/dev/ata/atapi-cd.h (working copy) @@ -312,4 +312,6 @@ struct acd_softc { u_int32_t iomax; /* Max I/O request (bytes) */ struct g_geom *gp; /* geom instance */ struct g_provider *pp[MAXTRK+1]; /* providers */ + u_int gnp; /* number of geom providers + attached to the device */ }; --- atapi-cd-tray-control-fixes.diff ends here --- >Release-Note: >Audit-Trail: >Unformatted:
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200807010955.m619tBYc036257>