linux-kernel - Re: [PATCH resend 3/6] cdrom: wait for tray to close

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]

Date:   Wed, 31 Jan 2018 19:20:27 +0100
From:   Michal Suchánek <msuchanek@...e.de>
To:     Bart Van Assche <Bart.VanAssche@....com>
Cc:     "corbet@....net" <corbet@....net>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-ide@...r.kernel.org" <linux-ide@...r.kernel.org>,
        "keescook@...omium.org" <keescook@...omium.org>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "martin.petersen@...cle.com" <martin.petersen@...cle.com>,
        "axboe@...nel.dk" <axboe@...nel.dk>,
        "linux-scsi@...r.kernel.org" <linux-scsi@...r.kernel.org>,
        "kstewart@...uxfoundation.org" <kstewart@...uxfoundation.org>,
        "pombredanne@...b.com" <pombredanne@...b.com>,
        "bp@...en8.de" <bp@...en8.de>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "jejb@...ux.vnet.ibm.com" <jejb@...ux.vnet.ibm.com>,
        "christophe.jaillet@...adoo.fr" <christophe.jaillet@...adoo.fr>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "linux-doc@...r.kernel.org" <linux-doc@...r.kernel.org>,
        "tim@...erelk.net" <tim@...erelk.net>
Subject: Re: [PATCH resend 3/6] cdrom: wait for tray to close

On Mon, 29 Jan 2018 17:05:47 +0000
Bart Van Assche <Bart.VanAssche@....com> wrote:

> On Fri, 2018-01-26 at 17:58 +0100, Michal Suchanek wrote:
> > +static int cdrom_tray_close(struct cdrom_device_info *cdi)
> > +{
> > +	int ret;
> > +
> > +	ret = cdi->ops->tray_move(cdi, 0);
> > +	if (ret || !cdi->ops->drive_status)
> > +		return ret;
> > +
> > +	return poll_event_interruptible(CDS_TRAY_OPEN !=
> > +			cdi->ops->drive_status(cdi, CDSL_CURRENT),
> > 500); +}
> > +
> >  static
> >  int open_for_common(struct cdrom_device_info *cdi, tracktype
> > *tracks) {
> > @@ -1048,7 +1062,9 @@ int open_for_common(struct cdrom_device_info
> > *cdi, tracktype *tracks) if (CDROM_CAN(CDC_CLOSE_TRAY) &&
> >  			    cdi->options & CDO_AUTO_CLOSE) {
> >  				cd_dbg(CD_OPEN, "trying to close
> > the tray\n");
> > -				ret = cdo->tray_move(cdi, 0);
> > +				ret = cdrom_tray_close(cdi);
> > +				if (ret == -ERESTARTSYS)
> > +					return ret;
> >  				if (ret) {
> >  					cd_dbg(CD_OPEN, "bummer.
> > tried to close the tray but failed.\n"); /* Ignore the error from
> > the low @@ -2312,7 +2328,8 @@ static int
> > cdrom_ioctl_closetray(struct cdrom_device_info *cdi) 
> >  	if (!CDROM_CAN(CDC_CLOSE_TRAY))
> >  		return -ENOSYS;
> > -	return cdi->ops->tray_move(cdi, 0);
> > +
> > +	return cdrom_tray_close(cdi);
> >  }  
> 
> So this patch changes code that does not wait into code that
> potentially waits forever? Sorry but I don't think that's ideal.
> Please make sure that after a certain time (a few seconds?) the loop
> finishes.

The problem is that few seconds does not cut it. We are waiting for a
mechanical tray or CD changer to move. On non-broken hardware the tray
either closes or an error is reported within a few seconds. For the
timeout to not race with the event we are waiting for it must be much
longer, though.

Also note that this code is only invoked when the user specifically
requested that the tray gets closed automatically which is off by
default.

Thanks

Michal