Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755399Ab0AZXx7 (ORCPT ); Tue, 26 Jan 2010 18:53:59 -0500 Received: (majordomo@vger.kernel.org) by vger.kernel.org id S1754697Ab0AZXnu (ORCPT ); Tue, 26 Jan 2010 18:43:50 -0500 Received: from kroah.org ([198.145.64.141]:35335 "EHLO coco.kroah.org" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754567Ab0AZXnl (ORCPT ); Tue, 26 Jan 2010 18:43:41 -0500 X-Mailbox-Line: From gregkh@mini.kroah.org Tue Jan 26 15:39:27 2010 Message-Id: <20100126233927.321178985@mini.kroah.org> User-Agent: quilt/0.48-1 Date: Tue, 26 Jan 2010 15:34:15 -0800 From: Greg KH To: linux-kernel@vger.kernel.org, stable@kernel.org Cc: stable-review@kernel.org, torvalds@linux-foundation.org, akpm@linux-foundation.org, alan@lxorguk.ukuu.org.uk, Chris Leech , Robert Love , James Bottomley Subject: [49/98] [SCSI] libfc: fix memory corruption caused by double frees and bad error handling In-Reply-To: <20100126233950.GA5372@kroah.com> Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 6530 Lines: 196 2.6.32-stable review patch. If anyone has any objections, please let us know. ------------------ From: Chris Leech commit 8f550f937e9fdafa5c37e348e214aecec851ef3f upstream. 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 Signed-off-by: Greg Kroah-Hartman --- drivers/scsi/libfc/fc_disc.c | 2 +- drivers/scsi/libfc/fc_elsct.c | 4 +++- drivers/scsi/libfc/fc_fcp.c | 7 ++----- drivers/scsi/libfc/fc_lport.c | 8 ++++---- drivers/scsi/libfc/fc_rport.c | 10 +++++----- 5 files changed, 15 insertions(+), 16 deletions(-) --- a/drivers/scsi/libfc/fc_disc.c +++ b/drivers/scsi/libfc/fc_disc.c @@ -371,7 +371,7 @@ static void fc_disc_gpn_ft_req(struct fc disc, lport->e_d_tov)) return; err: - fc_disc_error(disc, fp); + fc_disc_error(disc, NULL); } /** --- a/drivers/scsi/libfc/fc_elsct.c +++ b/drivers/scsi/libfc/fc_elsct.c @@ -53,8 +53,10 @@ static struct fc_seq *fc_elsct_send(stru did = FC_FID_DIR_SERV; } - if (rc) + if (rc) { + fc_frame_free(fp); return NULL; + } fc_fill_fc_hdr(fp, r_ctl, did, fc_host_port_id(lport->host), fh_type, FC_FC_FIRST_SEQ | FC_FC_END_SEQ | FC_FC_SEQ_INIT, 0); --- a/drivers/scsi/libfc/fc_fcp.c +++ b/drivers/scsi/libfc/fc_fcp.c @@ -1052,7 +1052,6 @@ static int fc_fcp_cmd_send(struct fc_lpo seq = lp->tt.exch_seq_send(lp, fp, resp, fc_fcp_pkt_destroy, fsp, 0); if (!seq) { - fc_frame_free(fp); rc = -1; goto unlock; } @@ -1317,7 +1316,6 @@ static void fc_fcp_rec(struct fc_fcp_pkt fc_fcp_pkt_hold(fsp); /* hold while REC outstanding */ return; } - fc_frame_free(fp); retry: if (fsp->recov_retry++ < FC_MAX_RECOV_RETRY) fc_fcp_timer_set(fsp, FC_SCSI_REC_TOV); @@ -1565,10 +1563,9 @@ static void fc_fcp_srr(struct fc_fcp_pkt seq = lp->tt.exch_seq_send(lp, fp, fc_fcp_srr_resp, NULL, fsp, jiffies_to_msecs(FC_SCSI_REC_TOV)); - if (!seq) { - fc_frame_free(fp); + if (!seq) goto retry; - } + fsp->recov_seq = seq; fsp->xfer_len = offset; fsp->xfer_contig_end = offset; --- a/drivers/scsi/libfc/fc_lport.c +++ b/drivers/scsi/libfc/fc_lport.c @@ -1115,7 +1115,7 @@ static void fc_lport_enter_scr(struct fc if (!lport->tt.elsct_send(lport, FC_FID_FCTRL, fp, ELS_SCR, fc_lport_scr_resp, lport, lport->e_d_tov)) - fc_lport_error(lport, fp); + fc_lport_error(lport, NULL); } /** @@ -1186,7 +1186,7 @@ static void fc_lport_enter_rpn_id(struct if (!lport->tt.elsct_send(lport, FC_FID_DIR_SERV, fp, FC_NS_RPN_ID, fc_lport_rpn_id_resp, lport, lport->e_d_tov)) - fc_lport_error(lport, fp); + fc_lport_error(lport, NULL); } static struct fc_rport_operations fc_lport_rport_ops = { @@ -1340,7 +1340,7 @@ static void fc_lport_enter_logo(struct f if (!lport->tt.elsct_send(lport, FC_FID_FLOGI, fp, ELS_LOGO, fc_lport_logo_resp, lport, lport->e_d_tov)) - fc_lport_error(lport, fp); + fc_lport_error(lport, NULL); } /** @@ -1456,7 +1456,7 @@ void fc_lport_enter_flogi(struct fc_lpor if (!lport->tt.elsct_send(lport, FC_FID_FLOGI, fp, ELS_FLOGI, fc_lport_flogi_resp, lport, lport->e_d_tov)) - fc_lport_error(lport, fp); + fc_lport_error(lport, NULL); } /* Configure a fc_lport */ --- 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 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 f 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 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 f 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 } 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); } -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/