| Message ID | 1415179284-19791-1-git-send-email-cristian.stoica@freescale.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Herbert Xu |
| Headers | show |
On Wed, 5 Nov 2014 11:21:24 +0200 Cristian Stoica <cristian.stoica@freescale.com> wrote: > The error code returned by hardware is four bits wide with an expected > zero MSB. A hardware error condition where the error code can get between > 0x8 and 0xf will trigger an out of bound array access on the error > message table. > This patch fixes the invalid array access following such an error and > reports the condition. > > Signed-off-by: Cristian Stoica <cristian.stoica@freescale.com> > --- no v2 change info? All I noticed is the additional string for "queue manager interface", which, without its implementation fn, intoduces an inconsistency wrt NULL checking, so this comment: http://www.mail-archive.com/linux-crypto@vger.kernel.org/msg12105.html still applies. Thanks, Kim -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Kim, Herbert, On 11/05/2014 06:43 PM, Kim Phillips wrote: > On Wed, 5 Nov 2014 11:21:24 +0200 > Cristian Stoica <cristian.stoica@freescale.com> wrote: > >> The error code returned by hardware is four bits wide with an expected >> zero MSB. A hardware error condition where the error code can get between >> 0x8 and 0xf will trigger an out of bound array access on the error >> message table. >> This patch fixes the invalid array access following such an error and >> reports the condition. >> >> Signed-off-by: Cristian Stoica <cristian.stoica@freescale.com> >> --- > no v2 change info? All I noticed is the additional string for > "queue manager interface", which, without its implementation fn, Yes. Thanks for review. That and the updated comment. > intoduces an inconsistency wrt NULL checking, so this comment: No. There is no inconsistency in the handling. Please be more specific. > > http://www.mail-archive.com/linux-crypto@vger.kernel.org/msg12105.html > > still applies. Which comment? You refer to a whole email. My rebutal still applies though and I think your version is inferior wrt handling the issue. Feel free to send a competing patch for review though. Herbert, consider that without a fix one way or another, there is a potential kernel attack vector through the error reporting function of the caam driver. Cristian S. -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Wed, Nov 05, 2014 at 11:21:24AM +0200, Cristian Stoica wrote: > The error code returned by hardware is four bits wide with an expected > zero MSB. A hardware error condition where the error code can get between > 0x8 and 0xf will trigger an out of bound array access on the error > message table. > This patch fixes the invalid array access following such an error and > reports the condition. > > Signed-off-by: Cristian Stoica <cristian.stoica@freescale.com> Patch applied. Thanks!
diff --git a/drivers/crypto/caam/error.c b/drivers/crypto/caam/error.c index 6531054..66d73bf 100644 --- a/drivers/crypto/caam/error.c +++ b/drivers/crypto/caam/error.c @@ -213,27 +213,36 @@ void caam_jr_strstatus(struct device *jrdev, u32 status) void (*report_ssed)(struct device *jrdev, const u32 status, const char *error); const char *error; - } status_src[] = { + } status_src[16] = { { NULL, "No error" }, { NULL, NULL }, { report_ccb_status, "CCB" }, { report_jump_status, "Jump" }, { report_deco_status, "DECO" }, - { NULL, NULL }, + { NULL, "Queue Manager Interface" }, { report_jr_status, "Job Ring" }, { report_cond_code_status, "Condition Code" }, + { NULL, NULL }, + { NULL, NULL }, + { NULL, NULL }, + { NULL, NULL }, + { NULL, NULL }, + { NULL, NULL }, + { NULL, NULL }, + { NULL, NULL }, }; u32 ssrc = status >> JRSTA_SSRC_SHIFT; const char *error = status_src[ssrc].error; /* - * If there is no further error handling function, just - * print the error code, error string and exit. Otherwise - * call the handler function. + * If there is an error handling function, call it to report the error. + * Otherwise print the error source name. */ - if (!status_src[ssrc].report_ssed) - dev_err(jrdev, "%08x: %s: \n", status, status_src[ssrc].error); - else + if (status_src[ssrc].report_ssed) status_src[ssrc].report_ssed(jrdev, status, error); + else if (error) + dev_err(jrdev, "%d: %s\n", ssrc, error); + else + dev_err(jrdev, "%d: unknown error source\n", ssrc); } EXPORT_SYMBOL(caam_jr_strstatus);
The error code returned by hardware is four bits wide with an expected zero MSB. A hardware error condition where the error code can get between 0x8 and 0xf will trigger an out of bound array access on the error message table. This patch fixes the invalid array access following such an error and reports the condition. Signed-off-by: Cristian Stoica <cristian.stoica@freescale.com> --- drivers/crypto/caam/error.c | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-)