| Message ID | 20221222162513.4021928-1-u.kleine-koenig@pengutronix.de (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | crypto: caam - Prevent fortify error | expand |
On Thu, Dec 22, 2022 at 05:25:13PM +0100, Uwe Kleine-König wrote: > When compiling arm64 allmodconfig with gcc 10.2.1 I get > > drivers/crypto/caam/desc_constr.h: In function ‘append_data.constprop’: > include/linux/fortify-string.h:57:29: error: argument 2 null where non-null expected [-Werror=nonnull] > > Fix this by skipping the memcpy if data is NULL and add a BUG_ON instead > that triggers on a problematic call that is now prevented to trigger. > After data == NULL && len != 0 is known to be false, logically > > if (len) > memcpy(...) > > could be enough to know that memcpy is not called with dest=NULL, but > gcc doesn't seem smart enough for that conclusion. gcc 12 doesn't have a > problem with the original code. > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > --- > drivers/crypto/caam/desc_constr.h | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) Does this patch fix your problem? https://lore.kernel.org/all/Y4mHjKXnF%2F4Pfw5I@gondor.apana.org.au/ If not please send me your kconfig file. Thanks,
On Fri, Dec 23, 2022 at 02:29:52PM +0800, Herbert Xu wrote: > On Thu, Dec 22, 2022 at 05:25:13PM +0100, Uwe Kleine-König wrote: > > When compiling arm64 allmodconfig with gcc 10.2.1 I get > > > > drivers/crypto/caam/desc_constr.h: In function ‘append_data.constprop’: > > include/linux/fortify-string.h:57:29: error: argument 2 null where non-null expected [-Werror=nonnull] > > > > Fix this by skipping the memcpy if data is NULL and add a BUG_ON instead > > that triggers on a problematic call that is now prevented to trigger. > > After data == NULL && len != 0 is known to be false, logically > > > > if (len) > > memcpy(...) > > > > could be enough to know that memcpy is not called with dest=NULL, but > > gcc doesn't seem smart enough for that conclusion. gcc 12 doesn't have a > > problem with the original code. > > > > Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> > > --- > > drivers/crypto/caam/desc_constr.h | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > Does this patch fix your problem? > > https://lore.kernel.org/all/Y4mHjKXnF%2F4Pfw5I@gondor.apana.org.au/ Using if (data && len) fixes it (that's the patch that b4 picks for the above message id :-\), if (!IS_ENABLED(CONFIG_CRYPTO_DEV_FSL_CAAM_DEBUG) && len) makes the problem go away, too. But I wonder if the latter is correct. Shouldn't the memcpy happen even with that debugging symbol enabled? > If not please send me your kconfig file. (It's a plain arm64 allmodconfig) Best regards Uwe
On Fri, Dec 23, 2022 at 06:47:19PM +0100, Uwe Kleine-König wrote: > > Using > > if (data && len) > > fixes it (that's the patch that b4 picks for the above message id :-\), > > if (!IS_ENABLED(CONFIG_CRYPTO_DEV_FSL_CAAM_DEBUG) && len) > > makes the problem go away, too. But I wonder if the latter is correct. Thanks for confirming! > Shouldn't the memcpy happen even with that debugging symbol enabled? It's just a pecularity with gcc. It only produces the warning because of the extra complexities introduced by the debugging code. Cheers,
diff --git a/drivers/crypto/caam/desc_constr.h b/drivers/crypto/caam/desc_constr.h index 62ce6421bb3f..163e0e740b11 100644 --- a/drivers/crypto/caam/desc_constr.h +++ b/drivers/crypto/caam/desc_constr.h @@ -163,7 +163,13 @@ static inline void append_data(u32 * const desc, const void *data, int len) { u32 *offset = desc_end(desc); - if (len) /* avoid sparse warning: memcpy with byte count of 0 */ + /* + * avoid sparse warning: "memcpy with byte count of 0" and + * and "error: argument 2 null where non-null expected + * [-Werror=nonnull]" with fortify enabled. + */ + BUG_ON(data == NULL && len != 0); + if (len && data) memcpy(offset, data, len); (*desc) = cpu_to_caam32(caam32_to_cpu(*desc) +
When compiling arm64 allmodconfig with gcc 10.2.1 I get drivers/crypto/caam/desc_constr.h: In function ‘append_data.constprop’: include/linux/fortify-string.h:57:29: error: argument 2 null where non-null expected [-Werror=nonnull] Fix this by skipping the memcpy if data is NULL and add a BUG_ON instead that triggers on a problematic call that is now prevented to trigger. After data == NULL && len != 0 is known to be false, logically if (len) memcpy(...) could be enough to know that memcpy is not called with dest=NULL, but gcc doesn't seem smart enough for that conclusion. gcc 12 doesn't have a problem with the original code. Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix.de> --- drivers/crypto/caam/desc_constr.h | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) base-commit: 9d2f6060fe4c3b49d0cdc1dce1c99296f33379c8