Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 8f550f93 authored by Chris Leech's avatar Chris Leech Committed by James Bottomley
Browse files

[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: default avatarChris Leech <christopher.leech@intel.com>
Signed-off-by: default avatarRobert Love <robert.w.love@intel.com>
Signed-off-by: default avatarJames Bottomley <James.Bottomley@suse.de>
parent b7a727f1

drivers/scsi/libfc/fc_disc.c

+1 −1
Original line number Original line Diff line number Diff line
@@ -371,7 +371,7 @@ static void fc_disc_gpn_ft_req(struct fc_disc *disc) 
				 disc, lport->e_d_tov))

				 disc, lport->e_d_tov))

		return;

		return;

err:

err:

	fc_disc_error(disc, fp);

	fc_disc_error(disc, NULL);

}

}





/**

/**

drivers/scsi/libfc/fc_elsct.c

+3 −1
Original line number Original line Diff line number Diff line
@@ -53,8 +53,10 @@ static struct fc_seq *fc_elsct_send(struct fc_lport *lport, 
		did = FC_FID_DIR_SERV;

		did = FC_FID_DIR_SERV;

	}

	}





	if (rc)

	if (rc) {

		fc_frame_free(fp);

		return NULL;

		return NULL;

	}





	fc_fill_fc_hdr(fp, r_ctl, did, fc_host_port_id(lport->host), fh_type,

	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);

		       FC_FC_FIRST_SEQ | FC_FC_END_SEQ | FC_FC_SEQ_INIT, 0);

drivers/scsi/libfc/fc_fcp.c

+2 −5
Original line number Original line Diff line number Diff line
@@ -1051,7 +1051,6 @@ static int fc_fcp_cmd_send(struct fc_lport *lp, struct fc_fcp_pkt *fsp, 




	seq = lp->tt.exch_seq_send(lp, fp, resp, fc_fcp_pkt_destroy, fsp, 0);

	seq = lp->tt.exch_seq_send(lp, fp, resp, fc_fcp_pkt_destroy, fsp, 0);

	if (!seq) {

	if (!seq) {

		fc_frame_free(fp);

		rc = -1;

		rc = -1;

		goto unlock;

		goto unlock;

	}

	}
@@ -1316,7 +1315,6 @@ static void fc_fcp_rec(struct fc_fcp_pkt *fsp) 
		fc_fcp_pkt_hold(fsp);		/* hold while REC outstanding */

		fc_fcp_pkt_hold(fsp);		/* hold while REC outstanding */

		return;

		return;

	}

	}

	fc_frame_free(fp);

retry:

retry:

	if (fsp->recov_retry++ < FC_MAX_RECOV_RETRY)

	if (fsp->recov_retry++ < FC_MAX_RECOV_RETRY)

		fc_fcp_timer_set(fsp, FC_SCSI_REC_TOV);

		fc_fcp_timer_set(fsp, FC_SCSI_REC_TOV);
@@ -1564,10 +1562,9 @@ static void fc_fcp_srr(struct fc_fcp_pkt *fsp, enum fc_rctl r_ctl, u32 offset) 




	seq = lp->tt.exch_seq_send(lp, fp, fc_fcp_srr_resp, NULL,

	seq = lp->tt.exch_seq_send(lp, fp, fc_fcp_srr_resp, NULL,

				   fsp, jiffies_to_msecs(FC_SCSI_REC_TOV));

				   fsp, jiffies_to_msecs(FC_SCSI_REC_TOV));

	if (!seq) {

	if (!seq)

		fc_frame_free(fp);

		goto retry;

		goto retry;

	}



	fsp->recov_seq = seq;

	fsp->recov_seq = seq;

	fsp->xfer_len = offset;

	fsp->xfer_len = offset;

	fsp->xfer_contig_end = offset;

	fsp->xfer_contig_end = offset;

drivers/scsi/libfc/fc_lport.c

