Re: [PATCH] crypto: talitos - protect against possible null ptr upon error

2009-03-24 Thread Lee Nipper
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

2009-03-24 Thread Kim Phillips
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

2009-03-15 Thread Lee Nipper
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