All of lore.kernel.org
 help / color / mirror / Atom feed
* [arm-integrator:ixp4xx-test 22/23] drivers/crypto/ixp4xx_crypto.c:1060 aead_perform() error: uninitialized symbol 'icv_rev_aes'.
@ 2019-10-11 10:41 ` Dan Carpenter
  0 siblings, 0 replies; 7+ messages in thread
From: Dan Carpenter @ 2019-10-11 10:41 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 2649 bytes --]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-integrator.git ixp4xx-test
head:   1b826e686b390fd1eeb347def38185fefcbd5140
commit: 811c4aaa704314770fe5a70944100732fdc1f3b8 [22/23] crypto: ixp4xx: avoid BUILD_BUG_ON()

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
drivers/crypto/ixp4xx_crypto.c:1060 aead_perform() error: uninitialized symbol 'icv_rev_aes'.

# https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-integrator.git/commit/?id=811c4aaa704314770fe5a70944100732fdc1f3b8
git remote add arm-integrator https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-integrator.git
git remote update arm-integrator
git checkout 811c4aaa704314770fe5a70944100732fdc1f3b8
vim +/icv_rev_aes +1060 drivers/crypto/ixp4xx_crypto.c

d7295a8dc965ee Herbert Xu           2015-07-30  1054  	if (unlikely(lastlen < authsize)) {
811c4aaa704314 Arnd Bergmann        2019-08-27  1055  		dma_addr_t icv_rev_aes;
81bef0150074d6 Christian Hohnstaedt 2008-06-25  1056  		/* The 12 hmac bytes are scattered,
81bef0150074d6 Christian Hohnstaedt 2008-06-25  1057  		 * we need to copy them into a safe buffer */
81bef0150074d6 Christian Hohnstaedt 2008-06-25  1058  		req_ctx->hmac_virt = dma_pool_alloc(buffer_pool, flags,
811c4aaa704314 Arnd Bergmann        2019-08-27  1059  						    &icv_rev_aes);
811c4aaa704314 Arnd Bergmann        2019-08-27 @1060  		crypt->icv_rev_aes = (u32)icv_rev_aes;
                                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Assigned before error handling.

81bef0150074d6 Christian Hohnstaedt 2008-06-25  1061  		if (unlikely(!req_ctx->hmac_virt))
28389575a8cf93 Herbert Xu           2017-08-02  1062  			goto free_buf_dst;
81bef0150074d6 Christian Hohnstaedt 2008-06-25  1063  		if (!encrypt) {
81bef0150074d6 Christian Hohnstaedt 2008-06-25  1064  			scatterwalk_map_and_copy(req_ctx->hmac_virt,
81bef0150074d6 Christian Hohnstaedt 2008-06-25  1065  				req->src, cryptlen, authsize, 0);
81bef0150074d6 Christian Hohnstaedt 2008-06-25  1066  		}
81bef0150074d6 Christian Hohnstaedt 2008-06-25  1067  		req_ctx->encrypt = encrypt;
81bef0150074d6 Christian Hohnstaedt 2008-06-25  1068  	} else {
81bef0150074d6 Christian Hohnstaedt 2008-06-25  1069  		req_ctx->hmac_virt = NULL;
81bef0150074d6 Christian Hohnstaedt 2008-06-25  1070  	}

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [arm-integrator:ixp4xx-test 22/23] drivers/crypto/ixp4xx_crypto.c:1060 aead_perform() error: uninitialized symbol 'icv_rev_aes'.
@ 2019-10-11 10:41 ` Dan Carpenter
  0 siblings, 0 replies; 7+ messages in thread
From: Dan Carpenter @ 2019-10-11 10:41 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2649 bytes --]

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-integrator.git ixp4xx-test
head:   1b826e686b390fd1eeb347def38185fefcbd5140
commit: 811c4aaa704314770fe5a70944100732fdc1f3b8 [22/23] crypto: ixp4xx: avoid BUILD_BUG_ON()

If you fix the issue, kindly add following tag
Reported-by: kbuild test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
drivers/crypto/ixp4xx_crypto.c:1060 aead_perform() error: uninitialized symbol 'icv_rev_aes'.

# https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-integrator.git/commit/?id=811c4aaa704314770fe5a70944100732fdc1f3b8
git remote add arm-integrator https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-integrator.git
git remote update arm-integrator
git checkout 811c4aaa704314770fe5a70944100732fdc1f3b8
vim +/icv_rev_aes +1060 drivers/crypto/ixp4xx_crypto.c