+4 −4
Original line number Original line Diff line number Diff line
@@ -1115,7 +1115,7 @@ static void fc_lport_enter_scr(struct fc_lport *lport) 




	if (!lport->tt.elsct_send(lport, FC_FID_FCTRL, fp, ELS_SCR,

	if (!lport->tt.elsct_send(lport, FC_FID_FCTRL, fp, ELS_SCR,

				  fc_lport_scr_resp, lport, lport->e_d_tov))

				  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 fc_lport *lport) 
	if (!lport->tt.elsct_send(lport, FC_FID_DIR_SERV, fp, FC_NS_RPN_ID,

	if (!lport->tt.elsct_send(lport, FC_FID_DIR_SERV, fp, FC_NS_RPN_ID,

				  fc_lport_rpn_id_resp,

				  fc_lport_rpn_id_resp,

				  lport, lport->e_d_tov))

				  lport, lport->e_d_tov))

		fc_lport_error(lport, fp);

		fc_lport_error(lport, NULL);

}

}





static struct fc_rport_operations fc_lport_rport_ops = {

static struct fc_rport_operations fc_lport_rport_ops = {
@@ -1340,7 +1340,7 @@ static void fc_lport_enter_logo(struct fc_lport *lport) 




	if (!lport->tt.elsct_send(lport, FC_FID_FLOGI, fp, ELS_LOGO,

	if (!lport->tt.elsct_send(lport, FC_FID_FLOGI, fp, ELS_LOGO,

				  fc_lport_logo_resp, lport, lport->e_d_tov))

				  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_lport *lport) 




	if (!lport->tt.elsct_send(lport, FC_FID_FLOGI, fp, ELS_FLOGI,

	if (!lport->tt.elsct_send(lport, FC_FID_FLOGI, fp, ELS_FLOGI,

				  fc_lport_flogi_resp, lport, lport->e_d_tov))

				  fc_lport_flogi_resp, lport, lport->e_d_tov))

		fc_lport_error(lport, fp);

		fc_lport_error(lport, NULL);

}

}





/* Configure a fc_lport */

/* Configure a fc_lport */

drivers/scsi/libfc/fc_rport.c

+5 −5
Original line number Original line Diff line number Diff line
@@ -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,

	if (!lport->tt.elsct_send(lport, rdata->ids.port_id, fp, ELS_PLOGI,

				  fc_rport_plogi_resp, rdata, lport->e_d_tov))

				  fc_rport_plogi_resp, rdata, lport->e_d_tov))

		fc_rport_error_retry(rdata, fp);

		fc_rport_error_retry(rdata, NULL);

	else

	else

		kref_get(&rdata->kref);

		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,

	if (!lport->tt.elsct_send(lport, rdata->ids.port_id, fp, ELS_PRLI,

				  fc_rport_prli_resp, rdata, lport->e_d_tov))

				  fc_rport_prli_resp, rdata, lport->e_d_tov))

		fc_rport_error_retry(rdata, fp);

		fc_rport_error_retry(rdata, NULL);

	else

	else

		kref_get(&rdata->kref);

		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,

	if (!lport->tt.elsct_send(lport, rdata->ids.port_id, fp, ELS_RTV,

				     fc_rport_rtv_resp, rdata, lport->e_d_tov))

				     fc_rport_rtv_resp, rdata, lport->e_d_tov))

		fc_rport_error_retry(rdata, fp);

		fc_rport_error_retry(rdata, NULL);

	else

	else

		kref_get(&rdata->kref);

		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,

	if (!lport->tt.elsct_send(lport, rdata->ids.port_id, fp, ELS_LOGO,

				  fc_rport_logo_resp, rdata, lport->e_d_tov))

				  fc_rport_logo_resp, rdata, lport->e_d_tov))

		fc_rport_error_retry(rdata, fp);

		fc_rport_error_retry(rdata, NULL);

	else

	else

		kref_get(&rdata->kref);

		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,

	if (!lport->tt.elsct_send(lport, rdata->ids.port_id, fp, ELS_ADISC,

				  fc_rport_adisc_resp, rdata, lport->e_d_tov))

				  fc_rport_adisc_resp, rdata, lport->e_d_tov))

		fc_rport_error_retry(rdata, fp);

		fc_rport_error_retry(rdata, NULL);

	else

	else

		kref_get(&rdata->kref);

		kref_get(&rdata->kref);

}

}