* Re: [PATCH v2 00/11] getting back -Wmaybe-uninitialized
From: Linus Torvalds @ 2016-11-11 17:13 UTC (permalink / raw)
To: Arnd Bergmann, Srinivas Kandagatla, sayli karnik,
Jonathan Cameron, Mark Brown
Cc: Andrew Morton, Anna Schumaker, David S. Miller, Herbert Xu,
Ilya Dryomov, Javier Martinez Canillas, Jiri Kosina, Ley Foon Tan,
Luis R . Rodriguez, Martin Schwidefsky, Mauro Carvalho Chehab,
Michal Marek, Russell King, Sean Young, Sebastian Ott,
Trond Myklebust, the arch/x86 maintainers,
Linux Kbuild mailing list, Linux
In-Reply-To: <20161110164454.293477-1-arnd-r2nGTMty4D4@public.gmane.org>
On Thu, Nov 10, 2016 at 8:44 AM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
>
> Please merge these directly if you are happy with the result.
I will take this.
I do see two warnings, but they both seem to be valid and recent,
though, so I have no issues with the spurious cases.
Warning #1:
sound/soc/qcom/lpass-platform.c: In function ‘lpass_platform_pcmops_open’:
sound/soc/qcom/lpass-platform.c:83:29: warning: ‘dma_ch’ may be used
uninitialized in this function [-Wmaybe-uninitialized]
drvdata->substream[dma_ch] = substream;
~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
and 'dma_ch' usage there really is crazy and wrong. Broken by
022d00ee0b55 ("ASoC: lpass-platform: Fix broken pcm data usage")
Warning #2 is not a real bug, but it's reasonable that gcc doesn't
know that storage_bytes (chip->read_size) has to be 2/4. Again,
introduced recently by commit 231147ee77f3 ("iio: maxim_thermocouple:
Align 16 bit big endian value of raw reads"), so you didn't see it.
drivers/iio/temperature/maxim_thermocouple.c: In function
‘maxim_thermocouple_read_raw’:
drivers/iio/temperature/maxim_thermocouple.c:141:5: warning: ‘ret’
may be used uninitialized in this function [-Wmaybe-uninitialized]
if (ret)
^
drivers/iio/temperature/maxim_thermocouple.c:128:6: note: ‘ret’ was
declared here
int ret;
^~~
and I guess that code can just initialize 'ret' to '-EINVAL' or
something to just make the theoretical "somehow we had a wrong
chip->read_size" case error out cleanly.
Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH v2 00/11] getting back -Wmaybe-uninitialized
From: Arnd Bergmann @ 2016-11-11 19:49 UTC (permalink / raw)
To: Linus Torvalds
Cc: Sean Young, sayli karnik, Trond Myklebust, Srinivas Kandagatla,
linux-s390, Herbert Xu, the arch/x86 maintainers, Sebastian Ott,
Russell King, Javier Martinez Canillas, Ilya Dryomov, arcml,
Linux Media Mailing List, Linux Kbuild mailing list, Jiri Kosina,
Mark Brown, nios2-dev, Mauro Carvalho Chehab,
Linux NFS Mailing List, gregkh,
Linux Kernel Mailing List <linux
In-Reply-To: <CA+55aFx_scFVFKU__TBmoffw_iHvrdAU2dj5u1WKfWJXAkS4QA@mail.gmail.com>
On Friday, November 11, 2016 9:13:00 AM CET Linus Torvalds wrote:
> On Thu, Nov 10, 2016 at 8:44 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> >
> > Please merge these directly if you are happy with the result.
>
> I will take this.
Thanks a lot!
> I do see two warnings, but they both seem to be valid and recent,
> though, so I have no issues with the spurious cases.
Ok, both of them should have my fixes coming your way already.
> Warning #1:
>
> sound/soc/qcom/lpass-platform.c: In function ‘lpass_platform_pcmops_open’:
> sound/soc/qcom/lpass-platform.c:83:29: warning: ‘dma_ch’ may be used
> uninitialized in this function [-Wmaybe-uninitialized]
> drvdata->substream[dma_ch] = substream;
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
>
> and 'dma_ch' usage there really is crazy and wrong. Broken by
> 022d00ee0b55 ("ASoC: lpass-platform: Fix broken pcm data usage")
Right, the patches crossed here, the bugfix patch that introduced
this came into linux-next over the kernel summit, and the fix I
sent on Tuesday made it into Mark Brown's tree on Wednesday but not
before you pulled alsa tree. It should be fixed the next time you
pull from the alsa tree, the commit is
3b89e4b77ef9 ("ASoC: lpass-platform: initialize dma channel number")
> Warning #2 is not a real bug, but it's reasonable that gcc doesn't
> know that storage_bytes (chip->read_size) has to be 2/4. Again,
> introduced recently by commit 231147ee77f3 ("iio: maxim_thermocouple:
> Align 16 bit big endian value of raw reads"), so you didn't see it.
This is the one I mentioned in the commit message as one that
is fixed in linux-next and that should make it in soon.
> drivers/iio/temperature/maxim_thermocouple.c: In function
> ‘maxim_thermocouple_read_raw’:
> drivers/iio/temperature/maxim_thermocouple.c:141:5: warning: ‘ret’
> may be used uninitialized in this function [-Wmaybe-uninitialized]
> if (ret)
> ^
> drivers/iio/temperature/maxim_thermocouple.c:128:6: note: ‘ret’ was
> declared here
> int ret;
> ^~~
>
> and I guess that code can just initialize 'ret' to '-EINVAL' or
> something to just make the theoretical "somehow we had a wrong
> chip->read_size" case error out cleanly.
Right, that was my conclusion too. I sent the bugfix on Oct 25
for linux-next but it didn't make it in until this Monday, after
you pulled the patch that introduced it on Oct 29.
The commit in staging-testing is
32cb7d27e65d ("iio: maxim_thermocouple: detect invalid storage size in read()")
Greg and Jonathan, I see now that this is part of the 'iio-for-4.10b'
branch, so I suspect you were not planning to send this before the
merge window. Could you make sure this ends up in v4.9 so we get
a clean build when -Wmaybe-uninitialized gets enabled again?
Arnd
_______________________________________________
linux-snps-arc mailing list
linux-snps-arc@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-snps-arc
^ permalink raw reply
* Re: [PATCH] crypto: arm64/sha2: integrate OpenSSL implementations of SHA256/SHA512
From: Will Deacon @ 2016-11-11 19:56 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: linux-crypto, linux-arm-kernel, herbert, catalin.marinas, appro,
victor.chong, daniel.thompson
In-Reply-To: <1478872273-16382-1-git-send-email-ard.biesheuvel@linaro.org>
On Fri, Nov 11, 2016 at 09:51:13PM +0800, Ard Biesheuvel wrote:
> This integrates both the accelerated scalar and the NEON implementations
> of SHA-224/256 as well as SHA-384/512 from the OpenSSL project.
>
> Relative performance compared to the respective generic C versions:
>
> | SHA256-scalar | SHA256-NEON* | SHA512 |
> ------------+-----------------+--------------+----------+
> Cortex-A53 | 1.63x | 1.63x | 2.34x |
> Cortex-A57 | 1.43x | 1.59x | 1.95x |
> Cortex-A73 | 1.26x | 1.56x | ? |
>
> The core crypto code was authored by Andy Polyakov of the OpenSSL
> project, in collaboration with whom the upstream code was adapted so
> that this module can be built from the same version of sha512-armv8.pl.
>
> The version in this patch was taken from OpenSSL commit
>
> 866e505e0d66 sha/asm/sha512-armv8.pl: add NEON version of SHA256.
>
> * The core SHA algorithm is fundamentally sequential, but there is a
> secondary transformation involved, called the schedule update, which
> can be performed independently. The NEON version of SHA-224/SHA-256
> only implements this part of the algorithm using NEON instructions,
> the sequential part is always done using scalar instructions.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
>
> This supersedes the SHA-256-NEON-only patch I sent out about 6 weeks ago.
>
> Will, Catalin: note that this pulls in a .pl script, and adds a build rule
> locally in arch/arm64/crypto to generate .S files on the fly from Perl
> scripts. I will leave it to you to decide whether you are ok with this as
> is, or whether you prefer .S_shipped files, in which case the Perl script
> is only included as a reference (this is how we did it for arch/arm in the
> past, but given that it adds about 3000 lines of generated code to the patch,
> I think we may want to simply keep it as below)
I think we should include the shipped files too. 3000 lines isn't that much
in the grand scheme of things, and there will be people who complain about
the unconditional perl dependency.
Will
^ permalink raw reply
* Re: [PATCH 2/3] crypto: AF_ALG - disregard AAD buffer space for output
From: Mat Martineau @ 2016-11-12 0:26 UTC (permalink / raw)
To: Stephan Mueller; +Cc: herbert, linux-crypto
In-Reply-To: <3506033.FskOdlTquT@positron.chronox.de>
Stephan,
On Thu, 10 Nov 2016, Stephan Mueller wrote:
> The kernel crypto API AEAD cipher operation generates output such that
> space for the AAD is reserved in the output buffer without being
> touched. The processed ciphertext/plaintext is appended to the reserved
> AAD buffer.
>
> The user space interface followed that approach. However, this is a
> violation of the POSIX read definition which requires that any read data
> is placed at the beginning of the caller-provided buffer. As the kernel
> crypto API would leave room for the AAD, the old approach did not fully
> comply with the POSIX specification.
>
> The patch changes the user space AF_ALG AEAD interface such that the
> processed ciphertext/plaintext are now placed at the beginning of the
> user buffer provided with the read system call. That means the user
> space interface now deviates from the in-kernel output buffer handling.
>
> For the cipher operation, the AAD buffer provided during input is
> pointed to by a new SGL which is chained with the output buffer SGL.
> With this approach, only pointers to one copy of the AAD are maintained
> to avoid data duplication.
>
> With this solution, the caller must not use sendpage with the exact same
> buffers for input and output. The following rationale applies: When
> the caller sends the same buffer for input/output to the sendpage
> operation, the cipher operation now will write the ciphertext to the
> beginning of the buffer where the AAD used to be. The subsequent tag
> calculation will now use the data it finds where the AAD is expected.
> As the cipher operation has already replaced the AAD with the ciphertext,
> the tag calculation will take the ciphertext as AAD and thus calculate
> a wrong tag.
If it's not much overhead, I suggest checking for this condition and
returning an error.
Other than that, I've done a quick test of the patches using sendmsg() and
read() and found that they work as expected.
Thanks,
Mat
> Reported-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
> Signed-off-by: Stephan Mueller <smueller@chronox.de>
> ---
> crypto/algif_aead.c | 143 +++++++++++++++++++++++++++++++++++++++++-----------
> 1 file changed, 113 insertions(+), 30 deletions(-)
>
> diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
> index c54bcb8..0212cc2 100644
> --- a/crypto/algif_aead.c
> +++ b/crypto/algif_aead.c
> @@ -32,6 +32,7 @@ struct aead_sg_list {
> struct aead_async_rsgl {
> struct af_alg_sgl sgl;
> struct list_head list;
> + bool new_page;
> };
>
> struct aead_async_req {
> @@ -405,6 +406,61 @@ static void aead_async_cb(struct crypto_async_request *_req, int err)
> iocb->ki_complete(iocb, err, err);
> }
>
> +/**
> + * scatterwalk_get_part() - get subset a scatterlist
> + *
> + * @dst: destination SGL to receive the pointers from source SGL
> + * @src: source SGL
> + * @len: data length in bytes to get from source SGL
> + * @max_sgs: number of SGs present in dst SGL to prevent overstepping boundaries
> + *
> + * @return: number of SG entries in dst
> + */
> +static inline int scatterwalk_get_part(struct scatterlist *dst,
> + struct scatterlist *src,
> + unsigned int len, unsigned int max_sgs)
> +{
> + /* leave one SG entry for chaining */
> + unsigned int j = 1;
> +
> + while (len && j < max_sgs) {
> + unsigned int todo = min_t(unsigned int, len, src->length);
> +
> + sg_set_page(dst, sg_page(src), todo, src->offset);
> + if (src->length >= len) {
> + sg_mark_end(dst);
> + break;
> + }
> + len -= todo;
> + j++;
> + src = sg_next(src);
> + dst = sg_next(dst);
> + }
> +
> + return j;
> +}
> +
> +static inline int aead_alloc_rsgl(struct sock *sk, struct aead_async_rsgl **ret)
> +{
> + struct aead_async_rsgl *rsgl =
> + sock_kmalloc(sk, sizeof(*rsgl), GFP_KERNEL);
> + if (unlikely(!rsgl))
> + return -ENOMEM;
> + *ret = rsgl;
> + return 0;
> +}
> +
> +static inline int aead_get_rsgl_areq(struct sock *sk,
> + struct aead_async_req *areq,
> + struct aead_async_rsgl **ret)
> +{
> + if (list_empty(&areq->list)) {
> + *ret = &areq->first_rsgl;
> + return 0;
> + } else
> + return aead_alloc_rsgl(sk, ret);
> +}
> +
> static int aead_recvmsg_async(struct socket *sock, struct msghdr *msg,
> int flags)
> {
> @@ -433,7 +489,7 @@ static int aead_recvmsg_async(struct socket *sock, struct msghdr *msg,
> if (!aead_sufficient_data(ctx))
> goto unlock;
>
> - used = ctx->used;
> + used = ctx->used - ctx->aead_assoclen;
> if (ctx->enc)
> outlen = used + as;
> else
> @@ -452,7 +508,6 @@ static int aead_recvmsg_async(struct socket *sock, struct msghdr *msg,
> aead_request_set_ad(req, ctx->aead_assoclen);
> aead_request_set_callback(req, CRYPTO_TFM_REQ_MAY_BACKLOG,
> aead_async_cb, sk);
> - used -= ctx->aead_assoclen;
>
> /* take over all tx sgls from ctx */
> areq->tsgl = sock_kmalloc(sk, sizeof(*areq->tsgl) * sgl->cur,
> @@ -467,21 +522,26 @@ static int aead_recvmsg_async(struct socket *sock, struct msghdr *msg,
>
> areq->tsgls = sgl->cur;
>
> + /* set AAD buffer */
> + err = aead_get_rsgl_areq(sk, areq, &rsgl);
> + if (err)
> + goto free;
> + list_add_tail(&rsgl->list, &areq->list);
> + sg_init_table(rsgl->sgl.sg, ALG_MAX_PAGES);
> + rsgl->sgl.npages = scatterwalk_get_part(rsgl->sgl.sg, sgl->sg,
> + ctx->aead_assoclen,
> + ALG_MAX_PAGES);
> + rsgl->new_page = false;
> + last_rsgl = rsgl;
> +
> /* create rx sgls */
> while (outlen > usedpages && iov_iter_count(&msg->msg_iter)) {
> size_t seglen = min_t(size_t, iov_iter_count(&msg->msg_iter),
> (outlen - usedpages));
>
> - if (list_empty(&areq->list)) {
> - rsgl = &areq->first_rsgl;
> -
> - } else {
> - rsgl = sock_kmalloc(sk, sizeof(*rsgl), GFP_KERNEL);
> - if (unlikely(!rsgl)) {
> - err = -ENOMEM;
> - goto free;
> - }
> - }
> + err = aead_get_rsgl_areq(sk, areq, &rsgl);
> + if (err)
> + goto free;
> rsgl->sgl.npages = 0;
> list_add_tail(&rsgl->list, &areq->list);
>
> @@ -491,6 +551,7 @@ static int aead_recvmsg_async(struct socket *sock, struct msghdr *msg,
> goto free;
>
> usedpages += err;
> + rsgl->new_page = true;
>
> /* chain the new scatterlist with previous one */
> if (last_rsgl)
> @@ -507,7 +568,7 @@ static int aead_recvmsg_async(struct socket *sock, struct msghdr *msg,
>
> if (used < less) {
> err = -EINVAL;
> - goto unlock;
> + goto free;
> }
> used -= less;
> outlen -= less;
> @@ -531,7 +592,8 @@ static int aead_recvmsg_async(struct socket *sock, struct msghdr *msg,
>
> free:
> list_for_each_entry(rsgl, &areq->list, list) {
> - af_alg_free_sg(&rsgl->sgl);
> + if (rsgl->new_page)
> + af_alg_free_sg(&rsgl->sgl);
> if (rsgl != &areq->first_rsgl)
> sock_kfree_s(sk, rsgl, sizeof(*rsgl));
> }
> @@ -545,6 +607,16 @@ static int aead_recvmsg_async(struct socket *sock, struct msghdr *msg,
> return err ? err : outlen;
> }
>
> +static inline int aead_get_rsgl_ctx(struct sock *sk, struct aead_ctx *ctx,
> + struct aead_async_rsgl **ret)
> +{
> + if (list_empty(&ctx->list)) {
> + *ret = &ctx->first_rsgl;
> + return 0;
> + } else
> + return aead_alloc_rsgl(sk, ret);
> +}
> +
> static int aead_recvmsg_sync(struct socket *sock, struct msghdr *msg, int flags)
> {
> struct sock *sk = sock->sk;
> @@ -582,9 +654,6 @@ static int aead_recvmsg_sync(struct socket *sock, struct msghdr *msg, int flags)
> goto unlock;
> }
>
> - /* data length provided by caller via sendmsg/sendpage */
> - used = ctx->used;
> -
> /*
> * Make sure sufficient data is present -- note, the same check is
> * is also present in sendmsg/sendpage. The checks in sendpage/sendmsg
> @@ -598,6 +667,12 @@ static int aead_recvmsg_sync(struct socket *sock, struct msghdr *msg, int flags)
> goto unlock;
>
> /*
> + * The cipher operation input data is reduced by the associated data
> + * as the destination buffer will not hold the AAD.
> + */
> + used = ctx->used - ctx->aead_assoclen;
> +
> + /*
> * Calculate the minimum output buffer size holding the result of the
> * cipher operation. When encrypting data, the receiving buffer is
> * larger by the tag length compared to the input buffer as the
> @@ -611,25 +686,29 @@ static int aead_recvmsg_sync(struct socket *sock, struct msghdr *msg, int flags)
> outlen = used - as;
>
> /*
> - * The cipher operation input data is reduced by the associated data
> - * length as this data is processed separately later on.
> + * Pre-pend the AAD buffer from the source SGL to the destination SGL.
> + * As the AAD buffer is not touched by the AEAD operation, the source
> + * SG buffers remain unchanged.
> */
> - used -= ctx->aead_assoclen;
> + err = aead_get_rsgl_ctx(sk, ctx, &rsgl);
> + if (err)
> + goto unlock;
> + list_add_tail(&rsgl->list, &ctx->list);
> + sg_init_table(rsgl->sgl.sg, ALG_MAX_PAGES);
> + rsgl->sgl.npages = scatterwalk_get_part(rsgl->sgl.sg, sgl->sg,
> + ctx->aead_assoclen,
> + ALG_MAX_PAGES);
> + rsgl->new_page = false;
> + last_rsgl = rsgl;
>
> /* convert iovecs of output buffers into scatterlists */
> while (outlen > usedpages && iov_iter_count(&msg->msg_iter)) {
> size_t seglen = min_t(size_t, iov_iter_count(&msg->msg_iter),
> (outlen - usedpages));
>
> - if (list_empty(&ctx->list)) {
> - rsgl = &ctx->first_rsgl;
> - } else {
> - rsgl = sock_kmalloc(sk, sizeof(*rsgl), GFP_KERNEL);
> - if (unlikely(!rsgl)) {
> - err = -ENOMEM;
> - goto unlock;
> - }
> - }
> + err = aead_get_rsgl_ctx(sk, ctx, &rsgl);
> + if (err)
> + goto unlock;
> rsgl->sgl.npages = 0;
> list_add_tail(&rsgl->list, &ctx->list);
>
> @@ -637,7 +716,10 @@ static int aead_recvmsg_sync(struct socket *sock, struct msghdr *msg, int flags)
> err = af_alg_make_sg(&rsgl->sgl, &msg->msg_iter, seglen);
> if (err < 0)
> goto unlock;
> +
> usedpages += err;
> + rsgl->new_page = true;
> +
> /* chain the new scatterlist with previous one */
> if (last_rsgl)
> af_alg_link_sg(&last_rsgl->sgl, &rsgl->sgl);
> @@ -688,7 +770,8 @@ static int aead_recvmsg_sync(struct socket *sock, struct msghdr *msg, int flags)
>
> unlock:
> list_for_each_entry_safe(rsgl, tmp, &ctx->list, list) {
> - af_alg_free_sg(&rsgl->sgl);
> + if (rsgl->new_page)
> + af_alg_free_sg(&rsgl->sgl);
> if (rsgl != &ctx->first_rsgl)
> sock_kfree_s(sk, rsgl, sizeof(*rsgl));
> list_del(&rsgl->list);
> --
> 2.7.4
>
>
>
--
Mat Martineau
Intel OTC
^ permalink raw reply
* Re: [PATCH 2/3] crypto: AF_ALG - disregard AAD buffer space for output
From: Herbert Xu @ 2016-11-12 1:55 UTC (permalink / raw)
To: Stephan Mueller; +Cc: mathew.j.martineau, linux-crypto
In-Reply-To: <3506033.FskOdlTquT@positron.chronox.de>
On Thu, Nov 10, 2016 at 04:32:03AM +0100, Stephan Mueller wrote:
> The kernel crypto API AEAD cipher operation generates output such that
> space for the AAD is reserved in the output buffer without being
> touched. The processed ciphertext/plaintext is appended to the reserved
> AAD buffer.
>
> The user space interface followed that approach. However, this is a
> violation of the POSIX read definition which requires that any read data
> is placed at the beginning of the caller-provided buffer. As the kernel
> crypto API would leave room for the AAD, the old approach did not fully
> comply with the POSIX specification.
Nack. The kernel AEAD API will copy the AD as is, it definitely
does not leave the output untouched unless of course when it is
an in-place operation. The user-space operation should operate
in the same manner.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH 2/3] crypto: AF_ALG - disregard AAD buffer space for output
From: Stephan Mueller @ 2016-11-12 2:12 UTC (permalink / raw)
To: Mat Martineau; +Cc: herbert, linux-crypto
In-Reply-To: <alpine.OSX.2.20.1611111622500.1815@mjmartin-mac01.sea.intel.com>
Am Freitag, 11. November 2016, 16:26:12 CET schrieb Mat Martineau:
Hi Mat,
> >
> > With this solution, the caller must not use sendpage with the exact same
> > buffers for input and output. The following rationale applies: When
> > the caller sends the same buffer for input/output to the sendpage
> > operation, the cipher operation now will write the ciphertext to the
> > beginning of the buffer where the AAD used to be. The subsequent tag
> > calculation will now use the data it finds where the AAD is expected.
> > As the cipher operation has already replaced the AAD with the ciphertext,
> > the tag calculation will take the ciphertext as AAD and thus calculate
> > a wrong tag.
>
> If it's not much overhead, I suggest checking for this condition and
> returning an error.
I can surely look into that. But Herbert's NACK seems to make this patch
unlikely.
>
> Other than that, I've done a quick test of the patches using sendmsg() and
> read() and found that they work as expected.
>
Thanks for testing.
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH 2/3] crypto: AF_ALG - disregard AAD buffer space for output
From: Stephan Mueller @ 2016-11-12 2:03 UTC (permalink / raw)
To: Herbert Xu; +Cc: mathew.j.martineau, linux-crypto
In-Reply-To: <20161112015519.GA32234@gondor.apana.org.au>
Am Samstag, 12. November 2016, 09:55:19 CET schrieb Herbert Xu:
Hi Herbert,
> On Thu, Nov 10, 2016 at 04:32:03AM +0100, Stephan Mueller wrote:
> > The kernel crypto API AEAD cipher operation generates output such that
> > space for the AAD is reserved in the output buffer without being
> > touched. The processed ciphertext/plaintext is appended to the reserved
> > AAD buffer.
> >
> > The user space interface followed that approach. However, this is a
> > violation of the POSIX read definition which requires that any read data
> > is placed at the beginning of the caller-provided buffer. As the kernel
> > crypto API would leave room for the AAD, the old approach did not fully
> > comply with the POSIX specification.
>
> Nack. The kernel AEAD API will copy the AD as is, it definitely
> does not leave the output untouched unless of course when it is
> an in-place operation. The user-space operation should operate
> in the same manner.
When you have separate buffers, the kernel does not seem to copy the AD over
to the target buffer.
>
> Cheers,
Ciao
Stephan
^ permalink raw reply
* Re: [PATCH 2/3] crypto: AF_ALG - disregard AAD buffer space for output
From: Herbert Xu @ 2016-11-12 2:13 UTC (permalink / raw)
To: Stephan Mueller; +Cc: mathew.j.martineau, linux-crypto
In-Reply-To: <11739696.UFOoJjMX73@positron.chronox.de>
On Sat, Nov 12, 2016 at 03:03:36AM +0100, Stephan Mueller wrote:
>
> When you have separate buffers, the kernel does not seem to copy the AD over
> to the target buffer.
OK we should definitely fix that.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] crypto: arm64/sha2: integrate OpenSSL implementations of SHA256/SHA512
From: Ard Biesheuvel @ 2016-11-12 12:26 UTC (permalink / raw)
To: Will Deacon
Cc: linux-crypto@vger.kernel.org,
linux-arm-kernel@lists.infradead.org, Herbert Xu, Catalin Marinas,
Andy Polyakov, Victor Chong, Daniel Thompson
In-Reply-To: <20161111195607.GB4457@arm.com>
On 11 November 2016 at 20:56, Will Deacon <will.deacon@arm.com> wrote:
> On Fri, Nov 11, 2016 at 09:51:13PM +0800, Ard Biesheuvel wrote:
>> This integrates both the accelerated scalar and the NEON implementations
>> of SHA-224/256 as well as SHA-384/512 from the OpenSSL project.
>>
>> Relative performance compared to the respective generic C versions:
>>
>> | SHA256-scalar | SHA256-NEON* | SHA512 |
>> ------------+-----------------+--------------+----------+
>> Cortex-A53 | 1.63x | 1.63x | 2.34x |
>> Cortex-A57 | 1.43x | 1.59x | 1.95x |
>> Cortex-A73 | 1.26x | 1.56x | ? |
>>
>> The core crypto code was authored by Andy Polyakov of the OpenSSL
>> project, in collaboration with whom the upstream code was adapted so
>> that this module can be built from the same version of sha512-armv8.pl.
>>
>> The version in this patch was taken from OpenSSL commit
>>
>> 866e505e0d66 sha/asm/sha512-armv8.pl: add NEON version of SHA256.
>>
>> * The core SHA algorithm is fundamentally sequential, but there is a
>> secondary transformation involved, called the schedule update, which
>> can be performed independently. The NEON version of SHA-224/SHA-256
>> only implements this part of the algorithm using NEON instructions,
>> the sequential part is always done using scalar instructions.
>>
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> ---
>>
>> This supersedes the SHA-256-NEON-only patch I sent out about 6 weeks ago.
>>
>> Will, Catalin: note that this pulls in a .pl script, and adds a build rule
>> locally in arch/arm64/crypto to generate .S files on the fly from Perl
>> scripts. I will leave it to you to decide whether you are ok with this as
>> is, or whether you prefer .S_shipped files, in which case the Perl script
>> is only included as a reference (this is how we did it for arch/arm in the
>> past, but given that it adds about 3000 lines of generated code to the patch,
>> I think we may want to simply keep it as below)
>
> I think we should include the shipped files too. 3000 lines isn't that much
> in the grand scheme of things, and there will be people who complain about
> the unconditional perl dependency.
>
OK, fair enough. I will repost with the generated files included.
^ permalink raw reply
* Re: [PATCH v2 00/11] getting back -Wmaybe-uninitialized
From: Jonathan Cameron @ 2016-11-12 13:27 UTC (permalink / raw)
To: Arnd Bergmann, Linus Torvalds
Cc: Srinivas Kandagatla, sayli karnik, Mark Brown, Andrew Morton,
Anna Schumaker, David S. Miller, Herbert Xu, Ilya Dryomov,
Javier Martinez Canillas, Jiri Kosina, Ley Foon Tan,
Luis R . Rodriguez, Martin Schwidefsky, Mauro Carvalho Chehab,
Michal Marek, Russell King, Sean Young, Sebastian Ott,
Trond Myklebust <trond.myklebu
In-Reply-To: <2695221.kyRJMsRMjs@wuerfel>
On 11/11/16 19:49, Arnd Bergmann wrote:
> On Friday, November 11, 2016 9:13:00 AM CET Linus Torvalds wrote:
>> On Thu, Nov 10, 2016 at 8:44 AM, Arnd Bergmann <arnd@arndb.de> wrote:
>>>
>>> Please merge these directly if you are happy with the result.
>>
>> I will take this.
>
> Thanks a lot!
>
>> I do see two warnings, but they both seem to be valid and recent,
>> though, so I have no issues with the spurious cases.
>
> Ok, both of them should have my fixes coming your way already.
>
>> Warning #1:
>>
>> sound/soc/qcom/lpass-platform.c: In function ‘lpass_platform_pcmops_open’:
>> sound/soc/qcom/lpass-platform.c:83:29: warning: ‘dma_ch’ may be used
>> uninitialized in this function [-Wmaybe-uninitialized]
>> drvdata->substream[dma_ch] = substream;
>> ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
>>
>> and 'dma_ch' usage there really is crazy and wrong. Broken by
>> 022d00ee0b55 ("ASoC: lpass-platform: Fix broken pcm data usage")
>
> Right, the patches crossed here, the bugfix patch that introduced
> this came into linux-next over the kernel summit, and the fix I
> sent on Tuesday made it into Mark Brown's tree on Wednesday but not
> before you pulled alsa tree. It should be fixed the next time you
> pull from the alsa tree, the commit is
>
> 3b89e4b77ef9 ("ASoC: lpass-platform: initialize dma channel number")
>
>> Warning #2 is not a real bug, but it's reasonable that gcc doesn't
>> know that storage_bytes (chip->read_size) has to be 2/4. Again,
>> introduced recently by commit 231147ee77f3 ("iio: maxim_thermocouple:
>> Align 16 bit big endian value of raw reads"), so you didn't see it.
>
> This is the one I mentioned in the commit message as one that
> is fixed in linux-next and that should make it in soon.
>
>> drivers/iio/temperature/maxim_thermocouple.c: In function
>> ‘maxim_thermocouple_read_raw’:
>> drivers/iio/temperature/maxim_thermocouple.c:141:5: warning: ‘ret’
>> may be used uninitialized in this function [-Wmaybe-uninitialized]
>> if (ret)
>> ^
>> drivers/iio/temperature/maxim_thermocouple.c:128:6: note: ‘ret’ was
>> declared here
>> int ret;
>> ^~~
>>
>> and I guess that code can just initialize 'ret' to '-EINVAL' or
>> something to just make the theoretical "somehow we had a wrong
>> chip->read_size" case error out cleanly.
>
> Right, that was my conclusion too. I sent the bugfix on Oct 25
> for linux-next but it didn't make it in until this Monday, after
> you pulled the patch that introduced it on Oct 29.
>
> The commit in staging-testing is
> 32cb7d27e65d ("iio: maxim_thermocouple: detect invalid storage size in read()")
>
> Greg and Jonathan, I see now that this is part of the 'iio-for-4.10b'
> branch, so I suspect you were not planning to send this before the
> merge window. Could you make sure this ends up in v4.9 so we get
> a clean build when -Wmaybe-uninitialized gets enabled again?
I'll queue this up and send a pull to Greg tomorrow.
Was highly doubtful that a false warning suppression (be it an
understandable one) was worth sending mid cycle, hence it was
taking the slow route.
Jonathan
>
> Arnd
>
^ permalink raw reply
* Re: [PATCH v3] crypto: arm64/sha2: integrate OpenSSL implementations of SHA256/SHA512
From: Will Deacon @ 2016-11-12 22:15 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-crypto, linux-arm-kernel, herbert, catalin.marinas
In-Reply-To: <1478953953-11523-1-git-send-email-ard.biesheuvel@linaro.org>
Hi Ard,
On Sat, Nov 12, 2016 at 01:32:33PM +0100, Ard Biesheuvel wrote:
> This integrates both the accelerated scalar and the NEON implementations
> of SHA-224/256 as well as SHA-384/512 from the OpenSSL project.
>
> Relative performance compared to the respective generic C versions:
>
> | SHA256-scalar | SHA256-NEON* | SHA512 |
> ------------+-----------------+--------------+----------+
> Cortex-A53 | 1.63x | 1.63x | 2.34x |
> Cortex-A57 | 1.43x | 1.59x | 1.95x |
> Cortex-A73 | 1.26x | 1.56x | ? |
>
> The core crypto code was authored by Andy Polyakov of the OpenSSL
> project, in collaboration with whom the upstream code was adapted so
> that this module can be built from the same version of sha512-armv8.pl.
>
> The version in this patch was taken from OpenSSL commit
>
> 866e505e0d66 sha/asm/sha512-armv8.pl: add NEON version of SHA256.
>
> * The core SHA algorithm is fundamentally sequential, but there is a
> secondary transformation involved, called the schedule update, which
> can be performed independently. The NEON version of SHA-224/SHA-256
> only implements this part of the algorithm using NEON instructions,
> the sequential part is always done using scalar instructions.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> v3: at Will's request, the generated assembly files are now included
> as .S_shipped files, for which generic build rules are defined
> already. Note that this has caused issues in the past with
> patchwork, so for Herbert's convenience, the patch can be pulled
> from http://git.kernel.org/cgit/linux/kernel/git/ardb/linux.git,
> branch arm64-sha256 (based on today's cryptodev)
Thanks.
Looking at the generated code, I see references to __ARMEB__ and __ILP32__.
The former is probably a bug, whilst the second is not required. There are
also some commented out instructions, which is weird.
Will
^ permalink raw reply
* Re: [PATCH v3] poly1305: generic C can be faster on chips with slow unaligned access
From: kbuild test robot @ 2016-11-12 23:27 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: kbuild-all, Herbert Xu, David S. Miller, linux-crypto,
linux-kernel, Martin Willi, Eric Biggers, René van Dorst,
Jason A. Donenfeld
In-Reply-To: <20161107194345.19955-1-Jason@zx2c4.com>
[-- Attachment #1: Type: text/plain, Size: 1708 bytes --]
Hi Jason,
[auto build test ERROR on cryptodev/master]
[also build test ERROR on v4.9-rc4 next-20161111]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/Jason-A-Donenfeld/poly1305-generic-C-can-be-faster-on-chips-with-slow-unaligned-access/20161108-053912
base: https://git.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git master
config: openrisc-allmodconfig (attached as .config)
compiler: or32-linux-gcc (GCC) 4.5.1-or32-1.0rc1
reproduce:
wget https://git.kernel.org/cgit/linux/kernel/git/wfg/lkp-tests.git/plain/sbin/make.cross -O ~/bin/make.cross
chmod +x ~/bin/make.cross
# save the attached .config to linux build tree
make.cross ARCH=openrisc
All errors (new ones prefixed by >>):
crypto/poly1305_generic.c: In function 'poly1305_setrkey':
>> crypto/poly1305_generic.c:63:2: error: implicit declaration of function 'get_unaligned_le32'
vim +/get_unaligned_le32 +63 crypto/poly1305_generic.c
57 }
58 EXPORT_SYMBOL_GPL(crypto_poly1305_setkey);
59
60 static void poly1305_setrkey(struct poly1305_desc_ctx *dctx, const u8 *key)
61 {
62 /* r &= 0xffffffc0ffffffc0ffffffc0fffffff */
> 63 dctx->r[0] = (get_unaligned_le32(key + 0) >> 0) & 0x3ffffff;
64 dctx->r[1] = (get_unaligned_le32(key + 3) >> 2) & 0x3ffff03;
65 dctx->r[2] = (get_unaligned_le32(key + 6) >> 4) & 0x3ffc0ff;
66 dctx->r[3] = (get_unaligned_le32(key + 9) >> 6) & 0x3f03fff;
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 39242 bytes --]
^ permalink raw reply
* Re: [PATCH v3] poly1305: generic C can be faster on chips with slow unaligned access
From: Jason A. Donenfeld @ 2016-11-12 23:31 UTC (permalink / raw)
To: kbuild test robot
Cc: kbuild-all, Herbert Xu, David S. Miller, linux-crypto, LKML,
Martin Willi, Eric Biggers, René van Dorst
Hello friendly test robot,
On Sun, Nov 13, 2016 at 12:27 AM, kbuild test robot <lkp@intel.com> wrote:
> Hi Jason,
>
> [auto build test ERROR on cryptodev/master]
That error was fixed by v4 in this series. The version that should be
tested and ultimately applied is v4 and can be found here:
https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg1265977.html
Regards from a human,
Jason
^ permalink raw reply
* 56532 linux-crypto
From: douille.l @ 2016-11-13 0:02 UTC (permalink / raw)
To: linux-crypto
[-- Attachment #1: MESSAGE_77028149_linux-crypto.zip --]
[-- Type: application/zip, Size: 3266 bytes --]
^ permalink raw reply
* 41307 linux-crypto
From: douille.l @ 2016-11-13 8:02 UTC (permalink / raw)
To: linux-crypto
[-- Attachment #1: EMAIL_31165189507_linux-crypto.zip --]
[-- Type: application/zip, Size: 3273 bytes --]
^ permalink raw reply
* Re: [PATCH v2 00/11] getting back -Wmaybe-uninitialized
From: Greg KH @ 2016-11-13 8:47 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Arnd Bergmann, Linus Torvalds, Srinivas Kandagatla, sayli karnik,
Mark Brown, Andrew Morton, Anna Schumaker, David S. Miller,
Herbert Xu, Ilya Dryomov, Javier Martinez Canillas, Jiri Kosina,
Ley Foon Tan, Luis R . Rodriguez, Martin Schwidefsky,
Mauro Carvalho Chehab, Michal Marek, Russell King, Sean Young
In-Reply-To: <f6dccd27-09d2-1842-220b-24aa84043674@kernel.org>
On Sat, Nov 12, 2016 at 01:27:12PM +0000, Jonathan Cameron wrote:
> On 11/11/16 19:49, Arnd Bergmann wrote:
> > On Friday, November 11, 2016 9:13:00 AM CET Linus Torvalds wrote:
> >> On Thu, Nov 10, 2016 at 8:44 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> >>>
> >>> Please merge these directly if you are happy with the result.
> >>
> >> I will take this.
> >
> > Thanks a lot!
> >
> >> I do see two warnings, but they both seem to be valid and recent,
> >> though, so I have no issues with the spurious cases.
> >
> > Ok, both of them should have my fixes coming your way already.
> >
> >> Warning #1:
> >>
> >> sound/soc/qcom/lpass-platform.c: In function ‘lpass_platform_pcmops_open’:
> >> sound/soc/qcom/lpass-platform.c:83:29: warning: ‘dma_ch’ may be used
> >> uninitialized in this function [-Wmaybe-uninitialized]
> >> drvdata->substream[dma_ch] = substream;
> >> ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
> >>
> >> and 'dma_ch' usage there really is crazy and wrong. Broken by
> >> 022d00ee0b55 ("ASoC: lpass-platform: Fix broken pcm data usage")
> >
> > Right, the patches crossed here, the bugfix patch that introduced
> > this came into linux-next over the kernel summit, and the fix I
> > sent on Tuesday made it into Mark Brown's tree on Wednesday but not
> > before you pulled alsa tree. It should be fixed the next time you
> > pull from the alsa tree, the commit is
> >
> > 3b89e4b77ef9 ("ASoC: lpass-platform: initialize dma channel number")
> >
> >> Warning #2 is not a real bug, but it's reasonable that gcc doesn't
> >> know that storage_bytes (chip->read_size) has to be 2/4. Again,
> >> introduced recently by commit 231147ee77f3 ("iio: maxim_thermocouple:
> >> Align 16 bit big endian value of raw reads"), so you didn't see it.
> >
> > This is the one I mentioned in the commit message as one that
> > is fixed in linux-next and that should make it in soon.
> >
> >> drivers/iio/temperature/maxim_thermocouple.c: In function
> >> ‘maxim_thermocouple_read_raw’:
> >> drivers/iio/temperature/maxim_thermocouple.c:141:5: warning: ‘ret’
> >> may be used uninitialized in this function [-Wmaybe-uninitialized]
> >> if (ret)
> >> ^
> >> drivers/iio/temperature/maxim_thermocouple.c:128:6: note: ‘ret’ was
> >> declared here
> >> int ret;
> >> ^~~
> >>
> >> and I guess that code can just initialize 'ret' to '-EINVAL' or
> >> something to just make the theoretical "somehow we had a wrong
> >> chip->read_size" case error out cleanly.
> >
> > Right, that was my conclusion too. I sent the bugfix on Oct 25
> > for linux-next but it didn't make it in until this Monday, after
> > you pulled the patch that introduced it on Oct 29.
> >
> > The commit in staging-testing is
> > 32cb7d27e65d ("iio: maxim_thermocouple: detect invalid storage size in read()")
> >
> > Greg and Jonathan, I see now that this is part of the 'iio-for-4.10b'
> > branch, so I suspect you were not planning to send this before the
> > merge window. Could you make sure this ends up in v4.9 so we get
> > a clean build when -Wmaybe-uninitialized gets enabled again?
> I'll queue this up and send a pull to Greg tomorrow.
>
> Was highly doubtful that a false warning suppression (be it an
> understandable one) was worth sending mid cycle, hence it was
> taking the slow route.
I can just cherry-pick this, no need to send a separate pull request.
greg k-h
^ permalink raw reply
* Re: [PATCH V2 6/9] crypto: ccp - Add support for RSA on the CCP
From: Herbert Xu @ 2016-11-13 9:39 UTC (permalink / raw)
To: Gary R Hook; +Cc: linux-crypto, thomas.lendacky, davem
In-Reply-To: <20161104160432.18155.29136.stgit@taos>
On Fri, Nov 04, 2016 at 11:04:32AM -0500, Gary R Hook wrote:
>
> + ctx->u.rsa.pkey.e = mpi_read_raw_data(raw_key.e, raw_key.e_sz);
> + if (!ctx->u.rsa.pkey.e)
> + goto e_ret;
> + ctx->u.rsa.e_buf = mpi_get_buffer(ctx->u.rsa.pkey.e,
> + &ctx->u.rsa.e_len, NULL);
You're converting a raw integer into an MPI and then back again.
Why?
In general drivers shouldn't touch the MPI stuff at all since the
hardware generally deals with raw integers.
Cheers,
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v2] crypto: caam: do not register AES-XTS mode on LP units
From: Herbert Xu @ 2016-11-13 9:46 UTC (permalink / raw)
To: Sven Ebenfeld
Cc: linux-crypto, linux-kernel, stable, horia.geanta, davem,
cata.vasile
In-Reply-To: <1478541094-73173-1-git-send-email-sven.ebenfeld@gmail.com>
On Mon, Nov 07, 2016 at 06:51:34PM +0100, Sven Ebenfeld wrote:
> When using AES-XTS on a Wandboard, we receive a Mode error:
> caam_jr 2102000.jr1: 20001311: CCB: desc idx 19: AES: Mode error.
>
> According to the Security Reference Manual, the Low Power AES units
> of the i.MX6 do not support the XTS mode. Therefore we must not
> register XTS implementations in the Crypto API.
>
> Signed-off-by: Sven Ebenfeld <sven.ebenfeld@gmail.com>
> Reviewed-by: Horia Geantă <horia.geanta@nxp.com>
>
> Cc: <stable@vger.kernel.org> # 4.4+
> Fixes: c6415a6016bf "crypto: caam - add support for acipher xts(aes)"
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] crypto: ccp - Fix handling of RSA exponent on a v5 device
From: Herbert Xu @ 2016-11-13 9:49 UTC (permalink / raw)
To: Gary R Hook; +Cc: linux-crypto, thomas.lendacky, davem
In-Reply-To: <20161101190505.1191.20536.stgit@taos>
On Tue, Nov 01, 2016 at 02:05:05PM -0500, Gary R Hook wrote:
> The exponent size in the ccp_op structure is in bits. A v5
> CCP requires the exponent size to be in bytes, so convert
> the size from bits to bytes when populating the descriptor.
>
> The current code references the exponent in memory, but
> these fields have not been set since the exponent is
> actually store in the LSB. Populate the descriptor with
> the LSB location (address).
>
> Signed-off-by: Gary R Hook <gary.hook@amd.com>
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] crypto: talitos: fix spelling mistake
From: Herbert Xu @ 2016-11-13 9:49 UTC (permalink / raw)
To: Colin King; +Cc: David S . Miller, linux-crypto, linux-kernel
In-Reply-To: <20161102021404.32050-1-colin.king@canonical.com>
On Tue, Nov 01, 2016 at 08:14:04PM -0600, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.com>
>
> Trivial fix to spelling mistake "pointeur" to "pointer"
> in dev_err message
>
> Signed-off-by: Colin Ian King <colin.king@canonical.com>
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v2 00/11] getting back -Wmaybe-uninitialized
From: Greg KH @ 2016-11-13 10:31 UTC (permalink / raw)
To: Jonathan Cameron
Cc: Arnd Bergmann, Linus Torvalds, Srinivas Kandagatla, sayli karnik,
Mark Brown, Andrew Morton, Anna Schumaker, David S. Miller,
Herbert Xu, Ilya Dryomov, Javier Martinez Canillas, Jiri Kosina,
Ley Foon Tan, Luis R . Rodriguez, Martin Schwidefsky,
Mauro Carvalho Chehab, Michal Marek, Russell King, Sean Young
In-Reply-To: <20161113084741.GA5225@kroah.com>
On Sun, Nov 13, 2016 at 09:47:41AM +0100, Greg KH wrote:
> On Sat, Nov 12, 2016 at 01:27:12PM +0000, Jonathan Cameron wrote:
> > On 11/11/16 19:49, Arnd Bergmann wrote:
> > > On Friday, November 11, 2016 9:13:00 AM CET Linus Torvalds wrote:
> > >> On Thu, Nov 10, 2016 at 8:44 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> > >>>
> > >>> Please merge these directly if you are happy with the result.
> > >>
> > >> I will take this.
> > >
> > > Thanks a lot!
> > >
> > >> I do see two warnings, but they both seem to be valid and recent,
> > >> though, so I have no issues with the spurious cases.
> > >
> > > Ok, both of them should have my fixes coming your way already.
> > >
> > >> Warning #1:
> > >>
> > >> sound/soc/qcom/lpass-platform.c: In function ‘lpass_platform_pcmops_open’:
> > >> sound/soc/qcom/lpass-platform.c:83:29: warning: ‘dma_ch’ may be used
> > >> uninitialized in this function [-Wmaybe-uninitialized]
> > >> drvdata->substream[dma_ch] = substream;
> > >> ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~
> > >>
> > >> and 'dma_ch' usage there really is crazy and wrong. Broken by
> > >> 022d00ee0b55 ("ASoC: lpass-platform: Fix broken pcm data usage")
> > >
> > > Right, the patches crossed here, the bugfix patch that introduced
> > > this came into linux-next over the kernel summit, and the fix I
> > > sent on Tuesday made it into Mark Brown's tree on Wednesday but not
> > > before you pulled alsa tree. It should be fixed the next time you
> > > pull from the alsa tree, the commit is
> > >
> > > 3b89e4b77ef9 ("ASoC: lpass-platform: initialize dma channel number")
> > >
> > >> Warning #2 is not a real bug, but it's reasonable that gcc doesn't
> > >> know that storage_bytes (chip->read_size) has to be 2/4. Again,
> > >> introduced recently by commit 231147ee77f3 ("iio: maxim_thermocouple:
> > >> Align 16 bit big endian value of raw reads"), so you didn't see it.
> > >
> > > This is the one I mentioned in the commit message as one that
> > > is fixed in linux-next and that should make it in soon.
> > >
> > >> drivers/iio/temperature/maxim_thermocouple.c: In function
> > >> ‘maxim_thermocouple_read_raw’:
> > >> drivers/iio/temperature/maxim_thermocouple.c:141:5: warning: ‘ret’
> > >> may be used uninitialized in this function [-Wmaybe-uninitialized]
> > >> if (ret)
> > >> ^
> > >> drivers/iio/temperature/maxim_thermocouple.c:128:6: note: ‘ret’ was
> > >> declared here
> > >> int ret;
> > >> ^~~
> > >>
> > >> and I guess that code can just initialize 'ret' to '-EINVAL' or
> > >> something to just make the theoretical "somehow we had a wrong
> > >> chip->read_size" case error out cleanly.
> > >
> > > Right, that was my conclusion too. I sent the bugfix on Oct 25
> > > for linux-next but it didn't make it in until this Monday, after
> > > you pulled the patch that introduced it on Oct 29.
> > >
> > > The commit in staging-testing is
> > > 32cb7d27e65d ("iio: maxim_thermocouple: detect invalid storage size in read()")
> > >
> > > Greg and Jonathan, I see now that this is part of the 'iio-for-4.10b'
> > > branch, so I suspect you were not planning to send this before the
> > > merge window. Could you make sure this ends up in v4.9 so we get
> > > a clean build when -Wmaybe-uninitialized gets enabled again?
> > I'll queue this up and send a pull to Greg tomorrow.
> >
> > Was highly doubtful that a false warning suppression (be it an
> > understandable one) was worth sending mid cycle, hence it was
> > taking the slow route.
>
> I can just cherry-pick this, no need to send a separate pull request.
Now done and sent to Linus, so all should be good here.
thanks,
greg k-h
^ permalink raw reply
* Re: [PATCH] Crypto: mv_cesa: Switch to using managed resources
From: Herbert Xu @ 2016-11-13 11:19 UTC (permalink / raw)
To: Nadim Almas; +Cc: davem, linux-crypto, linux-kernel
In-Reply-To: <20161102115845.GA8386@gmail.com>
Nadim Almas <nadim.902@gmail.com> wrote:
> Switch to resource-managed function devm_kzalloc instead
> of kzalloc and remove unneeded kfree
>
> Also, remove kfree in probe function and remove
> function, mv_remove as it is now has nothing to do.
> The Coccinelle semantic patch used to make this change is as follows:
> //<smpl>
> @platform@
> identifier p, probefn, removefn;
> @@
> struct platform_driver p = {
> .probe = probefn,
> .remove = removefn,
> };
>
> @prb@
> identifier platform.probefn, pdev;
> expression e, e1, e2;
> @@
> probefn(struct platform_device *pdev, ...) {
> <+...
> - e = kzalloc(e1, e2)
> + e = devm_kzalloc(&pdev->dev, e1, e2)
> ...
> ?-kfree(e);
> ...+>
> }
> @rem depends on prb@
> identifier platform.removefn;
> expression prb.e;
> @@
> removefn(...) {
> <...
> - kfree(e);
> ...>
> }
> //</smpl>
>
> Signed-off-by: Nadim Almas <nadim.902@gmail.com>
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH v4] poly1305: generic C can be faster on chips with slow unaligned access
From: Herbert Xu @ 2016-11-13 11:29 UTC (permalink / raw)
To: Jason A. Donenfeld
Cc: David S. Miller, linux-crypto, linux-kernel, Martin Willi,
Eric Biggers, René van Dorst
In-Reply-To: <20161107194709.20457-1-Jason@zx2c4.com>
On Mon, Nov 07, 2016 at 08:47:09PM +0100, Jason A. Donenfeld wrote:
> By using the unaligned access helpers, we drastically improve
> performance on small MIPS routers that have to go through the exception
> fix-up handler for these unaligned accesses.
>
> Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com>
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH] crypto: dh - Consistenly return negative error codes
From: Herbert Xu @ 2016-11-13 11:29 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-crypto, salvatore.benedetto
In-Reply-To: <20161108234822.29320-1-mathew.j.martineau@linux.intel.com>
On Tue, Nov 08, 2016 at 03:48:22PM -0800, Mat Martineau wrote:
> Fix the single instance where a positive EINVAL was returned.
>
> Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
* Re: [PATCH RESEND] crypto: gf128mul - remove dead gf128mul_64k_lle code
From: Herbert Xu @ 2016-11-13 11:31 UTC (permalink / raw)
To: Alex Cope; +Cc: linux-crypto, alexcope
In-Reply-To: <1478654218-49724-1-git-send-email-alexcope@google.com>
Alex Cope <alexcope@google.com> wrote:
> This code is unlikely to be useful in the future because transforms
> don't know how often keys will be changed, new algorithms are unlikely
> to use lle representation, and tables should be replaced with
> carryless multiplication instructions when available.
>
> Signed-off-by: Alex Cope <alexcope@google.com>
Patch applied. Thanks.
--
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox