Re: [PATCH] fix for ide-scsi crash

[Andries . Brouwer]

    From: Willem Riede <wrlk@xxxxxxxxx>

    > -    spin_unlock_irq(cmd->device->host->host_lock);
    > -    (void) ide_do_drive_cmd (drive, rq, ide_end);
    > -    spin_lock_irq(cmd->device->host->host_lock);
    > +    {
    > +        struct Scsi_Host *host = cmd->device->host;
    > +        spin_unlock_irq(host->host_lock);
    > +        (void) ide_do_drive_cmd(drive, rq, ide_end);
    > +        spin_lock_irq(host->host_lock);
    > +    }

    Interesting. So cmd->device->host changed after ide_do_drive_cmd?

It is a race.
ide_do_drive_cmd(..., ide_end) does not wait for I/O to finish,
but when I/O finishes cmd is freed. So, if the device is quick
and the kernel is slow - maybe a timer interrupt in between -
the command can have finished before the spin_lock_irq() is done.

    That may be a problem if the scsi error handler has to spring 
    into action for this command, as it uses that field extensively...

    > -    if (pc) kfree (pc);
    > -    if (rq) kfree (rq);
    > +    kfree (pc);
    > +    kfree (rq);

    So it is OK to rely on the NULL check in kfree?

Yes. Many years ago I regarded it as ugly to do so, but by now
it is common usage.

Andries


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