Possible use after free in scsi_put_command()?

Possible use after free in scsi_put_command()?

[Maurizio Lombardi]

Hi Hannes,

I've a question regarding the asynchronous scsi abort handler,
look at the scsi_put_command() function:

void scsi_put_command(struct scsi_cmnd *cmd)
{
        unsigned long flags;
	[...]
        cancel_delayed_work(&cmd->abort_work);
        __scsi_put_command(cmd->device->host, cmd);
}

cancel_delayed_work() may return while the abort handler is still running,
the problem is that __scsi_put_command() frees the cmd pointer that
is still used by the abort handler.

Is it correct? Isn't safer to use cancel_delayed_work_sync() here?

Thanks,
Maurizio Lombardi

Re: Possible use after free in scsi_put_command()?

[Bart Van Assche]

On 06/24/14 17:07, Maurizio Lombardi wrote:
> I've a question regarding the asynchronous scsi abort handler,
> look at the scsi_put_command() function:
> 
> void scsi_put_command(struct scsi_cmnd *cmd)
> {
>         unsigned long flags;
> 	[...]
>         cancel_delayed_work(&cmd->abort_work);
>         __scsi_put_command(cmd->device->host, cmd);
> }
> 
> cancel_delayed_work() may return while the abort handler is still running,
> the problem is that __scsi_put_command() frees the cmd pointer that
> is still used by the abort handler.
> 
> Is it correct? Isn't safer to use cancel_delayed_work_sync() here?

Hello Maurizio,

I agree that that cancel_delayed_work() call is confusing. Hence
"[PATCH] Remove two cancel_delayed_work() calls from the mid-layer"
(http://thread.gmane.org/gmane.linux.scsi/91027). Had you already
noticed that patch ?

Best regards,

Bart.

Re: Possible use after free in scsi_put_command()?

[Maurizio Lombardi]

Hi Bart,

On 06/25/2014 08:52 AM, Bart Van Assche wrote:
> Hello Maurizio,
> 
> I agree that that cancel_delayed_work() call is confusing. Hence
> "[PATCH] Remove two cancel_delayed_work() calls from the mid-layer"
> (http://thread.gmane.org/gmane.linux.scsi/91027). Had you already
> noticed that patch ?
> 

No, I missed it, thanks for pointing it out to me.

Thanks,
Maurizio Lombardi

Re: Possible use after free in scsi_put_command()?

[Bart Van Assche]

On 06/25/14 09:03, Maurizio Lombardi wrote:
> On 06/25/2014 08:52 AM, Bart Van Assche wrote:
>> I agree that that cancel_delayed_work() call is confusing. Hence
>> "[PATCH] Remove two cancel_delayed_work() calls from the mid-layer"
>> (http://thread.gmane.org/gmane.linux.scsi/91027). Had you already
>> noticed that patch ?
> 
> No, I missed it, thanks for pointing it out to me.

Hello James and Christoph,

Is it correct that the patch mentioned above has not yet been queued up
? Has it already been decided in which kernel version that patch should
be included ?

Thanks,

Bart.

Re: Possible use after free in scsi_put_command()?

[Christoph Hellwig]

On Wed, Jun 25, 2014 at 09:45:07AM +0200, Bart Van Assche wrote:
> Hello James and Christoph,
> 
> Is it correct that the patch mentioned above has not yet been queued up
> ? Has it already been decided in which kernel version that patch should
> be included ?

I've got it in my local pending queue for 3.17.  I'll push that one out
soon.