d7295a8dc965ee Herbert Xu           2015-07-30  1054  	if (unlikely(lastlen < authsize)) {
811c4aaa704314 Arnd Bergmann        2019-08-27  1055  		dma_addr_t icv_rev_aes;
81bef0150074d6 Christian Hohnstaedt 2008-06-25  1056  		/* The 12 hmac bytes are scattered,
81bef0150074d6 Christian Hohnstaedt 2008-06-25  1057  		 * we need to copy them into a safe buffer */
81bef0150074d6 Christian Hohnstaedt 2008-06-25  1058  		req_ctx->hmac_virt = dma_pool_alloc(buffer_pool, flags,
811c4aaa704314 Arnd Bergmann        2019-08-27  1059  						    &icv_rev_aes);
811c4aaa704314 Arnd Bergmann        2019-08-27 @1060  		crypt->icv_rev_aes = (u32)icv_rev_aes;
                                                                ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Assigned before error handling.

81bef0150074d6 Christian Hohnstaedt 2008-06-25  1061  		if (unlikely(!req_ctx->hmac_virt))
28389575a8cf93 Herbert Xu           2017-08-02  1062  			goto free_buf_dst;
81bef0150074d6 Christian Hohnstaedt 2008-06-25  1063  		if (!encrypt) {
81bef0150074d6 Christian Hohnstaedt 2008-06-25  1064  			scatterwalk_map_and_copy(req_ctx->hmac_virt,
81bef0150074d6 Christian Hohnstaedt 2008-06-25  1065  				req->src, cryptlen, authsize, 0);
81bef0150074d6 Christian Hohnstaedt 2008-06-25  1066  		}
81bef0150074d6 Christian Hohnstaedt 2008-06-25  1067  		req_ctx->encrypt = encrypt;
81bef0150074d6 Christian Hohnstaedt 2008-06-25  1068  	} else {
81bef0150074d6 Christian Hohnstaedt 2008-06-25  1069  		req_ctx->hmac_virt = NULL;
81bef0150074d6 Christian Hohnstaedt 2008-06-25  1070  	}

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [arm-integrator:ixp4xx-test 22/23] drivers/crypto/ixp4xx_crypto.c:1060 aead_perform() error: uninitialized symbol 'icv_rev_aes'.
  2019-10-11 10:41 ` Dan Carpenter
  (?)
@ 2019-10-11 21:48 ` Arnd Bergmann
  2019-10-11 21:53   ` Arnd Bergmann
  2019-10-12  8:08     ` Dan Carpenter
  -1 siblings, 2 replies; 7+ messages in thread
From: Arnd Bergmann @ 2019-10-11 21:48 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2019 bytes --]

On Fri, Oct 11, 2019 at 12:41 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-integrator.git ixp4xx-test
> head:   1b826e686b390fd1eeb347def38185fefcbd5140
> commit: 811c4aaa704314770fe5a70944100732fdc1f3b8 [22/23] crypto: ixp4xx: avoid BUILD_BUG_ON()
>
> If you fix the issue, kindly add following tag
> Reported-by: kbuild test robot <lkp@intel.com>
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> New smatch warnings:
> drivers/crypto/ixp4xx_crypto.c:1060 aead_perform() error: uninitialized symbol 'icv_rev_aes'.

Right, this was uncovered by allowing the driver to be compile-tested on
x86. smatch is of course right to point out the uninitialized variable,
but it seems harmless as it is only uninitialized in case of an error,
and the uninitialized variable here gets copied into another data structure
that then gets put back, so it seems completely harmless

There is a trivial fix:

