On Thu, 2010-07-29 at 20:07 +0200, Bart Van Assche wrote: > On Thu, Jul 29, 2010 at 7:21 PM, David Dillow <[email protected]> wrote: > > 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.
Hmm, looking at it, it seems you got lucky or unlucky depending on the viewpoint -- you must have either sent the SRP_RSP early on with a tag that matched an unused request (so that req->scmnd would still be NULL) or the tag value was garbage that happened to have a NULL pointer at the correct offset. In any event, it would seem that the initiator is a bit too trusting of the target's returned tag. I hate to add a conditional check to the path, but it does seem preferable to letting a buggy or malicious target induce us to read random kernel memory. > 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 ? I don't think so -- we only allocate them for SCSI commands we send, and look them up based on the tag value in the SRP_RSP. Dave -- 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
