On Wed, 2015-05-27 at 15:12 +0200, Bart Van Assche wrote:
> On 05/27/15 03:06, vasu.dev@xxxxxxxxxxxxxxx wrote:
> > I'm trying to look back but may be you can help me why we added
> > lport_recv() along adding of fc_invoke_resp() by you in commit 7030fd62.
> >
> > I'm digging at history since it has been while since you added
> > fc_invoke_resp() logic. Looks like you had lport_recv() called only when
> > fc_invoke_resp() was not done calling upper later fcp handler but then
> > you added fp free there and thats where this bug got introduced but not
> > sure lport_recv() calling was for a reason which you had it left along
> > adding fc_invoke_resp().
>
> Hello Vasu,
>
> The lport_recv() call wasn't added by me. Anyway, how about replacing patch
> 3/4 with the patch below ?
Looks good with the patch below while keeping lport_recv() as it was
before for incoming ELS requests.
Thanks Bart for quick update.
Vasu
>
> Thanks,
>
> Bart.
>
>
> [PATCH] libfc: Fix fc_exch_recv_req() error path
>
> Due to patch "libfc: Do not invoke the response handler after
> fc_exch_done()" (commit ID f65fc5770adc) the lport_recv() call
> in fc_exch_recv_req() is passed a dangling pointer. Avoid this
> by moving the fc_frame_free() call from fc_invoke_resp() to its
> callers. This patch fixes the following crash:
>
> general protection fault: 0000 [#3] PREEMPT SMP
> RIP: fc_lport_recv_req+0x72/0x280 [libfc]
> Call Trace:
> fc_exch_recv+0x642/0xde0 [libfc]
> fcoe_percpu_receive_thread+0x46a/0x5ed [fcoe]
> kthread+0x10a/0x120
> ret_from_fork+0x42/0x70
>
> Signed-off-by: Bart Van Assche <bart.vanassche@xxxxxxxxxxx>
> Cc: Vasu Dev <vasu.dev@xxxxxxxxx>
> Cc: stable <stable@xxxxxxxxxxxxxxx>
> ---
> drivers/scsi/libfc/fc_exch.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/libfc/fc_exch.c b/drivers/scsi/libfc/fc_exch.c
> index 1b3a094..30f9ef0 100644
> --- a/drivers/scsi/libfc/fc_exch.c
> +++ b/drivers/scsi/libfc/fc_exch.c
> @@ -733,8 +733,6 @@ static bool fc_invoke_resp(struct fc_exch *ep, struct fc_seq *sp,
> if (resp) {
> resp(sp, fp, arg);
> res = true;
> - } else if (!IS_ERR(fp)) {
> - fc_frame_free(fp);
> }
>
> spin_lock_bh(&ep->ex_lock);
> @@ -1596,7 +1594,8 @@ static void fc_exch_recv_seq_resp(struct fc_exch_mgr *mp, struct fc_frame *fp)
> * If new exch resp handler is valid then call that
> * first.
> */
> - fc_invoke_resp(ep, sp, fp);
> + if (!fc_invoke_resp(ep, sp, fp))
> + fc_frame_free(fp);
>
> fc_exch_release(ep);
> return;
> @@ -1695,7 +1694,8 @@ static void fc_exch_abts_resp(struct fc_exch *ep, struct fc_frame *fp)
> fc_exch_hold(ep);
> if (!rc)
> fc_exch_delete(ep);
> - fc_invoke_resp(ep, sp, fp);
> + if (!fc_invoke_resp(ep, sp, fp))
> + fc_frame_free(fp);
> if (has_rec)
> fc_exch_timer_set(ep, ep->r_a_tov);
> fc_exch_release(ep);
--
To unsubscribe from this list: send the line "unsubscribe target-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
