On Wed, 2015-05-27 at 15:12 +0200, Bart Van Assche wrote:
> On 05/27/15 03:06, [email protected] 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 <[email protected]>
> Cc: Vasu Dev <[email protected]>
> Cc: stable <[email protected]>
> ---
>  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);


_______________________________________________
fcoe-devel mailing list
[email protected]
http://lists.open-fcoe.org/mailman/listinfo/fcoe-devel

Reply via email to