From 8f550f937e9fdafa5c37e348e214aecec851ef3f Mon Sep 17 00:00:00 2001 From: Chris Leech Date: Wed, 21 Oct 2009 16:28:09 -0700 Subject: [SCSI] libfc: fix memory corruption caused by double frees and bad error handling I was running into several different panics under stress, which I traced down to a few different possible slab corruption issues in error handling paths. I have not yet looked into why these exchange sends fail, but with these fixes my test system is much more stable under stress than before. fc_elsct_send() could fail and either leave the passed in frame intact (failure in fc_ct/els_fill) or the frame could have been freed if the failure was is fc_exch_seq_send(). The caller had no way of knowing, and there was a potential double free in the error handling in fc_fcp_rec(). Make fc_elsct_send() always free the frame before returning, and remove the fc_frame_free() call in fc_fcp_rec(). While fc_exch_seq_send() did always consume the frame, there were double free bugs in the error handling of fc_fcp_cmd_send() and fc_fcp_srr() as well. Numerous calls to error handling routines (fc_disc_error(), fc_lport_error(), fc_rport_error_retry() ) were passing in a frame pointer that had already been freed in the case of an error. I have changed the call sites to pass in a NULL pointer, but there may be more appropriate error codes to use. Question: Why do these error routines take a frame pointer anyway? I understand passing in a pointer encoded error to the response handlers, but the error routines take no action on a valid pointer and should never be called that way. Signed-off-by: Chris Leech Signed-off-by: Robert Love Signed-off-by: James Bottomley --- drivers/scsi/libfc/fc_rport.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) (limited to 'drivers/scsi/libfc/fc_rport.c') diff --git a/drivers/scsi/libfc/fc_rport.c b/drivers/scsi/libfc/fc_rport.c index 1f795e4e474..49abb839a22 100644 --- a/drivers/scsi/libfc/fc_rport.c +++ b/drivers/scsi/libfc/fc_rport.c @@ -632,7 +632,7 @@ static void fc_rport_enter_plogi(struct fc_rport_priv *rdata) if (!lport->tt.elsct_send(lport, rdata->ids.port_id, fp, ELS_PLOGI, fc_rport_plogi_resp, rdata, lport->e_d_tov)) - fc_rport_error_retry(rdata, fp); + fc_rport_error_retry(rdata, NULL); else kref_get(&rdata->kref); } @@ -793,7 +793,7 @@ static void fc_rport_enter_prli(struct fc_rport_priv *rdata) if (!lport->tt.elsct_send(lport, rdata->ids.port_id, fp, ELS_PRLI, fc_rport_prli_resp, rdata, lport->e_d_tov)) - fc_rport_error_retry(rdata, fp); + fc_rport_error_retry(rdata, NULL); else kref_get(&rdata->kref); } @@ -889,7 +889,7 @@ static void fc_rport_enter_rtv(struct fc_rport_priv *rdata) if (!lport->tt.elsct_send(lport, rdata->ids.port_id, fp, ELS_RTV, fc_rport_rtv_resp, rdata, lport->e_d_tov)) - fc_rport_error_retry(rdata, fp); + fc_rport_error_retry(rdata, NULL); else kref_get(&rdata->kref); } @@ -919,7 +919,7 @@ static void fc_rport_enter_logo(struct fc_rport_priv *rdata) if (!lport->tt.elsct_send(lport, rdata->ids.port_id, fp, ELS_LOGO, fc_rport_logo_resp, rdata, lport->e_d_tov)) - fc_rport_error_retry(rdata, fp); + fc_rport_error_retry(rdata, NULL); else kref_get(&rdata->kref); } @@ -1006,7 +1006,7 @@ static void fc_rport_enter_adisc(struct fc_rport_priv *rdata) } if (!lport->tt.elsct_send(lport, rdata->ids.port_id, fp, ELS_ADISC, fc_rport_adisc_resp, rdata, lport->e_d_tov)) - fc_rport_error_retry(rdata, fp); + fc_rport_error_retry(rdata, NULL); else kref_get(&rdata->kref); } -- cgit v1.2.3