On 1/23/2012 10:51 AM, Parikh, Neerav wrote:
-----Original Message-----
From: devel-boun...@open-fcoe.org [mailto:devel-boun...@open-fcoe.org]
On Behalf Of Bhanu Prakash Gollapudi
Sent: Friday, January 20, 2012 4:00 PM
To: devel@open-fcoe.org
Subject: [Open-FCoE] [PATCH] libfc: Fix panic in fc_exch_recv
Adding and removing the host into the zone causes this panic.
BUG: unable to handle kernel NULL pointer dereference at
00000000000000a0
IP: [<ffffffffa0491707>] fc_exch_recv+0xc57/0xe70 [libfc]
Call Trace:
[<ffffffffa050e04b>] bnx2fc_l2_rcv_thread+0x37b/0x430 [bnx2fc]
[<ffffffffa050dcd0>] ? bnx2fc_l2_rcv_thread+0x0/0x430 [bnx2fc]
[<ffffffff81090886>] kthread+0x96/0xa0
[<ffffffff8100c14a>] child_rip+0xa/0x20
[<ffffffff810907f0>] ? kthread+0x0/0xa0
[<ffffffff8100c140>] ? child_rip+0x0/0x20
During fc_exch_reset, the active exchanges are aborted and the exch is
deleted.
As part of processing ABTS response, due to 'ep' being NULL, any access
to ep in
fc_exch_recv_bls() causes this panic. Fixed to return without accessing
ep, if
NULL.
Can you explain where in fc_exch_recv_bls() "ep" is accessed when it's NULL?
As, looking at the code for that function it doesn't seem like there is any
flow that may cause a NULL pointer dereference accessing "ep" if it is NULL.
It accesses ep here:
default:
FC_EXCH_DBG(ep, "BLS rctl %x - %s received",
fh->fh_r_ctl,
fc_exch_rctl_name(fh->fh_r_ctl));
break;
}
In f_ctl, we have both FC_FC_EX_CTX and FC_FC_SEQ_CTX set. This happens
because, with one of the recent changes, fc_exch_reset issues LOGO and
immediately sends ABTS for that LOGO. If the target sees ABTS before
processing LOGO, it can reply with both these bits set in the response.
however, in fc_exch_reset, we would have free'd up the ep, and hence the
code path goes here. Since it is not class 2, it tries printing the
debug message references NULL ep.
For the fc_exch_recv_abts() where the "ep" is passed as it is the function
handles the case where "ep" might be NULL. Also, prior to the change
fc_exch_recv_abts() function sent out a BA_RJT in case "ep" was NULL but after
the proposed change that will not be the case, so there is an impact there that
perhaps changes the behavior for the target mode.
Yes, it did not go into this code path, and you are right. So, we have
to check for NULL ep, case-by-case basis. We should check for NULL ep
for dbg case and before calling fc_exch_abts_resp().
If you do not have objections, I'll send a v2 patch.
thanks.
Thanks,
Neerav
Signed-off-by: Bhanu Prakash Gollapudi<bprak...@broadcom.com>
---
drivers/scsi/libfc/fc_exch.c | 12 ++++++------
1 files changed, 6 insertions(+), 6 deletions(-)
diff --git a/drivers/scsi/libfc/fc_exch.c
b/drivers/scsi/libfc/fc_exch.c
index 4d70d96..23d3075 100644
--- a/drivers/scsi/libfc/fc_exch.c
+++ b/drivers/scsi/libfc/fc_exch.c
@@ -1632,6 +1632,10 @@ static void fc_exch_recv_bls(struct fc_exch_mgr
*mp, struct fc_frame *fp)
ep->esb_stat |= ESB_ST_SEQ_INIT;
spin_unlock_bh(&ep->ex_lock);
}
+ if (!ep) {
+ fc_frame_free(fp);
+ return;
+ }
if (f_ctl& FC_FC_SEQ_CTX) {
/*
* A response to a sequence we initiated.
@@ -1652,10 +1656,7 @@ static void fc_exch_recv_bls(struct fc_exch_mgr
*mp, struct fc_frame *fp)
switch (fh->fh_r_ctl) {
case FC_RCTL_BA_RJT:
case FC_RCTL_BA_ACC:
- if (ep)
- fc_exch_abts_resp(ep, fp);
- else
- fc_frame_free(fp);
+ fc_exch_abts_resp(ep, fp);
break;
case FC_RCTL_BA_ABTS:
fc_exch_recv_abts(ep, fp);
@@ -1665,8 +1666,7 @@ static void fc_exch_recv_bls(struct fc_exch_mgr
*mp, struct fc_frame *fp)
break;
}
}
- if (ep)
- fc_exch_release(ep); /* release hold taken by
fc_exch_find */
+ fc_exch_release(ep); /* release hold taken by fc_exch_find */
}
/**
--
1.7.0.6
_______________________________________________
devel mailing list
devel@open-fcoe.org
https://lists.open-fcoe.org/mailman/listinfo/devel
_______________________________________________
devel mailing list
devel@open-fcoe.org
https://lists.open-fcoe.org/mailman/listinfo/devel