diff --git a/drivers/crypto/ixp4xx_crypto.c b/drivers/crypto/ixp4xx_crypto.c
index 1c6a04ecbf8f..a4eb1e98c87b 100644
--- a/drivers/crypto/ixp4xx_crypto.c
+++ b/drivers/crypto/ixp4xx_crypto.c
@@ -1057,9 +1057,9 @@ static int aead_perform(struct aead_request
*req, int encrypt,
                 * we need to copy them into a safe buffer */
                req_ctx->hmac_virt = dma_pool_alloc(buffer_pool, flags,
                                                    &icv_rev_aes);
-               crypt->icv_rev_aes = (u32)icv_rev_aes;
                if (unlikely(!req_ctx->hmac_virt))
                        goto free_buf_dst;
+               crypt->icv_rev_aes = (u32)icv_rev_aes;
                if (!encrypt) {
                        scatterwalk_map_and_copy(req_ctx->hmac_virt,
                                req->src, cryptlen, authsize, 0);

Dan, I think I would just leave it unchanged for a case like this, but I
can send a patch if you think it's worth addressing the warning.

     Arnd

^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [arm-integrator:ixp4xx-test 22/23] drivers/crypto/ixp4xx_crypto.c:1060 aead_perform() error: uninitialized symbol 'icv_rev_aes'.
  2019-10-11 21:48 ` Arnd Bergmann
@ 2019-10-11 21:53   ` Arnd Bergmann
  2019-10-18 14:49     ` Linus Walleij
  2019-10-12  8:08     ` Dan Carpenter
  1 sibling, 1 reply; 7+ messages in thread
From: Arnd Bergmann @ 2019-10-11 21:53 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 1435 bytes --]

On Fri, Oct 11, 2019 at 11:48 PM Arnd Bergmann <arnd@arndb.de> wrote:
> On Fri, Oct 11, 2019 at 12:41 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:

> diff --git a/drivers/crypto/ixp4xx_crypto.c b/drivers/crypto/ixp4xx_crypto.c
> index 1c6a04ecbf8f..a4eb1e98c87b 100644
> --- a/drivers/crypto/ixp4xx_crypto.c
> +++ b/drivers/crypto/ixp4xx_crypto.c
> @@ -1057,9 +1057,9 @@ static int aead_perform(struct aead_request
> *req, int encrypt,
>                  * we need to copy them into a safe buffer */
>                 req_ctx->hmac_virt = dma_pool_alloc(buffer_pool, flags,
>                                                     &icv_rev_aes);
> -               crypt->icv_rev_aes = (u32)icv_rev_aes;
>                 if (unlikely(!req_ctx->hmac_virt))
>                         goto free_buf_dst;
> +               crypt->icv_rev_aes = (u32)icv_rev_aes;
>                 if (!encrypt) {
>                         scatterwalk_map_and_copy(req_ctx->hmac_virt,
>                                 req->src, cryptlen, authsize, 0);
>
> Dan, I think I would just leave it unchanged for a case like this, but I
> can send a patch if you think it's worth addressing the warning.

Scratch that, it was indeed a bug that I introduced, let's fold that fix
into my patch.

Linus, are you going to handle the entire series and add the fix,
or should I add it in my tree and send you a new copy later?

      Arnd

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [arm-integrator:ixp4xx-test 22/23] drivers/crypto/ixp4xx_crypto.c:1060 aead_perform() error: uninitialized symbol 'icv_rev_aes'.
  2019-10-11 21:48 ` Arnd Bergmann
@ 2019-10-12  8:08     ` Dan Carpenter
  2019-10-12  8:08     ` Dan Carpenter
  1 sibling, 0 replies; 7+ messages in thread
From: Dan Carpenter @ 2019-10-12  8:08 UTC (permalink / raw)
  To: kbuild

[-- Attachment #1: Type: text/plain, Size: 2449 bytes --]

On Fri, Oct 11, 2019 at 11:48:16PM +0200, Arnd Bergmann wrote:
> On Fri, Oct 11, 2019 at 12:41 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> >
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-integrator.git ixp4xx-test
> > head:   1b826e686b390fd1eeb347def38185fefcbd5140
> > commit: 811c4aaa704314770fe5a70944100732fdc1f3b8 [22/23] crypto: ixp4xx: avoid BUILD_BUG_ON()
> >
> > If you fix the issue, kindly add following tag
> > Reported-by: kbuild test robot <lkp@intel.com>
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> >
> > New smatch warnings:
> > drivers/crypto/ixp4xx_crypto.c:1060 aead_perform() error: uninitialized symbol 'icv_rev_aes'.
> 
> Right, this was uncovered by allowing the driver to be compile-tested on
> x86. smatch is of course right to point out the uninitialized variable,
> but it seems harmless as it is only uninitialized in case of an error,
> and the uninitialized variable here gets copied into another data structure
> that then gets put back, so it seems completely harmless
> 
> There is a trivial fix:
> 
> diff --git a/drivers/crypto/ixp4xx_crypto.c b/drivers/crypto/ixp4xx_crypto.c
> index 1c6a04ecbf8f..a4eb1e98c87b 100644
> --- a/drivers/crypto/ixp4xx_crypto.c
> +++ b/drivers/crypto/ixp4xx_crypto.c
> @@ -1057,9 +1057,9 @@ static int aead_perform(struct aead_request
> *req, int encrypt,
>                  * we need to copy them into a safe buffer */
>                 req_ctx->hmac_virt = dma_pool_alloc(buffer_pool, flags,
>                                                     &icv_rev_aes);
> -               crypt->icv_rev_aes = (u32)icv_rev_aes;
>                 if (unlikely(!req_ctx->hmac_virt))
>                         goto free_buf_dst;
> +               crypt->icv_rev_aes = (u32)icv_rev_aes;
>                 if (!encrypt) {
>                         scatterwalk_map_and_copy(req_ctx->hmac_virt,
>                                 req->src, cryptlen, authsize, 0);
> 
> Dan, I think I would just leave it unchanged for a case like this, but I
> can send a patch if you think it's worth addressing the warning.

This is a new sort of harmless but not false positive warning.  I've
been conflicted on what to do with them, so I just report them when
they're new or ignore them when they're old.

I feel like ten years from now we will have fixed all these sorts of
warnings.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [arm-integrator:ixp4xx-test 22/23] drivers/crypto/ixp4xx_crypto.c:1060 aead_perform() error: uninitialized symbol 'icv_rev_aes'.
@ 2019-10-12  8:08     ` Dan Carpenter
  0 siblings, 0 replies; 7+ messages in thread
From: Dan Carpenter @ 2019-10-12  8:08 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 2449 bytes --]

On Fri, Oct 11, 2019 at 11:48:16PM +0200, Arnd Bergmann wrote:
> On Fri, Oct 11, 2019 at 12:41 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
> >
> > tree:   https://git.kernel.org/pub/scm/linux/kernel/git/linusw/linux-integrator.git ixp4xx-test
> > head:   1b826e686b390fd1eeb347def38185fefcbd5140
> > commit: 811c4aaa704314770fe5a70944100732fdc1f3b8 [22/23] crypto: ixp4xx: avoid BUILD_BUG_ON()
> >
> > If you fix the issue, kindly add following tag
> > Reported-by: kbuild test robot <lkp@intel.com>
> > Reported-by: Dan Carpenter <dan.carpenter@oracle.com>
> >
> > New smatch warnings:
> > drivers/crypto/ixp4xx_crypto.c:1060 aead_perform() error: uninitialized symbol 'icv_rev_aes'.
> 
> Right, this was uncovered by allowing the driver to be compile-tested on
> x86. smatch is of course right to point out the uninitialized variable,
> but it seems harmless as it is only uninitialized in case of an error,
> and the uninitialized variable here gets copied into another data structure
> that then gets put back, so it seems completely harmless
> 
> There is a trivial fix:
> 
> diff --git a/drivers/crypto/ixp4xx_crypto.c b/drivers/crypto/ixp4xx_crypto.c
> index 1c6a04ecbf8f..a4eb1e98c87b 100644
> --- a/drivers/crypto/ixp4xx_crypto.c
> +++ b/drivers/crypto/ixp4xx_crypto.c
> @@ -1057,9 +1057,9 @@ static int aead_perform(struct aead_request
> *req, int encrypt,
>                  * we need to copy them into a safe buffer */
>                 req_ctx->hmac_virt = dma_pool_alloc(buffer_pool, flags,
>                                                     &icv_rev_aes);
> -               crypt->icv_rev_aes = (u32)icv_rev_aes;
>                 if (unlikely(!req_ctx->hmac_virt))
>                         goto free_buf_dst;
> +               crypt->icv_rev_aes = (u32)icv_rev_aes;
>                 if (!encrypt) {
>                         scatterwalk_map_and_copy(req_ctx->hmac_virt,
>                                 req->src, cryptlen, authsize, 0);
> 
> Dan, I think I would just leave it unchanged for a case like this, but I
> can send a patch if you think it's worth addressing the warning.

This is a new sort of harmless but not false positive warning.  I've
been conflicted on what to do with them, so I just report them when
they're new or ignore them when they're old.

I feel like ten years from now we will have fixed all these sorts of
warnings.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [arm-integrator:ixp4xx-test 22/23] drivers/crypto/ixp4xx_crypto.c:1060 aead_perform() error: uninitialized symbol 'icv_rev_aes'.
  2019-10-11 21:53   ` Arnd Bergmann
@ 2019-10-18 14:49     ` Linus Walleij
  0 siblings, 0 replies; 7+ messages in thread
From: Linus Walleij @ 2019-10-18 14:49 UTC (permalink / raw)
  To: kbuild-all

[-- Attachment #1: Type: text/plain, Size: 490 bytes --]

On Fri, Oct 11, 2019 at 11:54 PM Arnd Bergmann <arnd@arndb.de> wrote:

> Scratch that, it was indeed a bug that I introduced, let's fold that fix
> into my patch.
>
> Linus, are you going to handle the entire series and add the fix,
> or should I add it in my tree and send you a new copy later?

I plan to get the front of the patch stack in shape for merging
so we get some movement in this patch series.

Compiling and testing on hardware as we speak.

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2019-10-18 14:49 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-11 10:41 [arm-integrator:ixp4xx-test 22/23] drivers/crypto/ixp4xx_crypto.c:1060 aead_perform() error: uninitialized symbol 'icv_rev_aes' Dan Carpenter
2019-10-11 10:41 ` Dan Carpenter
2019-10-11 21:48 ` Arnd Bergmann
2019-10-11 21:53   ` Arnd Bergmann
2019-10-18 14:49     ` Linus Walleij
2019-10-12  8:08   ` Dan Carpenter
2019-10-12  8:08     ` Dan Carpenter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.