Re: [PATCH] crypto: talitos - protect against possible null ptr upon error
On Tue, Mar 24, 2009 at 16:23, Kim Phillips wrote: > On Sun, 15 Mar 2009 20:21:47 -0500 > Lee Nipper wrote: > >> Added test for null descriptor returned from current_desc. >> Also removed the diagnostic from current_desc, >> and added one instead in talitos_error >> to report an EU error without finding the descriptor. > > Hi Lee, thanks for this - I'm not clear on how the channel error gets > triggered, points to an EU, and all of this without a referencing a > current descriptor. I don't remember the exact case when I observed it, but I did. In any case, it seemed the code needed the protection against the failed look up case (prevent null pointer use). > How did you run into this case? I had a misconfigured descriptor (I think), and the problem case is not in the code. > Assuming one does > get propagated back, what error status does the callback receive? Ug. If I were still at Freescale and had access to my logs, I could answer with detail, But that's not the case. > afaict, interrupt mitigation should not be intervening in this case. > That's right. The interrupt with error status was occurring, and it was handling it during the isr.. Thanks, Lee -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] crypto: talitos - protect against possible null ptr upon error
On Sun, 15 Mar 2009 20:21:47 -0500 Lee Nipper wrote: > Added test for null descriptor returned from current_desc. > Also removed the diagnostic from current_desc, > and added one instead in talitos_error > to report an EU error without finding the descriptor. Hi Lee, thanks for this - I'm not clear on how the channel error gets triggered, points to an EU, and all of this without a referencing a current descriptor. How did you run into this case? Assuming one does get propagated back, what error status does the callback receive? afaict, interrupt mitigation should not be intervening in this case. Kim -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] crypto: talitos - protect against possible null ptr upon error
Added test for null descriptor returned from current_desc. Also removed the diagnostic from current_desc, and added one instead in talitos_error to report an EU error without finding the descriptor. Signed-off-by: Lee Nipper --- drivers/crypto/talitos.c | 14 +- 1 files changed, 9 insertions(+), 5 deletions(-) diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c index a3918c1..d853530 100644 --- a/drivers/crypto/talitos.c +++ b/drivers/crypto/talitos.c @@ -401,10 +401,8 @@ static struct talitos_desc *current_desc(struct device *dev, int ch) while (priv->fifo[ch][tail].dma_desc != cur_desc) { tail = (tail + 1) & (priv->fifo_len - 1); - if (tail == priv->tail[ch]) { - dev_err(dev, "couldn't locate current descriptor\n"); + if (tail == priv->tail[ch]) return NULL; - } } return priv->fifo[ch][tail].desc; @@ -523,8 +521,14 @@ static void talitos_error(unsigned long data, u32 isr, u32 isr_lo) dev_err(dev, "illegal descriptor header error\n"); if (v_lo & TALITOS_CCPSR_LO_IEU) dev_err(dev, "invalid execution unit error\n"); - if (v_lo & TALITOS_CCPSR_LO_EU) - report_eu_error(dev, ch, current_desc(dev, ch)); + if (v_lo & TALITOS_CCPSR_LO_EU) { + struct talitos_desc *desc = current_desc(dev, ch); + if (desc) + report_eu_error(dev, ch, desc); + else + dev_err(dev, + "EU error, but no descriptor found.\n"); + } if (v_lo & TALITOS_CCPSR_LO_GB) dev_err(dev, "gather boundary error\n"); if (v_lo & TALITOS_CCPSR_LO_GRL) -- 1.5.6.3 -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html