Message ID | 20230106041945.never.831-kees@kernel.org (mailing list archive) |
---|---|
State | Changes Requested |
Delegated to: | Herbert Xu |
Headers | show |
Series | crypto: hisilicon: Wipe entire pool on error | expand |
On Thu, Jan 05, 2023 at 08:19:48PM -0800, Kees Cook wrote: > To work around a Clang __builtin_object_size bug that shows up under > CONFIG_FORTIFY_SOURCE and UBSAN_BOUNDS, move the per-loop-iteration > mem_block wipe into a single wipe of the entire pool structure after > the loop. > > Reported-by: Nathan Chancellor <nathan@kernel.org> > Link: https://github.com/ClangBuiltLinux/linux/issues/1780 > Cc: Weili Qian <qianweili@huawei.com> > Cc: Zhou Wang <wangzhou1@hisilicon.com> > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: linux-crypto@vger.kernel.org > Signed-off-by: Kees Cook <keescook@chromium.org> Thanks for the patch! Tested-by: Nathan Chancellor <nathan@kernel.org> # build > --- > drivers/crypto/hisilicon/sgl.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/crypto/hisilicon/sgl.c b/drivers/crypto/hisilicon/sgl.c > index 2b6f2281cfd6..0974b0041405 100644 > --- a/drivers/crypto/hisilicon/sgl.c > +++ b/drivers/crypto/hisilicon/sgl.c > @@ -124,9 +124,8 @@ struct hisi_acc_sgl_pool *hisi_acc_create_sgl_pool(struct device *dev, > for (j = 0; j < i; j++) { > dma_free_coherent(dev, block_size, block[j].sgl, > block[j].sgl_dma); > - memset(block + j, 0, sizeof(*block)); > } > - kfree(pool); > + kfree_sensitive(pool); > return ERR_PTR(-ENOMEM); > } > EXPORT_SYMBOL_GPL(hisi_acc_create_sgl_pool); > -- > 2.34.1 >
On 2023/1/6 12:19, Kees Cook wrote: > To work around a Clang __builtin_object_size bug that shows up under > CONFIG_FORTIFY_SOURCE and UBSAN_BOUNDS, move the per-loop-iteration > mem_block wipe into a single wipe of the entire pool structure after > the loop. > > Reported-by: Nathan Chancellor <nathan@kernel.org> > Link: https://github.com/ClangBuiltLinux/linux/issues/1780 > Cc: Weili Qian <qianweili@huawei.com> > Cc: Zhou Wang <wangzhou1@hisilicon.com> > Cc: Herbert Xu <herbert@gondor.apana.org.au> > Cc: "David S. Miller" <davem@davemloft.net> > Cc: linux-crypto@vger.kernel.org > Signed-off-by: Kees Cook <keescook@chromium.org> > --- > drivers/crypto/hisilicon/sgl.c | 3 +-- > 1 file changed, 1 insertion(+), 2 deletions(-) > > diff --git a/drivers/crypto/hisilicon/sgl.c b/drivers/crypto/hisilicon/sgl.c > index 2b6f2281cfd6..0974b0041405 100644 > --- a/drivers/crypto/hisilicon/sgl.c > +++ b/drivers/crypto/hisilicon/sgl.c > @@ -124,9 +124,8 @@ struct hisi_acc_sgl_pool *hisi_acc_create_sgl_pool(struct device *dev, > for (j = 0; j < i; j++) { > dma_free_coherent(dev, block_size, block[j].sgl, > block[j].sgl_dma); > - memset(block + j, 0, sizeof(*block)); > } > - kfree(pool); > + kfree_sensitive(pool); > return ERR_PTR(-ENOMEM); > } > EXPORT_SYMBOL_GPL(hisi_acc_create_sgl_pool); > Thanks for your patch. There is no sensitive data in the pool, so memset zero can be deleted directly. Thanks, Weili
diff --git a/drivers/crypto/hisilicon/sgl.c b/drivers/crypto/hisilicon/sgl.c index 2b6f2281cfd6..0974b0041405 100644 --- a/drivers/crypto/hisilicon/sgl.c +++ b/drivers/crypto/hisilicon/sgl.c @@ -124,9 +124,8 @@ struct hisi_acc_sgl_pool *hisi_acc_create_sgl_pool(struct device *dev, for (j = 0; j < i; j++) { dma_free_coherent(dev, block_size, block[j].sgl, block[j].sgl_dma); - memset(block + j, 0, sizeof(*block)); } - kfree(pool); + kfree_sensitive(pool); return ERR_PTR(-ENOMEM); } EXPORT_SYMBOL_GPL(hisi_acc_create_sgl_pool);
To work around a Clang __builtin_object_size bug that shows up under CONFIG_FORTIFY_SOURCE and UBSAN_BOUNDS, move the per-loop-iteration mem_block wipe into a single wipe of the entire pool structure after the loop. Reported-by: Nathan Chancellor <nathan@kernel.org> Link: https://github.com/ClangBuiltLinux/linux/issues/1780 Cc: Weili Qian <qianweili@huawei.com> Cc: Zhou Wang <wangzhou1@hisilicon.com> Cc: Herbert Xu <herbert@gondor.apana.org.au> Cc: "David S. Miller" <davem@davemloft.net> Cc: linux-crypto@vger.kernel.org Signed-off-by: Kees Cook <keescook@chromium.org> --- drivers/crypto/hisilicon/sgl.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-)