Hi Steffen, mit dem Herumdoktern dort tu ich mich insofern schwer, als dass das ganze pl-array auf'm Stack broken ist (Grösse: 8 byte * (36 + 1) = 296 Bytes). Lieber wär mir, pl rauswerfen und dabei alles richtig zu machen :)
Mir fallen auch andere gute Gründe ein, warum das pl-array eine schlechte Idee war: - Layering Violation (dient nur dbf, warum also in zfcp_qdio_int_resp() damit rummachen?) - es wird zu häufig befüllt (weil zfcp_qdio_int_resp() nicht wissen kann, ob zfcp_dbf_hba_def_err() davon Gebrauch macht) - es ist eine Loop und Konvertierung (SBALEs -> pl -> trace, vorher: SBALEs -> trace) mehr als notwendig... und das im schnellen hot-path Interrupt Handler M. On Di, 2012-08-14 at 17:23 +0200, Steffen Maier wrote: > plain text document attachment > (707-zfcp-bounds-checking-for-deferred-error-trace.patch) > From: Steffen Maier <[email protected]> > > The pl vector has scount elements, i.e. pl[scount-1] is the last valid > element. For maximum sized requests, payload->counter == scount after > the last loop iteration. Therefore, do bounds checking first (with > boolean shortcut) to not access the invalid element pl[scount]. > > Do not trust the maximum sbale->scount value from the HBA > but ensure we won't access the pl vector out of our allocated bounds. > > Minor fix for 86a9668a8d29ea711613e1cb37efa68e7c4db564 > "[SCSI] zfcp: support for hardware data router" > > Signed-off-by: Steffen Maier <[email protected]> > Cc: <[email protected]> #3.2+ > --- > drivers/s390/scsi/zfcp_dbf.c | 2 +- > drivers/s390/scsi/zfcp_qdio.c | 4 +++- > 2 files changed, 4 insertions(+), 2 deletions(-) > > --- a/drivers/s390/scsi/zfcp_dbf.c > +++ b/drivers/s390/scsi/zfcp_dbf.c > @@ -191,7 +191,7 @@ void zfcp_dbf_hba_def_err(struct zfcp_ad > length = min((u16)sizeof(struct qdio_buffer), > (u16)ZFCP_DBF_PAY_MAX_REC); > > - while ((char *)pl[payload->counter] && payload->counter < scount) { > + while (payload->counter < scount && (char *)pl[payload->counter]) { > memcpy(payload->data, (char *)pl[payload->counter], length); > debug_event(dbf->pay, 1, payload, zfcp_dbf_plen(length)); > payload->counter++; > --- a/drivers/s390/scsi/zfcp_qdio.c > +++ b/drivers/s390/scsi/zfcp_qdio.c > @@ -113,7 +113,9 @@ static void zfcp_qdio_int_resp(struct cc > if (zfcp_adapter_multi_buffer_active(adapter)) { > sbale = qdio->res_q[idx]->element; > req_id = (u64) sbale->addr; > - scount = sbale->scount + 1; /* incl. signaling SBAL */ > + scount = min(sbale->scount + 1, > + FSF_MAX_SBALS_PER_REQ + 1); > + /* incl. signaling SBAL */ > > for (sbal_no = 0; sbal_no < scount; sbal_no++) { > sbal_idx = (idx + sbal_no) % > -- To unsubscribe from this list: send the line "unsubscribe stable" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
