[ofa-general] Re: [PATCH 2.6.30.4] Fix for NULL pointer dereference by SRP initiator triggered by a SCSI reset after the SRP connection has been closed

[Bart Van Assche]

On Tue, Aug 4, 2009 at 11:39 PM, Roland Dreier<rdreier at cisco.com> wrote:
>
>  > By the way, Vladislav Bolkhovitin was so kind to inform me that this
>  > issue is not specific to the SRP initiator. For more information, see
>  > also http://thread.gmane.org/gmane.linux.scsi/26166.
>
> I'm not sure I follow this exactly -- the idea is that sg_reset
> generates SCSI commands that are somehow different?  What does the LLD
> have to do to handle them?
>
> Is the problem that we get a command with bogus host_scribble (since SRP
> never saw it before) and so srp_find_req() gets confused?

A search with grep for the text '->eh_device_reset_handler' through
the kernel sources learned me that this handler can be invoked from
the following two functions:
* scsi_try_bus_device_reset() in drivers/scsi/scsi_error.c;
* try_to_reset_cmd_device() in drivers/scsi/libsas/sas_scsi_host.c.

So if the function srp_reset_device() is called, it is called from
scsi_try_bus_device_reset(). This last function can be invoked by
scsi_abort_eh_cmnd(), by scsi_eh_bus_device_reset() or by
scsi_reset_provider(). The last function, scsi_reset_provider(), is
invoked by the sg_reset command by issuing an SG_SCSI_RESET ioctl.

The NULL pointer dereference happens when srp_reset_device() calls
srp_send_tsk_mgmt(target, req, SRP_TSK_LUN_RESET) with
req->scmnd->device == NULL. When the sg_reset command issues an
SG_SCSI_RESET ioctl, scsi_reset_provider() is invoked and allocates an
scmnd structure and sets scmnd->device to NULL. It is this scmnd
structure that is passed to srp_reset_device(). What I'm not sure
about is whether scsi_reset_provider() should set req->scmnd->device
to a non-NULL value or whether srp_send_tsk_mgmt() should be able to
handle the condition req->scmnd->device == NULL.

Bart.