On Thu, Jul 29, 2010 at 7:21 PM, David Dillow <[email protected]> wrote: > > On Thu, 2010-07-29 at 17:56 +0200, Bart Van Assche wrote: > > If an SRP target sends an invalid SRP_RSP information unit to the SRP > > initiator this can cause a NULL pointer dereference on the initiator system. > > This patch avoids such NULL pointer dereferences and makes sure the SRP > > inititator keeps working. > > While I'm all for increasing the robustness of the initiator, I'm > curious how you would hit this -- I don't see where we would send a SRP > request without an associated SCSI command. > > I'm also not sure you're doing the right thing here -- you seem to skip > the cleanup of the associated SRP request. I suppose that may be handled > for you if/when srp_abort() gets called for a missed SCSI request, but > again, we should never be in the situation where a request doesn't have > a SCSI command. And if srp_abort() doesn't get called, we've leaked a > request and possibly a DMA mapping, no?
This condition was hit accidentally while testing newly added target code that sent an SRP_RSP not associated with an SRP_CMD or SRP_TSK_MGMT information unit. Since I was surprised to find a test of the scmnd pointer in ib_srp.c followed by an unconditional dereference of it, I coded up this patch. The target code that triggered this condition has been fixed by now and even was never committed to its source code repository, so this patch isn't important to me. Regarding leaks: isn't there already a leak in the current ib_srp code if any information unit with another opcode than SRP_RSP is received ? Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
