diff options
| author | Chris Leech <christopher.leech@intel.com> | 2009-10-21 16:28:09 -0700 |
|---|---|---|
| committer | Greg Kroah-Hartman <gregkh@suse.de> | 2010-01-28 15:01:37 -0800 |
| commit | a3d46ca93e4625246a89e62c913ba60742d7bc70 (patch) | |
| tree | 3dc2b262b2c70ca5f449add59e29d773a4425264 /drivers/scsi/libfc/fc_lport.c | |
| parent | 4c40dbe52412690b4a686e4e6c1625f19cb8c6c6 (diff) | |
libfc: fix memory corruption caused by double frees and bad error handling
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 <christopher.leech@intel.com>
Signed-off-by: Robert Love <robert.w.love@intel.com>
Signed-off-by: James Bottomley <James.Bottomley@suse.de>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
Diffstat (limited to 'drivers/scsi/libfc/fc_lport.c')
| -rw-r--r-- | drivers/scsi/libfc/fc_lport.c | 8 |
1 files changed, 4 insertions, 4 deletions
diff --git a/drivers/scsi/libfc/fc_lport.c b/drivers/scsi/libfc/fc_lport.c index 0d19ffa88716..536492ae6a88 100644 --- 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_lport *lport) 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 fc_lport *lport) 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 fc_lport *lport) 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_lport *lport) 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 */ |