* Re: [PATCH 1/16] crypto: skcipher - Add skcipher walk interface
From: Herbert Xu @ 2016-11-11 11:19 UTC (permalink / raw)
To: Eric Biggers; +Cc: Linux Crypto Mailing List
In-Reply-To: <20161102205420.GA17645@google.com>
On Wed, Nov 02, 2016 at 01:54:20PM -0700, Eric Biggers wrote:
>
> I think the case where skcipher_copy_iv() fails may be handled incorrectly.
> Wouldn't it need to set walk.nbytes to 0 so as to not confuse callers which
> expect that behavior? Or maybe it should be calling skcipher_walk_done().
Good catch. I'll fix and repost.
> Setting walk->in.sg and walk->out.sg is redundant with the scatterwalk_start()
> calls.
Will remove.
> This gets called with uninitialized 'walk.flags'. This was somewhat of a
> theoretical problem with the old blkcipher_walk code but it looks like now it
> will interact badly with the new SKCIPHER_WALK_SLEEP flag. As far as I can see,
> whether the flag will end up set or not can depend on the uninitialized value.
> It would be nice if this problem could be avoided entirely be setting flags=0.
Right. I'll fix this as well.
> I'm also wondering about the choice to not look at 'atomic' until after the call
> to skcipher_walk_skcipher(). Wouldn't this mean that the choice of 'atomic'
> would not be respected in e.g. the kmalloc() in skcipher_copy_iv()?
The atomic flag is meant to be used in cases such as aesni where
you need to do kernel_fpu_begin after the call to start the walk.
IOW sleeping is fine at the start but not on subsequent walk calls.
> I don't see any users of the "async" walking being introduced; are some planned?
skcipher_walk is meant to unite blkcipher_walk and ablkcipher_walk.
The latter will use the async case.
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
* [PATCH v2 11/11] Kbuild: enable -Wmaybe-uninitialized warnings by default
From: Arnd Bergmann @ 2016-11-10 16:44 UTC (permalink / raw)
To: Linus Torvalds
Cc: Sean Young, Trond Myklebust, linux-s390, Herbert Xu, x86,
Sebastian Ott, Russell King, Javier Martinez Canillas,
Ilya Dryomov, linux-snps-arc, linux-media, Arnd Bergmann,
linux-kbuild, Jiri Kosina, Michal Marek, nios2-dev,
Mauro Carvalho Chehab, linux-nfs, linux-kernel, Anna Schumaker,
Luis R . Rodriguez, linux-crypto, Martin Schwidefsky,
Ley Foon Tan, Andrew Morton
In-Reply-To: <20161110164454.293477-1-arnd@arndb.de>
Previously the warnings were added back at the W=1 level and above,
this now turns them on again by default, assuming that we have addressed
all warnings and again have a clean build for v4.10.
I found a number of new warnings in linux-next already and submitted
bugfixes for those. Hopefully they are caught by the 0day builder
in the future as soon as this patch is merged.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Please check if there are any remaining warnings with this
patch before applying.
The one known issue right now is commit 32cb7d27e65d ("iio:
maxim_thermocouple: detect invalid storage size in read()"), which
is currently in linux-next but not yet in mainline.
There are a couple of warnings that I get with randconfig builds,
and I have submitted patches for all of them at some point and
will follow up on them to make sure they get addressed eventually.
---
| 2 --
1 file changed, 2 deletions(-)
--git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 7fc2c5a..7c321a6 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -60,8 +60,6 @@ endif
KBUILD_CFLAGS += $(warning)
else
-KBUILD_CFLAGS += $(call cc-disable-warning, maybe-uninitialized)
-
ifeq ($(cc-name),clang)
KBUILD_CFLAGS += $(call cc-disable-warning, initializer-overrides)
KBUILD_CFLAGS += $(call cc-disable-warning, unused-value)
--
2.9.0
^ permalink raw reply related
* [PATCH v2 10/11] pcmcia: fix return value of soc_pcmcia_regulator_set
From: Arnd Bergmann @ 2016-11-10 16:44 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andrew Morton, Arnd Bergmann, Anna Schumaker, David S. Miller,
Herbert Xu, Ilya Dryomov, Javier Martinez Canillas, Jiri Kosina,
Jonathan Cameron, Ley Foon Tan, Luis R . Rodriguez,
Martin Schwidefsky, Mauro Carvalho Chehab, Michal Marek,
Russell King, Sean Young, Sebastian Ott, Trond Myklebust, x86,
linux-kbuild
In-Reply-To: <20161110164454.293477-1-arnd@arndb.de>
The newly introduced soc_pcmcia_regulator_set() function sometimes returns
without setting its return code, as shown by this warning:
drivers/pcmcia/soc_common.c: In function 'soc_pcmcia_regulator_set':
drivers/pcmcia/soc_common.c:112:5: error: 'ret' may be used uninitialized in this function [-Werror=maybe-uninitialized]
This changes it to propagate the regulator_disable() result instead.
Fixes: ac61b6001a63 ("pcmcia: soc_common: add support for Vcc and Vpp regulators")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/pcmcia/soc_common.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/pcmcia/soc_common.c b/drivers/pcmcia/soc_common.c
index 153f312..b6b316d 100644
--- a/drivers/pcmcia/soc_common.c
+++ b/drivers/pcmcia/soc_common.c
@@ -107,7 +107,7 @@ int soc_pcmcia_regulator_set(struct soc_pcmcia_socket *skt,
ret = regulator_enable(r->reg);
} else {
- regulator_disable(r->reg);
+ ret = regulator_disable(r->reg);
}
if (ret == 0)
r->on = on;
--
2.9.0
^ permalink raw reply related
* [PATCH v2 09/11] [v3] infiniband: shut up a maybe-uninitialized warning
From: Arnd Bergmann @ 2016-11-10 16:44 UTC (permalink / raw)
To: Linus Torvalds
Cc: Sean Young, Trond Myklebust, linux-s390, Herbert Xu, x86,
Sebastian Ott, Russell King, Javier Martinez Canillas,
Ilya Dryomov, linux-snps-arc, linux-media, Arnd Bergmann,
linux-kbuild, Jiri Kosina, Michal Marek, nios2-dev,
Mauro Carvalho Chehab, linux-nfs, linux-kernel, Anna Schumaker,
Luis R . Rodriguez, linux-crypto, Martin Schwidefsky,
Ley Foon Tan, Andrew Morton
In-Reply-To: <20161110164454.293477-1-arnd@arndb.de>
Some configurations produce this harmless warning when built with
gcc -Wmaybe-uninitialized:
infiniband/core/cma.c: In function 'cma_get_net_dev':
infiniband/core/cma.c:1242:12: warning: 'src_addr_storage.sin_addr.s_addr' may be used uninitialized in this function [-Wmaybe-uninitialized]
I previously reported this for the powerpc64 defconfig, but have now
reproduced the same thing for x86 as well, using gcc-5 or higher.
The code looks correct to me, and this change just rearranges it
by making sure we alway initialize the entire address structure
to make the warning disappear. My first approach added an
initialization at the time of the declaration, which Doug commented
may be too costly, so I hope this version doesn't add overhead.
Link: http://arm-soc.lixom.net/buildlogs/mainline/v4.7-rc6/buildall.powerpc.ppc64_defconfig.log.passed
Link: https://patchwork.kernel.org/patch/9212825/
Acked-by: Haggai Eran <haggaie@mellanox.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
This is marked v2 as the rest of the series but is actually version
three of the patch as I had to do some other changes already.
v3: remove accidental leftover change of the original patch
drivers/infiniband/core/cma.c | 54 ++++++++++++++++++++++---------------------
1 file changed, 28 insertions(+), 26 deletions(-)
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
index 36bf50e..89a6b05 100644
--- a/drivers/infiniband/core/cma.c
+++ b/drivers/infiniband/core/cma.c
@@ -1094,47 +1094,47 @@ static void cma_save_ib_info(struct sockaddr *src_addr,
}
}
-static void cma_save_ip4_info(struct sockaddr *src_addr,
- struct sockaddr *dst_addr,
+static void cma_save_ip4_info(struct sockaddr_in *src_addr,
+ struct sockaddr_in *dst_addr,
struct cma_hdr *hdr,
__be16 local_port)
{
- struct sockaddr_in *ip4;
-
if (src_addr) {
- ip4 = (struct sockaddr_in *)src_addr;
- ip4->sin_family = AF_INET;
- ip4->sin_addr.s_addr = hdr->dst_addr.ip4.addr;
- ip4->sin_port = local_port;
+ *src_addr = (struct sockaddr_in) {
+ .sin_family = AF_INET,
+ .sin_addr.s_addr = hdr->dst_addr.ip4.addr,
+ .sin_port = local_port,
+ };
}
if (dst_addr) {
- ip4 = (struct sockaddr_in *)dst_addr;
- ip4->sin_family = AF_INET;
- ip4->sin_addr.s_addr = hdr->src_addr.ip4.addr;
- ip4->sin_port = hdr->port;
+ *dst_addr = (struct sockaddr_in) {
+ .sin_family = AF_INET,
+ .sin_addr.s_addr = hdr->src_addr.ip4.addr,
+ .sin_port = hdr->port,
+ };
}
}
-static void cma_save_ip6_info(struct sockaddr *src_addr,
- struct sockaddr *dst_addr,
+static void cma_save_ip6_info(struct sockaddr_in6 *src_addr,
+ struct sockaddr_in6 *dst_addr,
struct cma_hdr *hdr,
__be16 local_port)
{
- struct sockaddr_in6 *ip6;
-
if (src_addr) {
- ip6 = (struct sockaddr_in6 *)src_addr;
- ip6->sin6_family = AF_INET6;
- ip6->sin6_addr = hdr->dst_addr.ip6;
- ip6->sin6_port = local_port;
+ *src_addr = (struct sockaddr_in6) {
+ .sin6_family = AF_INET6,
+ .sin6_addr = hdr->dst_addr.ip6,
+ .sin6_port = local_port,
+ };
}
if (dst_addr) {
- ip6 = (struct sockaddr_in6 *)dst_addr;
- ip6->sin6_family = AF_INET6;
- ip6->sin6_addr = hdr->src_addr.ip6;
- ip6->sin6_port = hdr->port;
+ *dst_addr = (struct sockaddr_in6) {
+ .sin6_family = AF_INET6,
+ .sin6_addr = hdr->src_addr.ip6,
+ .sin6_port = hdr->port,
+ };
}
}
@@ -1159,10 +1159,12 @@ static int cma_save_ip_info(struct sockaddr *src_addr,
switch (cma_get_ip_ver(hdr)) {
case 4:
- cma_save_ip4_info(src_addr, dst_addr, hdr, port);
+ cma_save_ip4_info((struct sockaddr_in *)src_addr,
+ (struct sockaddr_in *)dst_addr, hdr, port);
break;
case 6:
- cma_save_ip6_info(src_addr, dst_addr, hdr, port);
+ cma_save_ip6_info((struct sockaddr_in6 *)src_addr,
+ (struct sockaddr_in6 *)dst_addr, hdr, port);
break;
default:
return -EAFNOSUPPORT;
--
2.9.0
^ permalink raw reply related
* [PATCH v2 08/11] crypto: aesni: shut up -Wmaybe-uninitialized warning
From: Arnd Bergmann @ 2016-11-10 16:44 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andrew Morton, Arnd Bergmann, Anna Schumaker, David S. Miller,
Herbert Xu, Ilya Dryomov, Javier Martinez Canillas, Jiri Kosina,
Jonathan Cameron, Ley Foon Tan, Luis R . Rodriguez,
Martin Schwidefsky, Mauro Carvalho Chehab, Michal Marek,
Russell King, Sean Young, Sebastian Ott, Trond Myklebust, x86,
linux-kbuild
In-Reply-To: <20161110164454.293477-1-arnd@arndb.de>
The rfc4106 encrypy/decrypt helper functions cause an annoying
false-positive warning in allmodconfig if we turn on
-Wmaybe-uninitialized warnings again:
arch/x86/crypto/aesni-intel_glue.c: In function ‘helper_rfc4106_decrypt’:
include/linux/scatterlist.h:67:31: warning: ‘dst_sg_walk.sg’ may be used uninitialized in this function [-Wmaybe-uninitialized]
The problem seems to be that the compiler doesn't track the state of the
'one_entry_in_sg' variable across the kernel_fpu_begin/kernel_fpu_end
section.
This takes the easy way out by adding a bogus initialization, which
should be harmless enough to get the patch into v4.9 so we can turn
on this warning again by default without producing useless output.
A follow-up patch for v4.10 rearranges the code to make the warning
go away.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
arch/x86/crypto/aesni-intel_glue.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/arch/x86/crypto/aesni-intel_glue.c b/arch/x86/crypto/aesni-intel_glue.c
index 0ab5ee1..aa8b067 100644
--- a/arch/x86/crypto/aesni-intel_glue.c
+++ b/arch/x86/crypto/aesni-intel_glue.c
@@ -888,7 +888,7 @@ static int helper_rfc4106_encrypt(struct aead_request *req)
unsigned long auth_tag_len = crypto_aead_authsize(tfm);
u8 iv[16] __attribute__ ((__aligned__(AESNI_ALIGN)));
struct scatter_walk src_sg_walk;
- struct scatter_walk dst_sg_walk;
+ struct scatter_walk dst_sg_walk = {};
unsigned int i;
/* Assuming we are supporting rfc4106 64-bit extended */
@@ -968,7 +968,7 @@ static int helper_rfc4106_decrypt(struct aead_request *req)
u8 iv[16] __attribute__ ((__aligned__(AESNI_ALIGN)));
u8 authTag[16];
struct scatter_walk src_sg_walk;
- struct scatter_walk dst_sg_walk;
+ struct scatter_walk dst_sg_walk = {};
unsigned int i;
if (unlikely(req->assoclen != 16 && req->assoclen != 20))
--
2.9.0
^ permalink raw reply related
* [PATCH v2 07/11] [media] rc: print correct variable for z8f0811
From: Arnd Bergmann @ 2016-11-10 16:44 UTC (permalink / raw)
To: Linus Torvalds
Cc: Sean Young, Trond Myklebust, linux-s390, Herbert Xu, x86,
Sebastian Ott, Russell King, Javier Martinez Canillas,
Ilya Dryomov, linux-snps-arc, linux-media, Arnd Bergmann,
linux-kbuild, Jiri Kosina, Michal Marek, nios2-dev,
Mauro Carvalho Chehab, linux-nfs, linux-kernel, Anna Schumaker,
Luis R . Rodriguez, linux-crypto, Martin Schwidefsky,
Ley Foon Tan, Andrew Morton
In-Reply-To: <20161110164454.293477-1-arnd@arndb.de>
A recent rework accidentally left a debugging printk untouched
while changing the meaning of the variables, leading to an
uninitialized variable being printed:
drivers/media/i2c/ir-kbd-i2c.c: In function 'get_key_haup_common':
drivers/media/i2c/ir-kbd-i2c.c:62:2: error: 'toggle' may be used uninitialized in this function [-Werror=maybe-uninitialized]
This prints the correct one instead, as we did before the patch.
Fixes: 00bb820755ed ("[media] rc: Hauppauge z8f0811 can decode RC6")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/media/i2c/ir-kbd-i2c.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
I submitted this repeatedly as it is a v4.9 regression, but
I never saw a reply for it.
diff --git a/drivers/media/i2c/ir-kbd-i2c.c b/drivers/media/i2c/ir-kbd-i2c.c
index f95a6bc..cede397 100644
--- a/drivers/media/i2c/ir-kbd-i2c.c
+++ b/drivers/media/i2c/ir-kbd-i2c.c
@@ -118,7 +118,7 @@ static int get_key_haup_common(struct IR_i2c *ir, enum rc_type *protocol,
*protocol = RC_TYPE_RC6_MCE;
dev &= 0x7f;
dprintk(1, "ir hauppauge (rc6-mce): t%d vendor=%d dev=%d code=%d\n",
- toggle, vendor, dev, code);
+ *ptoggle, vendor, dev, code);
} else {
*ptoggle = 0;
*protocol = RC_TYPE_RC6_6A_32;
--
2.9.0
^ permalink raw reply related
* [PATCH v2 06/11] [media] dib0700: fix nec repeat handling
From: Arnd Bergmann @ 2016-11-10 16:44 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andrew Morton, Arnd Bergmann, Anna Schumaker, David S. Miller,
Herbert Xu, Ilya Dryomov, Javier Martinez Canillas, Jiri Kosina,
Jonathan Cameron, Ley Foon Tan, Luis R . Rodriguez,
Martin Schwidefsky, Mauro Carvalho Chehab, Michal Marek,
Russell King, Sean Young, Sebastian Ott, Trond Myklebust, x86,
linux-kbuild
In-Reply-To: <20161110164454.293477-1-arnd@arndb.de>
From: Sean Young <sean@mess.org>
When receiving a nec repeat, ensure the correct scancode is repeated
rather than a random value from the stack. This removes the need
for the bogus uninitialized_var() and also fixes the warnings:
drivers/media/usb/dvb-usb/dib0700_core.c: In function ‘dib0700_rc_urb_completion’:
drivers/media/usb/dvb-usb/dib0700_core.c:679: warning: ‘protocol’ may be used uninitialized in this function
[sean addon: So after writing the patch and submitting it, I've bought the
hardware on ebay. Without this patch you get random scancodes
on nec repeats, which the patch indeed fixes.]
Signed-off-by: Sean Young <sean@mess.org>
Tested-by: Sean Young <sean@mess.org>
Cc: stable@vger.kernel.org
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
drivers/media/usb/dvb-usb/dib0700_core.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/drivers/media/usb/dvb-usb/dib0700_core.c b/drivers/media/usb/dvb-usb/dib0700_core.c
index 92d5408..47ce9d5 100644
--- a/drivers/media/usb/dvb-usb/dib0700_core.c
+++ b/drivers/media/usb/dvb-usb/dib0700_core.c
@@ -704,7 +704,7 @@ static void dib0700_rc_urb_completion(struct urb *purb)
struct dvb_usb_device *d = purb->context;
struct dib0700_rc_response *poll_reply;
enum rc_type protocol;
- u32 uninitialized_var(keycode);
+ u32 keycode;
u8 toggle;
deb_info("%s()\n", __func__);
@@ -745,7 +745,8 @@ static void dib0700_rc_urb_completion(struct urb *purb)
poll_reply->nec.data == 0x00 &&
poll_reply->nec.not_data == 0xff) {
poll_reply->data_state = 2;
- break;
+ rc_repeat(d->rc_dev);
+ goto resubmit;
}
if ((poll_reply->nec.data ^ poll_reply->nec.not_data) != 0xff) {
--
2.9.0
^ permalink raw reply related
* [PATCH v2 05/11] s390: pci: don't print uninitialized data for debugging
From: Arnd Bergmann @ 2016-11-10 16:44 UTC (permalink / raw)
To: Linus Torvalds
Cc: Sean Young, Trond Myklebust, linux-s390, Herbert Xu, x86,
Sebastian Ott, Russell King, Javier Martinez Canillas,
Ilya Dryomov, linux-snps-arc, linux-media, Arnd Bergmann,
linux-kbuild, Jiri Kosina, Michal Marek, nios2-dev,
Mauro Carvalho Chehab, linux-nfs, linux-kernel, Anna Schumaker,
Luis R . Rodriguez, linux-crypto, Martin Schwidefsky,
Ley Foon Tan, Andrew Morton
In-Reply-To: <20161110164454.293477-1-arnd@arndb.de>
gcc correctly warns about an incorrect use of the 'pa' variable
in case we pass an empty scatterlist to __s390_dma_map_sg:
arch/s390/pci/pci_dma.c: In function '__s390_dma_map_sg':
arch/s390/pci/pci_dma.c:309:13: warning: 'pa' may be used uninitialized in this function [-Wmaybe-uninitialized]
This adds a bogus initialization to the function to sanitize
the debug output. I would have preferred a solution without
the initialization, but I only got the report from the
kbuild bot after turning on the warning again, and didn't
manage to reproduce it myself.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Acked-by: Sebastian Ott <sebott@linux.vnet.ibm.com>
Acked-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
---
arch/s390/pci/pci_dma.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/s390/pci/pci_dma.c b/arch/s390/pci/pci_dma.c
index 7350c8b..6b2f72f 100644
--- a/arch/s390/pci/pci_dma.c
+++ b/arch/s390/pci/pci_dma.c
@@ -423,7 +423,7 @@ static int __s390_dma_map_sg(struct device *dev, struct scatterlist *sg,
dma_addr_t dma_addr_base, dma_addr;
int flags = ZPCI_PTE_VALID;
struct scatterlist *s;
- unsigned long pa;
+ unsigned long pa = 0;
int ret;
size = PAGE_ALIGN(size);
--
2.9.0
^ permalink raw reply related
* [PATCH v2 04/11] nios2: fix timer initcall return value
From: Arnd Bergmann @ 2016-11-10 16:44 UTC (permalink / raw)
To: Linus Torvalds
Cc: Sean Young, Trond Myklebust, linux-s390, Herbert Xu, x86,
Sebastian Ott, Russell King, Javier Martinez Canillas,
Ilya Dryomov, linux-snps-arc, linux-media, Arnd Bergmann,
linux-kbuild, Jiri Kosina, Michal Marek, nios2-dev,
Mauro Carvalho Chehab, linux-nfs, linux-kernel, Anna Schumaker,
Luis R . Rodriguez, linux-crypto, Martin Schwidefsky,
Ley Foon Tan, Andrew Morton
In-Reply-To: <20161110164454.293477-1-arnd@arndb.de>
When called more than twice, the nios2_time_init() function
return an uninitialized value, as detected by gcc -Wmaybe-uninitialized
arch/nios2/kernel/time.c: warning: 'ret' may be used uninitialized in this function
This makes it return '0' here, matching the comment above the
function.
Acked-by: Ley Foon Tan <lftan@altera.com>
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
arch/nios2/kernel/time.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/nios2/kernel/time.c b/arch/nios2/kernel/time.c
index d9563dd..746bf5c 100644
--- a/arch/nios2/kernel/time.c
+++ b/arch/nios2/kernel/time.c
@@ -324,6 +324,7 @@ static int __init nios2_time_init(struct device_node *timer)
ret = nios2_clocksource_init(timer);
break;
default:
+ ret = 0;
break;
}
--
2.9.0
^ permalink raw reply related
* [PATCH v2 03/11] x86: apm: avoid uninitialized data
From: Arnd Bergmann @ 2016-11-10 16:44 UTC (permalink / raw)
To: Linus Torvalds
Cc: Andrew Morton, Arnd Bergmann, Anna Schumaker, David S. Miller,
Herbert Xu, Ilya Dryomov, Javier Martinez Canillas, Jiri Kosina,
Jonathan Cameron, Ley Foon Tan, Luis R . Rodriguez,
Martin Schwidefsky, Mauro Carvalho Chehab, Michal Marek,
Russell King, Sean Young, Sebastian Ott, Trond Myklebust, x86,
linux-kbuild
In-Reply-To: <20161110164454.293477-1-arnd@arndb.de>
apm_bios_call() can fail, and return a status in its argument
structure. If that status however is zero during a call from
apm_get_power_status(), we end up using data that may have
never been set, as reported by "gcc -Wmaybe-uninitialized":
arch/x86/kernel/apm_32.c: In function ‘apm’:
arch/x86/kernel/apm_32.c:1729:17: error: ‘bx’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
arch/x86/kernel/apm_32.c:1835:5: error: ‘cx’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
arch/x86/kernel/apm_32.c:1730:17: note: ‘cx’ was declared here
arch/x86/kernel/apm_32.c:1842:27: error: ‘dx’ may be used uninitialized in this function [-Werror=maybe-uninitialized]
arch/x86/kernel/apm_32.c:1731:17: note: ‘dx’ was declared here
This changes the function to return "APM_NO_ERROR" here, which
makes the code more robust to broken BIOS versions, and avoids
the warning.
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Jiri Kosina <jkosina@suse.cz>
Reviewed-by: Luis R. Rodriguez <mcgrof@kernel.org>
---
arch/x86/kernel/apm_32.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/apm_32.c b/arch/x86/kernel/apm_32.c
index c7364bd..51287cd 100644
--- a/arch/x86/kernel/apm_32.c
+++ b/arch/x86/kernel/apm_32.c
@@ -1042,8 +1042,11 @@ static int apm_get_power_status(u_short *status, u_short *bat, u_short *life)
if (apm_info.get_power_status_broken)
return APM_32_UNSUPPORTED;
- if (apm_bios_call(&call))
+ if (apm_bios_call(&call)) {
+ if (!call.err)
+ return APM_NO_ERROR;
return call.err;
+ }
*status = call.ebx;
*bat = call.ecx;
if (apm_info.get_power_status_swabinminutes) {
--
2.9.0
^ permalink raw reply related
* [PATCH v2 02/11] NFSv4.1: work around -Wmaybe-uninitialized warning
From: Arnd Bergmann @ 2016-11-10 16:44 UTC (permalink / raw)
To: Linus Torvalds
Cc: Sean Young, Trond Myklebust, linux-s390, Herbert Xu, x86,
Sebastian Ott, Russell King, Javier Martinez Canillas,
Ilya Dryomov, linux-snps-arc, linux-media, Arnd Bergmann,
linux-kbuild, Jiri Kosina, Michal Marek, nios2-dev,
Mauro Carvalho Chehab, linux-nfs, linux-kernel, Anna Schumaker,
Luis R . Rodriguez, linux-crypto, Martin Schwidefsky,
Ley Foon Tan, Andrew Morton
In-Reply-To: <20161110164454.293477-1-arnd@arndb.de>
A bugfix introduced a harmless gcc warning in nfs4_slot_seqid_in_use
if we enable -Wmaybe-uninitialized again:
fs/nfs/nfs4session.c:203:54: error: 'cur_seq' may be used uninitialized in this function [-Werror=maybe-uninitialized]
gcc is not smart enough to conclude that the IS_ERR/PTR_ERR pair
results in a nonzero return value here. Using PTR_ERR_OR_ZERO()
instead makes this clear to the compiler.
Fixes: e09c978aae5b ("NFSv4.1: Fix Oopsable condition in server callback races")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
First submitted on Aug 31, but ended up not getting applied then
as the warning was disabled in v4.8-rc
Anna Schumaker said at the kernel summit that she had applied
it and would send it for 4.9, but as of 2016-11-09 it has not
made it into linux-next.
fs/nfs/nfs4session.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/fs/nfs/nfs4session.c b/fs/nfs/nfs4session.c
index b629730..150c5a1 100644
--- a/fs/nfs/nfs4session.c
+++ b/fs/nfs/nfs4session.c
@@ -178,12 +178,14 @@ static int nfs4_slot_get_seqid(struct nfs4_slot_table *tbl, u32 slotid,
__must_hold(&tbl->slot_tbl_lock)
{
struct nfs4_slot *slot;
+ int ret;
slot = nfs4_lookup_slot(tbl, slotid);
- if (IS_ERR(slot))
- return PTR_ERR(slot);
- *seq_nr = slot->seq_nr;
- return 0;
+ ret = PTR_ERR_OR_ZERO(slot);
+ if (!ret)
+ *seq_nr = slot->seq_nr;
+
+ return ret;
}
/*
--
2.9.0
^ permalink raw reply related
* [PATCH v2 01/11] Kbuild: enable -Wmaybe-uninitialized warning for "make W=1"
From: Arnd Bergmann @ 2016-11-10 16:44 UTC (permalink / raw)
To: Linus Torvalds
Cc: Sean Young, Trond Myklebust, linux-s390, Herbert Xu, x86,
Sebastian Ott, Russell King, Javier Martinez Canillas,
Ilya Dryomov, linux-snps-arc, linux-media, Arnd Bergmann,
linux-kbuild, Jiri Kosina, Michal Marek, nios2-dev,
Mauro Carvalho Chehab, linux-nfs, linux-kernel, Anna Schumaker,
Luis R . Rodriguez, linux-crypto, Martin Schwidefsky,
Ley Foon Tan, Andrew Morton
In-Reply-To: <20161110164454.293477-1-arnd@arndb.de>
Traditionally, we have always had warnings about uninitialized variables
enabled, as this is part of -Wall, and generally a good idea [1], but it
also always produced false positives, mainly because this is a variation
of the halting problem and provably impossible to get right in all cases
[2].
Various people have identified cases that are particularly bad for false
positives, and in commit e74fc973b6e5 ("Turn off -Wmaybe-uninitialized
when building with -Os"), I turned off the warning for any build that
was done with CC_OPTIMIZE_FOR_SIZE. This drastically reduced the number
of false positive warnings in the default build but unfortunately had
the side effect of turning the warning off completely in 'allmodconfig'
builds, which in turn led to a lot of warnings (both actual bugs, and
remaining false positives) to go in unnoticed.
With commit 877417e6ffb9 ("Kbuild: change CC_OPTIMIZE_FOR_SIZE
definition") enabled the warning again for allmodconfig builds in v4.7
and in v4.8-rc1, I had finally managed to address all warnings I get in
an ARM allmodconfig build and most other maybe-uninitialized warnings
for ARM randconfig builds.
However, commit 6e8d666e9253 ("Disable "maybe-uninitialized" warning
globally") was merged at the same time and disabled it completely for
all configurations, because of false-positive warnings on x86 that I had
not addressed until then. This caused a lot of actual bugs to get merged
into mainline, and I sent several dozen patches for these during the v4.9
development cycle. Most of these are actual bugs, some are for correct
code that is safe because it is only called under external constraints
that make it impossible to run into the case that gcc sees, and in a
few cases gcc is just stupid and finds something that can obviously
never happen.
I have now done a few thousand randconfig builds on x86 and collected all
patches that I needed to address every single warning I got (I can provide
the combined patch for the other warnings if anyone is interested),
so I hope we can get the warning back and let people catch the actual
bugs earlier.
This reverts the change to disable the warning completely and for
now brings it back at the "make W=1" level, so we can get it merged
into mainline without introducing false positives. A follow-up
patch enables it on all levels unless some configuration option
turns it off because of false-positives.
Link: https://rusty.ozlabs.org/?p=232 [1]
Link: https://gcc.gnu.org/wiki/Better_Uninitialized_Warnings [2]
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
Makefile | 10 ++++++----
arch/arc/Makefile | 4 +++-
| 3 +++
scripts/Makefile.ubsan | 4 ++++
4 files changed, 16 insertions(+), 5 deletions(-)
diff --git a/Makefile b/Makefile
index f97f786..06e2b73 100644
--- a/Makefile
+++ b/Makefile
@@ -370,7 +370,7 @@ LDFLAGS_MODULE =
CFLAGS_KERNEL =
AFLAGS_KERNEL =
LDFLAGS_vmlinux =
-CFLAGS_GCOV = -fprofile-arcs -ftest-coverage -fno-tree-loop-im
+CFLAGS_GCOV = -fprofile-arcs -ftest-coverage -fno-tree-loop-im -Wno-maybe-uninitialized
CFLAGS_KCOV := $(call cc-option,-fsanitize-coverage=trace-pc,)
@@ -620,7 +620,6 @@ ARCH_CFLAGS :=
include arch/$(SRCARCH)/Makefile
KBUILD_CFLAGS += $(call cc-option,-fno-delete-null-pointer-checks,)
-KBUILD_CFLAGS += $(call cc-disable-warning,maybe-uninitialized,)
KBUILD_CFLAGS += $(call cc-disable-warning,frame-address,)
ifdef CONFIG_LD_DEAD_CODE_DATA_ELIMINATION
@@ -629,15 +628,18 @@ KBUILD_CFLAGS += $(call cc-option,-fdata-sections,)
endif
ifdef CONFIG_CC_OPTIMIZE_FOR_SIZE
-KBUILD_CFLAGS += -Os
+KBUILD_CFLAGS += -Os $(call cc-disable-warning,maybe-uninitialized,)
else
ifdef CONFIG_PROFILE_ALL_BRANCHES
-KBUILD_CFLAGS += -O2
+KBUILD_CFLAGS += -O2 $(call cc-disable-warning,maybe-uninitialized,)
else
KBUILD_CFLAGS += -O2
endif
endif
+KBUILD_CFLAGS += $(call cc-ifversion, -lt, 0409, \
+ $(call cc-disable-warning,maybe-uninitialized,))
+
# Tell gcc to never replace conditional load with a non-conditional one
KBUILD_CFLAGS += $(call cc-option,--param=allow-store-data-races=0)
diff --git a/arch/arc/Makefile b/arch/arc/Makefile
index 864adad..25f81a1 100644
--- a/arch/arc/Makefile
+++ b/arch/arc/Makefile
@@ -68,7 +68,9 @@ cflags-$(CONFIG_ARC_DW2_UNWIND) += -fasynchronous-unwind-tables $(cfi)
ifndef CONFIG_CC_OPTIMIZE_FOR_SIZE
# Generic build system uses -O2, we want -O3
# Note: No need to add to cflags-y as that happens anyways
-ARCH_CFLAGS += -O3
+#
+# Disable the false maybe-uninitialized warings gcc spits out at -O3
+ARCH_CFLAGS += -O3 $(call cc-disable-warning,maybe-uninitialized,)
endif
# small data is default for elf32 tool-chain. If not usable, disable it
--git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 53449a6..7fc2c5a 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -36,6 +36,7 @@ warning-2 += -Wshadow
warning-2 += $(call cc-option, -Wlogical-op)
warning-2 += $(call cc-option, -Wmissing-field-initializers)
warning-2 += $(call cc-option, -Wsign-compare)
+warning-2 += $(call cc-option, -Wmaybe-uninitialized)
warning-3 := -Wbad-function-cast
warning-3 += -Wcast-qual
@@ -59,6 +60,8 @@ endif
KBUILD_CFLAGS += $(warning)
else
+KBUILD_CFLAGS += $(call cc-disable-warning, maybe-uninitialized)
+
ifeq ($(cc-name),clang)
KBUILD_CFLAGS += $(call cc-disable-warning, initializer-overrides)
KBUILD_CFLAGS += $(call cc-disable-warning, unused-value)
diff --git a/scripts/Makefile.ubsan b/scripts/Makefile.ubsan
index dd779c4..3b1b138 100644
--- a/scripts/Makefile.ubsan
+++ b/scripts/Makefile.ubsan
@@ -17,4 +17,8 @@ endif
ifdef CONFIG_UBSAN_NULL
CFLAGS_UBSAN += $(call cc-option, -fsanitize=null)
endif
+
+ # -fsanitize=* options makes GCC less smart than usual and
+ # increase number of 'maybe-uninitialized false-positives
+ CFLAGS_UBSAN += $(call cc-option, -Wno-maybe-uninitialized)
endif
--
2.9.0
^ permalink raw reply related
* [PATCH v2 00/11] getting back -Wmaybe-uninitialized
From: Arnd Bergmann @ 2016-11-10 16:44 UTC (permalink / raw)
To: Linus Torvalds
Cc: Sean Young, Trond Myklebust, linux-s390, Herbert Xu, x86,
Sebastian Ott, Russell King, Javier Martinez Canillas,
Ilya Dryomov, linux-snps-arc, linux-media, Arnd Bergmann,
linux-kbuild, Jiri Kosina, Michal Marek, nios2-dev,
Mauro Carvalho Chehab, linux-nfs, linux-kernel, Anna Schumaker,
Luis R . Rodriguez, linux-crypto, Martin Schwidefsky,
Ley Foon Tan, Andrew Morton
Hi Linus,
It took a while for some patches to make it into mainline through
maintainer trees, but the 28-patch series is now reduced to 10, with
one tiny patch added at the end. I hope this can still make it into
v4.9. Aside from patches that are no longer required, I did these changes
compared to version 1:
- Dropped "iio: maxim_thermocouple: detect invalid storage size in
read()", which is currently in linux-next as commit 32cb7d27e65d.
This is the only remaining warning I see for a couple of corner
cases (kbuild bot reports it on blackfin, kernelci bot and
arm-soc bot both report it on arm64)
- Dropped "brcmfmac: avoid maybe-uninitialized warning in
brcmf_cfg80211_start_ap", which is currently in net/master
merge pending.
- Dropped two x86 patches, "x86: math-emu: possible uninitialized
variable use" and "x86: mark target address as output in 'insb' asm"
as they do not seem to trigger for a default build, and I got
no feedback on them. Both of these are ancient issues and seem
harmless, I will send them again to the x86 maintainers once
the rest is merged.
- Dropped "rbd: false-postive gcc-4.9 -Wmaybe-uninitialized" based on
feedback from Ilya Dryomov, who already has a different fix queued up
for v4.10. The kbuild bot reports this as a warning for xtensa.
- Replaced "crypto: aesni: avoid -Wmaybe-uninitialized warning" with a
simpler patch, this one always triggers but my first solution would not
be safe for linux-4.9 any more at this point. I'll follow up with
the larger patch as a cleanup for 4.10.
- Replaced "dib0700: fix nec repeat handling" with a better one,
contributed by Sean Young.
Please merge these directly if you are happy with the result.
As the minimum, I'd hope to see the first patch get in soon,
but the individual bugfixes are hopefully now all appropriate
as well. If you see any regressions with the final patch, just
leave that one out and let me know what problems remain.
Arnd
Arnd Bergmann (10):
Kbuild: enable -Wmaybe-uninitialized warning for "make W=1"
NFSv4.1: work around -Wmaybe-uninitialized warning
x86: apm: avoid uninitialized data
nios2: fix timer initcall return value
s390: pci: don't print uninitialized data for debugging
[media] rc: print correct variable for z8f0811
crypto: aesni: shut up -Wmaybe-uninitialized warning
infiniband: shut up a maybe-uninitialized warning
pcmcia: fix return value of soc_pcmcia_regulator_set
Kbuild: enable -Wmaybe-uninitialized warnings by default
Sean Young (1):
[media] dib0700: fix nec repeat handling
Makefile | 10 +++---
arch/arc/Makefile | 4 ++-
arch/nios2/kernel/time.c | 1 +
arch/s390/pci/pci_dma.c | 2 +-
arch/x86/crypto/aesni-intel_glue.c | 4 +--
arch/x86/kernel/apm_32.c | 5 ++-
drivers/infiniband/core/cma.c | 54 +++++++++++++++++---------------
drivers/media/i2c/ir-kbd-i2c.c | 2 +-
drivers/media/usb/dvb-usb/dib0700_core.c | 5 +--
drivers/pcmcia/soc_common.c | 2 +-
fs/nfs/nfs4session.c | 10 +++---
scripts/Makefile.extrawarn | 1 +
scripts/Makefile.ubsan | 4 +++
13 files changed, 61 insertions(+), 43 deletions(-)
--
2.9.0
Cc: Anna Schumaker <anna.schumaker@netapp.com>
Cc: "David S. Miller" <davem@davemloft.net>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Ilya Dryomov <idryomov@gmail.com>
Cc: Javier Martinez Canillas <javier@osg.samsung.com>
Cc: Jiri Kosina <jikos@kernel.org>
Cc: Jonathan Cameron <jic23@kernel.org>
Cc: Ley Foon Tan <lftan@altera.com>
Cc: Luis R. Rodriguez <mcgrof@kernel.org>
Cc: Martin Schwidefsky <schwidefsky@de.ibm.com>
Cc: Mauro Carvalho Chehab <mchehab@kernel.org>
Cc: Michal Marek <mmarek@suse.com>
Cc: Russell King <linux@armlinux.org.uk>
Cc: Sean Young <sean@mess.org>
Cc: Sebastian Ott <sebott@linux.vnet.ibm.com>
Cc: Trond Myklebust <trond.myklebust@primarydata.com>
Cc: x86@kernel.org
Cc: linux-kbuild@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-snps-arc@lists.infradead.org
Cc: nios2-dev@lists.rocketboards.org
Cc: linux-s390@vger.kernel.org
Cc: linux-crypto@vger.kernel.org
Cc: linux-media@vger.kernel.org
Cc: linux-nfs@vger.kernel.org
^ permalink raw reply
* Re: [PATCH] nvmem: sunxi-sid: SID content is not a valid source of randomness
From: Corentin Labbe @ 2016-11-10 15:14 UTC (permalink / raw)
To: Maxime Ripard, herbert
Cc: srinivas.kandagatla, wens, linux-kernel, linux-arm-kernel,
linux-crypto
In-Reply-To: <20161025132648.txeo3rw6yz5wutrg@lukather>
On Tue, Oct 25, 2016 at 03:26:48PM +0200, Maxime Ripard wrote:
> On Tue, Oct 25, 2016 at 07:38:55AM +0200, LABBE Corentin wrote:
> > On Mon, Oct 24, 2016 at 10:10:20PM +0200, Maxime Ripard wrote:
> > > On Sat, Oct 22, 2016 at 03:53:28PM +0200, Corentin Labbe wrote:
> > > > Since SID's content is constant over reboot,
> > >
> > > That's not true, at least not across all the Allwinner SoCs, and
> > > especially not on the A10 and A20 that this driver supports.
> > >
> >
> > On my cubieboard2 (A20)
> > hexdump -C /sys/devices/platform/soc\@01c00000/1c23800.eeprom/sunxi-sid0/nvmem
> > 00000000 16 51 66 83 80 48 50 72 56 54 48 48 03 c2 75 72 |.Qf..HPrVTHH..ur|
> > 00000010 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
> > *
> > 00000100 16 51 66 83 80 48 50 72 56 54 48 48 03 c2 75 72 |.Qf..HPrVTHH..ur|
> > 00000110 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
> > *
> > 00000200
> > cubiedev ~ # reboot
> > cubiedev ~ # hexdump -C /sys/devices/platform/soc\@01c00000/1c23800.eeprom/sunxi-sid0/nvmem
> > 00000000 16 51 66 83 80 48 50 72 56 54 48 48 03 c2 75 72 |.Qf..HPrVTHH..ur|
> > 00000010 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
> > *
> > 00000100 16 51 66 83 80 48 50 72 56 54 48 48 03 c2 75 72 |.Qf..HPrVTHH..ur|
> > 00000110 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 |................|
> > *
> > 00000200
> >
> > So clearly for me its constant.
>
> It's constant across reboots, but not across devices. Each device have
> a different SID content, therefore it's a relevant source of entropy
> in the system.
>
Not the 3 leading digit and not the tailing zeros which are the same accross device.
So only 50% of data are really different accross devices.
Perhaps a "random-range" property could be used ?
Herbert, does it is safe to add that 50% duplicate content via add_device_randomness() ?
Reading add_device_randomness doc, it seems finally it is safe, but if you could confirm it.
Regards
^ permalink raw reply
* Re: [PATCH 10/16] crypto: testmgr - Do not test internal algorithms
From: Marcelo Cerri @ 2016-11-10 11:32 UTC (permalink / raw)
To: Herbert Xu; +Cc: Linux Crypto Mailing List
In-Reply-To: <E1c1iKm-0004DQ-A4@gondolin.me.apana.org.au>
[-- Attachment #1: Type: text/plain, Size: 9874 bytes --]
I tested this patch and it's working fine.
--
Regards,
Marcelo
On Wed, Nov 02, 2016 at 07:19:12AM +0800, Herbert Xu wrote:
> Currently we manually filter out internal algorithms using a list
> in testmgr. This is dangerous as internal algorithms cannot be
> safely used even by testmgr. This patch ensures that they're never
> processed by testmgr at all.
>
> This patch also removes an obsolete bypass for nivciphers which
> no longer exist.
>
> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
> ---
>
> crypto/algboss.c | 8 --
> crypto/testmgr.c | 153 +++----------------------------------------------------
> 2 files changed, 11 insertions(+), 150 deletions(-)
>
> diff --git a/crypto/algboss.c b/crypto/algboss.c
> index 6e39d9c..ccb85e1 100644
> --- a/crypto/algboss.c
> +++ b/crypto/algboss.c
> @@ -247,12 +247,8 @@ static int cryptomgr_schedule_test(struct crypto_alg *alg)
> memcpy(param->alg, alg->cra_name, sizeof(param->alg));
> type = alg->cra_flags;
>
> - /* This piece of crap needs to disappear into per-type test hooks. */
> - if (!((type ^ CRYPTO_ALG_TYPE_BLKCIPHER) &
> - CRYPTO_ALG_TYPE_BLKCIPHER_MASK) && !(type & CRYPTO_ALG_GENIV) &&
> - ((alg->cra_flags & CRYPTO_ALG_TYPE_MASK) ==
> - CRYPTO_ALG_TYPE_BLKCIPHER ? alg->cra_blkcipher.ivsize :
> - alg->cra_ablkcipher.ivsize))
> + /* Do not test internal algorithms. */
> + if (type & CRYPTO_ALG_INTERNAL)
> type |= CRYPTO_ALG_TESTED;
>
> param->type = type;
> diff --git a/crypto/testmgr.c b/crypto/testmgr.c
> index ded50b6..6ac4696 100644
> --- a/crypto/testmgr.c
> +++ b/crypto/testmgr.c
> @@ -1625,7 +1625,7 @@ static int alg_test_aead(const struct alg_test_desc *desc, const char *driver,
> struct crypto_aead *tfm;
> int err = 0;
>
> - tfm = crypto_alloc_aead(driver, type | CRYPTO_ALG_INTERNAL, mask);
> + tfm = crypto_alloc_aead(driver, type, mask);
> if (IS_ERR(tfm)) {
> printk(KERN_ERR "alg: aead: Failed to load transform for %s: "
> "%ld\n", driver, PTR_ERR(tfm));
> @@ -1654,7 +1654,7 @@ static int alg_test_cipher(const struct alg_test_desc *desc,
> struct crypto_cipher *tfm;
> int err = 0;
>
> - tfm = crypto_alloc_cipher(driver, type | CRYPTO_ALG_INTERNAL, mask);
> + tfm = crypto_alloc_cipher(driver, type, mask);
> if (IS_ERR(tfm)) {
> printk(KERN_ERR "alg: cipher: Failed to load transform for "
> "%s: %ld\n", driver, PTR_ERR(tfm));
> @@ -1683,7 +1683,7 @@ static int alg_test_skcipher(const struct alg_test_desc *desc,
> struct crypto_skcipher *tfm;
> int err = 0;
>
> - tfm = crypto_alloc_skcipher(driver, type | CRYPTO_ALG_INTERNAL, mask);
> + tfm = crypto_alloc_skcipher(driver, type, mask);
> if (IS_ERR(tfm)) {
> printk(KERN_ERR "alg: skcipher: Failed to load transform for "
> "%s: %ld\n", driver, PTR_ERR(tfm));
> @@ -1750,7 +1750,7 @@ static int alg_test_hash(const struct alg_test_desc *desc, const char *driver,
> struct crypto_ahash *tfm;
> int err;
>
> - tfm = crypto_alloc_ahash(driver, type | CRYPTO_ALG_INTERNAL, mask);
> + tfm = crypto_alloc_ahash(driver, type, mask);
> if (IS_ERR(tfm)) {
> printk(KERN_ERR "alg: hash: Failed to load transform for %s: "
> "%ld\n", driver, PTR_ERR(tfm));
> @@ -1778,7 +1778,7 @@ static int alg_test_crc32c(const struct alg_test_desc *desc,
> if (err)
> goto out;
>
> - tfm = crypto_alloc_shash(driver, type | CRYPTO_ALG_INTERNAL, mask);
> + tfm = crypto_alloc_shash(driver, type, mask);
> if (IS_ERR(tfm)) {
> printk(KERN_ERR "alg: crc32c: Failed to load transform for %s: "
> "%ld\n", driver, PTR_ERR(tfm));
> @@ -1820,7 +1820,7 @@ static int alg_test_cprng(const struct alg_test_desc *desc, const char *driver,
> struct crypto_rng *rng;
> int err;
>
> - rng = crypto_alloc_rng(driver, type | CRYPTO_ALG_INTERNAL, mask);
> + rng = crypto_alloc_rng(driver, type, mask);
> if (IS_ERR(rng)) {
> printk(KERN_ERR "alg: cprng: Failed to load transform for %s: "
> "%ld\n", driver, PTR_ERR(rng));
> @@ -1847,7 +1847,7 @@ static int drbg_cavs_test(struct drbg_testvec *test, int pr,
> if (!buf)
> return -ENOMEM;
>
> - drng = crypto_alloc_rng(driver, type | CRYPTO_ALG_INTERNAL, mask);
> + drng = crypto_alloc_rng(driver, type, mask);
> if (IS_ERR(drng)) {
> printk(KERN_ERR "alg: drbg: could not allocate DRNG handle for "
> "%s\n", driver);
> @@ -2041,7 +2041,7 @@ static int alg_test_kpp(const struct alg_test_desc *desc, const char *driver,
> struct crypto_kpp *tfm;
> int err = 0;
>
> - tfm = crypto_alloc_kpp(driver, type | CRYPTO_ALG_INTERNAL, mask);
> + tfm = crypto_alloc_kpp(driver, type, mask);
> if (IS_ERR(tfm)) {
> pr_err("alg: kpp: Failed to load tfm for %s: %ld\n",
> driver, PTR_ERR(tfm));
> @@ -2200,7 +2200,7 @@ static int alg_test_akcipher(const struct alg_test_desc *desc,
> struct crypto_akcipher *tfm;
> int err = 0;
>
> - tfm = crypto_alloc_akcipher(driver, type | CRYPTO_ALG_INTERNAL, mask);
> + tfm = crypto_alloc_akcipher(driver, type, mask);
> if (IS_ERR(tfm)) {
> pr_err("alg: akcipher: Failed to load tfm for %s: %ld\n",
> driver, PTR_ERR(tfm));
> @@ -2223,88 +2223,6 @@ static int alg_test_null(const struct alg_test_desc *desc,
> /* Please keep this list sorted by algorithm name. */
> static const struct alg_test_desc alg_test_descs[] = {
> {
> - .alg = "__cbc-cast5-avx",
> - .test = alg_test_null,
> - }, {
> - .alg = "__cbc-cast6-avx",
> - .test = alg_test_null,
> - }, {
> - .alg = "__cbc-serpent-avx",
> - .test = alg_test_null,
> - }, {
> - .alg = "__cbc-serpent-avx2",
> - .test = alg_test_null,
> - }, {
> - .alg = "__cbc-serpent-sse2",
> - .test = alg_test_null,
> - }, {
> - .alg = "__cbc-twofish-avx",
> - .test = alg_test_null,
> - }, {
> - .alg = "__driver-cbc-aes-aesni",
> - .test = alg_test_null,
> - .fips_allowed = 1,
> - }, {
> - .alg = "__driver-cbc-camellia-aesni",
> - .test = alg_test_null,
> - }, {
> - .alg = "__driver-cbc-camellia-aesni-avx2",
> - .test = alg_test_null,
> - }, {
> - .alg = "__driver-cbc-cast5-avx",
> - .test = alg_test_null,
> - }, {
> - .alg = "__driver-cbc-cast6-avx",
> - .test = alg_test_null,
> - }, {
> - .alg = "__driver-cbc-serpent-avx",
> - .test = alg_test_null,
> - }, {
> - .alg = "__driver-cbc-serpent-avx2",
> - .test = alg_test_null,
> - }, {
> - .alg = "__driver-cbc-serpent-sse2",
> - .test = alg_test_null,
> - }, {
> - .alg = "__driver-cbc-twofish-avx",
> - .test = alg_test_null,
> - }, {
> - .alg = "__driver-ecb-aes-aesni",
> - .test = alg_test_null,
> - .fips_allowed = 1,
> - }, {
> - .alg = "__driver-ecb-camellia-aesni",
> - .test = alg_test_null,
> - }, {
> - .alg = "__driver-ecb-camellia-aesni-avx2",
> - .test = alg_test_null,
> - }, {
> - .alg = "__driver-ecb-cast5-avx",
> - .test = alg_test_null,
> - }, {
> - .alg = "__driver-ecb-cast6-avx",
> - .test = alg_test_null,
> - }, {
> - .alg = "__driver-ecb-serpent-avx",
> - .test = alg_test_null,
> - }, {
> - .alg = "__driver-ecb-serpent-avx2",
> - .test = alg_test_null,
> - }, {
> - .alg = "__driver-ecb-serpent-sse2",
> - .test = alg_test_null,
> - }, {
> - .alg = "__driver-ecb-twofish-avx",
> - .test = alg_test_null,
> - }, {
> - .alg = "__driver-gcm-aes-aesni",
> - .test = alg_test_null,
> - .fips_allowed = 1,
> - }, {
> - .alg = "__ghash-pclmulqdqni",
> - .test = alg_test_null,
> - .fips_allowed = 1,
> - }, {
> .alg = "ansi_cprng",
> .test = alg_test_cprng,
> .suite = {
> @@ -2791,55 +2709,6 @@ static int alg_test_null(const struct alg_test_desc *desc,
> }
> }
> }, {
> - .alg = "cryptd(__driver-cbc-aes-aesni)",
> - .test = alg_test_null,
> - .fips_allowed = 1,
> - }, {
> - .alg = "cryptd(__driver-cbc-camellia-aesni)",
> - .test = alg_test_null,
> - }, {
> - .alg = "cryptd(__driver-cbc-camellia-aesni-avx2)",
> - .test = alg_test_null,
> - }, {
> - .alg = "cryptd(__driver-cbc-serpent-avx2)",
> - .test = alg_test_null,
> - }, {
> - .alg = "cryptd(__driver-ecb-aes-aesni)",
> - .test = alg_test_null,
> - .fips_allowed = 1,
> - }, {
> - .alg = "cryptd(__driver-ecb-camellia-aesni)",
> - .test = alg_test_null,
> - }, {
> - .alg = "cryptd(__driver-ecb-camellia-aesni-avx2)",
> - .test = alg_test_null,
> - }, {
> - .alg = "cryptd(__driver-ecb-cast5-avx)",
> - .test = alg_test_null,
> - }, {
> - .alg = "cryptd(__driver-ecb-cast6-avx)",
> - .test = alg_test_null,
> - }, {
> - .alg = "cryptd(__driver-ecb-serpent-avx)",
> - .test = alg_test_null,
> - }, {
> - .alg = "cryptd(__driver-ecb-serpent-avx2)",
> - .test = alg_test_null,
> - }, {
> - .alg = "cryptd(__driver-ecb-serpent-sse2)",
> - .test = alg_test_null,
> - }, {
> - .alg = "cryptd(__driver-ecb-twofish-avx)",
> - .test = alg_test_null,
> - }, {
> - .alg = "cryptd(__driver-gcm-aes-aesni)",
> - .test = alg_test_null,
> - .fips_allowed = 1,
> - }, {
> - .alg = "cryptd(__ghash-pclmulqdqni)",
> - .test = alg_test_null,
> - .fips_allowed = 1,
> - }, {
> .alg = "ctr(aes)",
> .test = alg_test_skcipher,
> .fips_allowed = 1,
> @@ -3166,10 +3035,6 @@ static int alg_test_null(const struct alg_test_desc *desc,
> .fips_allowed = 1,
> .test = alg_test_null,
> }, {
> - .alg = "ecb(__aes-aesni)",
> - .test = alg_test_null,
> - .fips_allowed = 1,
> - }, {
> .alg = "ecb(aes)",
> .test = alg_test_skcipher,
> .fips_allowed = 1,
> --
> 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
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: [PATCH 6/6] Add support for AEAD algos.
From: Harsh Jain @ 2016-11-10 5:00 UTC (permalink / raw)
To: Stephan Mueller
Cc: dan.carpenter, herbert, linux-crypto, jlulla, atul.gupta,
yeshaswi, hariprasad
In-Reply-To: <25e12af5-190d-8204-0d5e-bf9a7fd190c9@chelsio.com>
On 08-11-2016 19:51, Harsh Jain wrote:
>
> On 08-11-2016 18:29, Stephan Mueller wrote:
>> Am Dienstag, 8. November 2016, 17:16:38 CET schrieb Harsh Jain:
>>
>> Hi Harsh,
>>
>>> On 08-11-2016 16:45, Stephan Mueller wrote:
>>>> Am Donnerstag, 27. Oktober 2016, 15:36:08 CET schrieb Harsh Jain:
>>>>
>>>> Hi Harsh,
>>>>
>>>>>>> +static void chcr_verify_tag(struct aead_request *req, u8 *input, int
>>>>>>> *err)
>>>>>>> +{
>>>>>>> + u8 temp[SHA512_DIGEST_SIZE];
>>>>>>> + struct crypto_aead *tfm = crypto_aead_reqtfm(req);
>>>>>>> + int authsize = crypto_aead_authsize(tfm);
>>>>>>> + struct cpl_fw6_pld *fw6_pld;
>>>>>>> + int cmp = 0;
>>>>>>> +
>>>>>>> + fw6_pld = (struct cpl_fw6_pld *)input;
>>>>>>> + if ((get_aead_subtype(tfm) == CRYPTO_ALG_SUB_TYPE_AEAD_RFC4106) ||
>>>>>>> + (get_aead_subtype(tfm) == CRYPTO_ALG_SUB_TYPE_AEAD_GCM)) {
>>>>>>> + cmp = memcmp(&fw6_pld->data[2], (fw6_pld + 1), authsize);
>>>>>>> + } else {
>>>>>>> +
>>>>>>> + sg_pcopy_to_buffer(req->src, sg_nents(req->src), temp,
>>>>>>> + authsize, req->assoclen +
>>>>>>> + req->cryptlen - authsize);
>>>>>> I am wondering whether the math is correct here in any case. It is
>>>>>> permissible that we have an AAD size of 0 and even a zero-sized
>>>>>> ciphertext. How is such scenario covered here?
>>>>> Here we are trying to copy user supplied tag to local buffer(temp) for
>>>>> decrypt operation only. relative index of tag in src sg list will not
>>>>> change when AAD is zero and in decrypt operation cryptlen > authsize.
>>>> I am just wondering where this is checked. Since all of these
>>>> implementations are directly accessible from unprivileged user space, we
>>>> should be careful.
>>> chcr_verify_tag() will be called when req->verify is set to "VERIFY_SW",
>>> same will set in decrypt callback function of Algo(like chcr_aead_decrypt)
>>> only. It will ensure calling of chcr_verify_tag() in de-crypt operation
>>> only.
>> I think that limiting to the decryption path may not be enough. What happens
>> if a caller sets some assoclen, but when invoking the decryption operation it
>> provides input data that is smaller than the assoclen? The API allows this
>> scenario.
> If I understand correctly, in this case passed sg list will be smaller. We should return with error -EINVAL at entry point only (like create_gcm_wr), control should not reach to chcr_verify_tag().
I had a look in software implementation for check related to aad len > src sg list. I doubt same is not handled in software also. See below
In "crypto_authenc_encrypt" if assoclen passed to "scatterwalk_ffwd" is greater than src. It may panic with NULL pointer exception.
I will add this check in V2 of chcr driver.
>
>> Ciao
>> Stephan
^ permalink raw reply
* [PATCH 0/3] crypto: AF_ALG - AEAD memory handling fixes
From: Stephan Mueller @ 2016-11-10 3:30 UTC (permalink / raw)
To: herbert; +Cc: mathew.j.martineau, linux-crypto
Hi Herbert,
The first patch is unchanged compared to a previous submission to the
mailing list. It is rolled into this patch set to have a common reference
for the AEAD user space interface changes. Therefore, please disregard
the old patch submission.
The third patch should go into stable as this fixes a kernel crash that
is present in current kernel versions.
The patch set can be tested with the HEAD branch of libkcapi found at [1].
If the patch set is accepted, the development branch will be released as
0.13.0.
[1] https://github.com/smuellerDD/libkcapi
Stephan Mueller (3):
crypto: AF_ALG - fix AEAD tag memory handling
crypto: AF_ALG - disregard AAD buffer space for output
crypto: AF_ALG - fix AEAD AIO handling of zero buffer
crypto/algif_aead.c | 215 ++++++++++++++++++++++++++++++++++++++++------------
1 file changed, 166 insertions(+), 49 deletions(-)
--
2.7.4
^ permalink raw reply
* [PATCH 1/3] crypto: AF_ALG - fix AEAD tag memory handling
From: Stephan Mueller @ 2016-11-10 3:31 UTC (permalink / raw)
To: herbert; +Cc: mathew.j.martineau, linux-crypto
In-Reply-To: <3302668.EORF2jai5r@positron.chronox.de>
For encryption, the AEAD ciphers require AAD || PT as input and generate
AAD || CT || Tag as output and vice versa for decryption. Prior to this
patch, the AF_ALG interface for AEAD ciphers requires the buffer to be
present as input for encryption. Similarly, the output buffer for
decryption required the presence of the tag buffer too. This implies
that the kernel reads / writes data buffers from/to kernel space
even though this operation is not required.
This patch changes the AF_ALG AEAD interface to be consistent with the
in-kernel AEAD cipher requirements.
In addition, the code now handles the situation where the provided
output buffer is too small by reducing the size of the processed
input buffer accordingly. Due to this handling, he changes are
transparent to user space with one exception: the return code of recv
indicates the mount of output buffer. That output buffer has a different
size compared to before the patch which implies that the return code of
recv will also be different. For example, a decryption operation uses 16
bytes AAD, 16 bytes CT and 16 bytes tag, the AF_ALG AEAD interface
before showed a recv return code of 48 (bytes) whereas after this patch,
the return code is 32 since the tag is not returned any more.
Reported-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
crypto/algif_aead.c | 77 ++++++++++++++++++++++++++++++++++++++---------------
1 file changed, 55 insertions(+), 22 deletions(-)
diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
index 80a0f1a..c54bcb8 100644
--- a/crypto/algif_aead.c
+++ b/crypto/algif_aead.c
@@ -81,7 +81,11 @@ static inline bool aead_sufficient_data(struct aead_ctx *ctx)
{
unsigned as = crypto_aead_authsize(crypto_aead_reqtfm(&ctx->aead_req));
- return ctx->used >= ctx->aead_assoclen + as;
+ /*
+ * The minimum amount of memory needed for an AEAD cipher is
+ * the AAD and in case of decryption the tag.
+ */
+ return ctx->used >= ctx->aead_assoclen + (ctx->enc ? 0 : as);
}
static void aead_reset_ctx(struct aead_ctx *ctx)
@@ -426,12 +430,15 @@ static int aead_recvmsg_async(struct socket *sock, struct msghdr *msg,
goto unlock;
}
- used = ctx->used;
- outlen = used;
-
if (!aead_sufficient_data(ctx))
goto unlock;
+ used = ctx->used;
+ if (ctx->enc)
+ outlen = used + as;
+ else
+ outlen = used - as;
+
req = sock_kmalloc(sk, reqlen, GFP_KERNEL);
if (unlikely(!req))
goto unlock;
@@ -445,7 +452,7 @@ 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 + (ctx->enc ? as : 0);
+ used -= ctx->aead_assoclen;
/* take over all tx sgls from ctx */
areq->tsgl = sock_kmalloc(sk, sizeof(*areq->tsgl) * sgl->cur,
@@ -461,7 +468,7 @@ static int aead_recvmsg_async(struct socket *sock, struct msghdr *msg,
areq->tsgls = sgl->cur;
/* create rx sgls */
- while (iov_iter_count(&msg->msg_iter)) {
+ while (outlen > usedpages && iov_iter_count(&msg->msg_iter)) {
size_t seglen = min_t(size_t, iov_iter_count(&msg->msg_iter),
(outlen - usedpages));
@@ -491,16 +498,20 @@ static int aead_recvmsg_async(struct socket *sock, struct msghdr *msg,
last_rsgl = rsgl;
- /* we do not need more iovecs as we have sufficient memory */
- if (outlen <= usedpages)
- break;
-
iov_iter_advance(&msg->msg_iter, err);
}
- err = -EINVAL;
+
/* ensure output buffer is sufficiently large */
- if (usedpages < outlen)
- goto free;
+ if (usedpages < outlen) {
+ size_t less = outlen - usedpages;
+
+ if (used < less) {
+ err = -EINVAL;
+ goto unlock;
+ }
+ used -= less;
+ outlen -= less;
+ }
aead_request_set_crypt(req, areq->tsgl, areq->first_rsgl.sgl.sg, used,
areq->iv);
@@ -571,6 +582,7 @@ 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;
/*
@@ -585,16 +597,27 @@ static int aead_recvmsg_sync(struct socket *sock, struct msghdr *msg, int flags)
if (!aead_sufficient_data(ctx))
goto unlock;
- outlen = used;
+ /*
+ * 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
+ * encryption operation generates the tag. For decryption, the input
+ * buffer provides the tag which is consumed resulting in only the
+ * plaintext without a buffer for the tag returned to the caller.
+ */
+ if (ctx->enc)
+ outlen = used + as;
+ else
+ outlen = used - as;
/*
* The cipher operation input data is reduced by the associated data
* length as this data is processed separately later on.
*/
- used -= ctx->aead_assoclen + (ctx->enc ? as : 0);
+ used -= ctx->aead_assoclen;
/* convert iovecs of output buffers into scatterlists */
- while (iov_iter_count(&msg->msg_iter)) {
+ while (outlen > usedpages && iov_iter_count(&msg->msg_iter)) {
size_t seglen = min_t(size_t, iov_iter_count(&msg->msg_iter),
(outlen - usedpages));
@@ -621,16 +644,26 @@ static int aead_recvmsg_sync(struct socket *sock, struct msghdr *msg, int flags)
last_rsgl = rsgl;
- /* we do not need more iovecs as we have sufficient memory */
- if (outlen <= usedpages)
- break;
iov_iter_advance(&msg->msg_iter, err);
}
- err = -EINVAL;
/* ensure output buffer is sufficiently large */
- if (usedpages < outlen)
- goto unlock;
+ if (usedpages < outlen) {
+ size_t less = outlen - usedpages;
+
+ if (used < less) {
+ err = -EINVAL;
+ goto unlock;
+ }
+
+ /*
+ * Caller has smaller output buffer than needed, reduce
+ * the input data length to be processed to fit the provided
+ * output buffer.
+ */
+ used -= less;
+ outlen -= less;
+ }
sg_mark_end(sgl->sg + sgl->cur - 1);
aead_request_set_crypt(&ctx->aead_req, sgl->sg, ctx->first_rsgl.sgl.sg,
--
2.7.4
^ permalink raw reply related
* [PATCH 2/3] crypto: AF_ALG - disregard AAD buffer space for output
From: Stephan Mueller @ 2016-11-10 3:32 UTC (permalink / raw)
To: herbert; +Cc: mathew.j.martineau, linux-crypto
In-Reply-To: <3302668.EORF2jai5r@positron.chronox.de>
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.
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
^ permalink raw reply related
* [PATCH 3/3] crypto: AF_ALG - fix AEAD AIO handling of zero buffer
From: Stephan Mueller @ 2016-11-10 3:32 UTC (permalink / raw)
To: herbert; +Cc: mathew.j.martineau, linux-crypto
In-Reply-To: <3302668.EORF2jai5r@positron.chronox.de>
Handle the case when the caller provided a zero buffer to
sendmsg/sendpage. Such scenario is legal for AEAD ciphers when no
plaintext / ciphertext and no AAD is provided and the caller only
requests the generation of the tag value.
Signed-off-by: Stephan Mueller <smueller@chronox.de>
---
crypto/algif_aead.c | 5 +++--
1 file changed, 3 insertions(+), 2 deletions(-)
diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
index 0212cc2..d52e8b2 100644
--- a/crypto/algif_aead.c
+++ b/crypto/algif_aead.c
@@ -510,12 +510,13 @@ static int aead_recvmsg_async(struct socket *sock, struct msghdr *msg,
aead_async_cb, sk);
/* take over all tx sgls from ctx */
- areq->tsgl = sock_kmalloc(sk, sizeof(*areq->tsgl) * sgl->cur,
+ areq->tsgl = sock_kmalloc(sk,
+ sizeof(*areq->tsgl) * max_t(u32, sgl->cur, 1),
GFP_KERNEL);
if (unlikely(!areq->tsgl))
goto free;
- sg_init_table(areq->tsgl, sgl->cur);
+ sg_init_table(areq->tsgl, max_t(u32, sgl->cur, 1));
for (i = 0; i < sgl->cur; i++)
sg_set_page(&areq->tsgl[i], sg_page(&sgl->sg[i]),
sgl->sg[i].length, sgl->sg[i].offset);
--
2.7.4
^ permalink raw reply related
* Re: [PATCH] crypto: fix AEAD tag memory handling
From: Mat Martineau @ 2016-11-10 0:26 UTC (permalink / raw)
To: Stephan Mueller; +Cc: herbert, linux-crypto
In-Reply-To: <13567923.CP2Rp1drvf@positron.chronox.de>
Stephan,
On Wed, 9 Nov 2016, Stephan Mueller wrote:
> Am Montag, 31. Oktober 2016, 16:18:32 CET schrieb Mat Martineau:
>
> Hi Mat,
>>
>> My main concern is getting the semantics correct and consistent in a
>> single patch series. It would be a big problem to explain that AF_ALG AEAD
>> read and write works one way in 4.x, another way in 4.y, and some
>> different way in 4.z.
>
> I do have a patch now available that exactly does what you suggest. See the
> patch attached. It works with the following exception.
>
> In the case of sendpage and using an in-place cipher operation, the patch
> breaks as follows. When the caller sends the same buffer for a 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.
>
> Thus, the only way to avoid that would be to duplicate the AAD into an
> internal buffer. But that would defeat the entire purpose of sendpage.
The use case for an "in place" operation would be to have the ciphertext
overwrite the plaintext, correct? If the src and dst overlap, does it make
sense to require the plaintext and ciphertext to be in exactly the same
location?
> The patch, however, works with sendmsg as well as sendpage when the src and
> dst buffers are different.
Thanks - I tested your patch and found that it works as expected with
sendmsg.
Regards,
--
Mat Martineau
Intel OTC
^ permalink raw reply
* Re: [PATCH 11/14] Revert "crypto: caam - get rid of tasklet"
From: Thomas Gleixner @ 2016-11-09 23:19 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Horia Geantă, Herbert Xu, David S. Miller, linux-crypto
In-Reply-To: <alpine.DEB.2.20.1611092358390.3501@nanos>
On Thu, 10 Nov 2016, Thomas Gleixner wrote:
> > > which corresponds to an 8% slowdown for the threaded IRQ case. So,
> > > tasklets are indeed faster than threaded IRQs.
Forgot to say, that this should be:
So, tasklets are indeed faster than threaded IRQs for this particular use
case. They are not generally faster.
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH 11/14] Revert "crypto: caam - get rid of tasklet"
From: Thomas Gleixner @ 2016-11-09 23:17 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Horia Geantă, Herbert Xu, David S. Miller, linux-crypto
In-Reply-To: <20161109085322.GY1041@n2100.armlinux.org.uk>
[-- Attachment #1: Type: text/plain, Size: 1957 bytes --]
On Wed, 9 Nov 2016, Russell King - ARM Linux wrote:
> Please include Thomas in this.
Thanks!
> On Wed, Nov 09, 2016 at 10:46:21AM +0200, Horia Geantă wrote:
> > This reverts commit 66d2e2028091a074aa1290d2eeda5ddb1a6c329c.
> >
> > Quoting from Russell's findings:
> > https://www.mail-archive.com/linux-crypto@vger.kernel.org/msg21136.html
> >
> > [quote]
> > Okay, I've re-tested, using a different way of measuring, because using
> > openssl speed is impractical for off-loaded engines. I've decided to
> > use this way to measure the performance:
> >
> > dd if=/dev/zero bs=1048576 count=128 | /usr/bin/time openssl dgst -md5
> >
> > For the threaded IRQs case gives:
> >
> > 0.05user 2.74system 0:05.30elapsed 52%CPU (0avgtext+0avgdata 2400maxresident)k
> > 0.06user 2.52system 0:05.18elapsed 49%CPU (0avgtext+0avgdata 2404maxresident)k
> > 0.12user 2.60system 0:05.61elapsed 48%CPU (0avgtext+0avgdata 2460maxresident)k
> > => 5.36s => 25.0MB/s
> >
> > and the tasklet case:
> >
> > 0.08user 2.53system 0:04.83elapsed 54%CPU (0avgtext+0avgdata 2468maxresident)k
> > 0.09user 2.47system 0:05.16elapsed 49%CPU (0avgtext+0avgdata 2368maxresident)k
> > 0.10user 2.51system 0:04.87elapsed 53%CPU (0avgtext+0avgdata 2460maxresident)k
> > => 4.95 => 27.1MB/s
> >
> > which corresponds to an 8% slowdown for the threaded IRQ case. So,
> > tasklets are indeed faster than threaded IRQs.
Each operation involves:
submit job
hard irq handler
wakeup irq thread
context switch
wakeup openssl
context switch
and that repeats over and over.
Russell has provided me traces and the extra wakeup/context switch is what
causes the slowdown vs. the tasklet case which has only one wakeup/context
switch, unless it gets outsourced to ksoftirqd which might get even slower
than the threaded handler.
You might add something like that to the changelog so it's not just a
number comparison which does not explain why this happens.
Thanks,
tglx
^ permalink raw reply
* Re: [PATCH] crypto: fix AEAD tag memory handling
From: Stephan Mueller @ 2016-11-09 13:20 UTC (permalink / raw)
To: Mat Martineau; +Cc: herbert, linux-crypto
In-Reply-To: <alpine.OSX.2.20.1610311456250.4111@mjmartin-mac01.local>
[-- Attachment #1: Type: text/plain, Size: 1231 bytes --]
Am Montag, 31. Oktober 2016, 16:18:32 CET schrieb Mat Martineau:
Hi Mat,
>
> My main concern is getting the semantics correct and consistent in a
> single patch series. It would be a big problem to explain that AF_ALG AEAD
> read and write works one way in 4.x, another way in 4.y, and some
> different way in 4.z.
I do have a patch now available that exactly does what you suggest. See the
patch attached. It works with the following exception.
In the case of sendpage and using an in-place cipher operation, the patch
breaks as follows. When the caller sends the same buffer for a 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.
Thus, the only way to avoid that would be to duplicate the AAD into an
internal buffer. But that would defeat the entire purpose of sendpage.
The patch, however, works with sendmsg as well as sendpage when the src and
dst buffers are different.
Ciao
Stephan
[-- Attachment #2: fix_read_offset.diff --]
[-- Type: text/x-patch, Size: 7509 bytes --]
diff --git a/crypto/algif_aead.c b/crypto/algif_aead.c
index c54bcb8..c8efd01 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)
@@ -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);
^ permalink raw reply related
* [PATCH 13/14] crypto: caam - constify pointer to descriptor buffer
From: Horia Geantă @ 2016-11-09 8:46 UTC (permalink / raw)
To: Herbert Xu; +Cc: David S. Miller, linux-crypto
In-Reply-To: <1478681184-9442-1-git-send-email-horia.geanta@nxp.com>
The pointer to the descriptor buffer is not touched,
it always points to start of the descriptor buffer.
Thus, make it const.
Signed-off-by: Horia Geantă <horia.geanta@nxp.com>
---
drivers/crypto/caam/desc_constr.h | 72 +++++++++++++++++++++------------------
1 file changed, 38 insertions(+), 34 deletions(-)
diff --git a/drivers/crypto/caam/desc_constr.h b/drivers/crypto/caam/desc_constr.h
index a8cd8a78ec1f..354da735af62 100644
--- a/drivers/crypto/caam/desc_constr.h
+++ b/drivers/crypto/caam/desc_constr.h
@@ -33,38 +33,39 @@
extern bool caam_little_end;
-static inline int desc_len(u32 *desc)
+static inline int desc_len(u32 * const desc)
{
return caam32_to_cpu(*desc) & HDR_DESCLEN_MASK;
}
-static inline int desc_bytes(void *desc)
+static inline int desc_bytes(void * const desc)
{
return desc_len(desc) * CAAM_CMD_SZ;
}
-static inline u32 *desc_end(u32 *desc)
+static inline u32 *desc_end(u32 * const desc)
{
return desc + desc_len(desc);
}
-static inline void *sh_desc_pdb(u32 *desc)
+static inline void *sh_desc_pdb(u32 * const desc)
{
return desc + 1;
}
-static inline void init_desc(u32 *desc, u32 options)
+static inline void init_desc(u32 * const desc, u32 options)
{
*desc = cpu_to_caam32((options | HDR_ONE) + 1);
}
-static inline void init_sh_desc(u32 *desc, u32 options)
+static inline void init_sh_desc(u32 * const desc, u32 options)
{
PRINT_POS;
init_desc(desc, CMD_SHARED_DESC_HDR | options);
}
-static inline void init_sh_desc_pdb(u32 *desc, u32 options, size_t pdb_bytes)
+static inline void init_sh_desc_pdb(u32 * const desc, u32 options,
+ size_t pdb_bytes)
{
u32 pdb_len = (pdb_bytes + CAAM_CMD_SZ - 1) / CAAM_CMD_SZ;
@@ -72,19 +73,20 @@ static inline void init_sh_desc_pdb(u32 *desc, u32 options, size_t pdb_bytes)
options);
}
-static inline void init_job_desc(u32 *desc, u32 options)
+static inline void init_job_desc(u32 * const desc, u32 options)
{
init_desc(desc, CMD_DESC_HDR | options);
}
-static inline void init_job_desc_pdb(u32 *desc, u32 options, size_t pdb_bytes)
+static inline void init_job_desc_pdb(u32 * const desc, u32 options,
+ size_t pdb_bytes)
{
u32 pdb_len = (pdb_bytes + CAAM_CMD_SZ - 1) / CAAM_CMD_SZ;
init_job_desc(desc, (((pdb_len + 1) << HDR_START_IDX_SHIFT)) | options);
}
-static inline void append_ptr(u32 *desc, dma_addr_t ptr)
+static inline void append_ptr(u32 * const desc, dma_addr_t ptr)
{
dma_addr_t *offset = (dma_addr_t *)desc_end(desc);
@@ -94,8 +96,8 @@ static inline void append_ptr(u32 *desc, dma_addr_t ptr)
CAAM_PTR_SZ / CAAM_CMD_SZ);
}
-static inline void init_job_desc_shared(u32 *desc, dma_addr_t ptr, int len,
- u32 options)
+static inline void init_job_desc_shared(u32 * const desc, dma_addr_t ptr,
+ int len, u32 options)
{
PRINT_POS;
init_job_desc(desc, HDR_SHARED | options |
@@ -103,7 +105,7 @@ static inline void init_job_desc_shared(u32 *desc, dma_addr_t ptr, int len,
append_ptr(desc, ptr);
}
-static inline void append_data(u32 *desc, void *data, int len)
+static inline void append_data(u32 * const desc, void *data, int len)
{
u32 *offset = desc_end(desc);
@@ -114,7 +116,7 @@ static inline void append_data(u32 *desc, void *data, int len)
(len + CAAM_CMD_SZ - 1) / CAAM_CMD_SZ);
}
-static inline void append_cmd(u32 *desc, u32 command)
+static inline void append_cmd(u32 * const desc, u32 command)
{
u32 *cmd = desc_end(desc);
@@ -125,7 +127,7 @@ static inline void append_cmd(u32 *desc, u32 command)
#define append_u32 append_cmd
-static inline void append_u64(u32 *desc, u64 data)
+static inline void append_u64(u32 * const desc, u64 data)
{
u32 *offset = desc_end(desc);
@@ -142,14 +144,14 @@ static inline void append_u64(u32 *desc, u64 data)
}
/* Write command without affecting header, and return pointer to next word */
-static inline u32 *write_cmd(u32 *desc, u32 command)
+static inline u32 *write_cmd(u32 * const desc, u32 command)
{
*desc = cpu_to_caam32(command);
return desc + 1;
}
-static inline void append_cmd_ptr(u32 *desc, dma_addr_t ptr, int len,
+static inline void append_cmd_ptr(u32 * const desc, dma_addr_t ptr, int len,
u32 command)
{
append_cmd(desc, command | len);
@@ -157,7 +159,7 @@ static inline void append_cmd_ptr(u32 *desc, dma_addr_t ptr, int len,
}
/* Write length after pointer, rather than inside command */
-static inline void append_cmd_ptr_extlen(u32 *desc, dma_addr_t ptr,
+static inline void append_cmd_ptr_extlen(u32 * const desc, dma_addr_t ptr,
unsigned int len, u32 command)
{
append_cmd(desc, command);
@@ -166,7 +168,7 @@ static inline void append_cmd_ptr_extlen(u32 *desc, dma_addr_t ptr,
append_cmd(desc, len);
}
-static inline void append_cmd_data(u32 *desc, void *data, int len,
+static inline void append_cmd_data(u32 * const desc, void *data, int len,
u32 command)
{
append_cmd(desc, command | IMMEDIATE | len);
@@ -174,7 +176,7 @@ static inline void append_cmd_data(u32 *desc, void *data, int len,
}
#define APPEND_CMD_RET(cmd, op) \
-static inline u32 *append_##cmd(u32 *desc, u32 options) \
+static inline u32 *append_##cmd(u32 * const desc, u32 options) \
{ \
u32 *cmd = desc_end(desc); \
PRINT_POS; \
@@ -184,13 +186,13 @@ static inline u32 *append_##cmd(u32 *desc, u32 options) \
APPEND_CMD_RET(jump, JUMP)
APPEND_CMD_RET(move, MOVE)
-static inline void set_jump_tgt_here(u32 *desc, u32 *jump_cmd)
+static inline void set_jump_tgt_here(u32 * const desc, u32 *jump_cmd)
{
*jump_cmd = cpu_to_caam32(caam32_to_cpu(*jump_cmd) |
(desc_len(desc) - (jump_cmd - desc)));
}
-static inline void set_move_tgt_here(u32 *desc, u32 *move_cmd)
+static inline void set_move_tgt_here(u32 * const desc, u32 *move_cmd)
{
u32 val = caam32_to_cpu(*move_cmd);
@@ -200,7 +202,7 @@ static inline void set_move_tgt_here(u32 *desc, u32 *move_cmd)
}
#define APPEND_CMD(cmd, op) \
-static inline void append_##cmd(u32 *desc, u32 options) \
+static inline void append_##cmd(u32 * const desc, u32 options) \
{ \
PRINT_POS; \
append_cmd(desc, CMD_##op | options); \
@@ -208,7 +210,8 @@ static inline void append_##cmd(u32 *desc, u32 options) \
APPEND_CMD(operation, OPERATION)
#define APPEND_CMD_LEN(cmd, op) \
-static inline void append_##cmd(u32 *desc, unsigned int len, u32 options) \
+static inline void append_##cmd(u32 * const desc, unsigned int len, \
+ u32 options) \
{ \
PRINT_POS; \
append_cmd(desc, CMD_##op | len | options); \
@@ -220,8 +223,8 @@ APPEND_CMD_LEN(seq_fifo_load, SEQ_FIFO_LOAD)
APPEND_CMD_LEN(seq_fifo_store, SEQ_FIFO_STORE)
#define APPEND_CMD_PTR(cmd, op) \
-static inline void append_##cmd(u32 *desc, dma_addr_t ptr, unsigned int len, \
- u32 options) \
+static inline void append_##cmd(u32 * const desc, dma_addr_t ptr, \
+ unsigned int len, u32 options) \
{ \
PRINT_POS; \
append_cmd_ptr(desc, ptr, len, CMD_##op | options); \
@@ -231,8 +234,8 @@ APPEND_CMD_PTR(load, LOAD)
APPEND_CMD_PTR(fifo_load, FIFO_LOAD)
APPEND_CMD_PTR(fifo_store, FIFO_STORE)
-static inline void append_store(u32 *desc, dma_addr_t ptr, unsigned int len,
- u32 options)
+static inline void append_store(u32 * const desc, dma_addr_t ptr,
+ unsigned int len, u32 options)
{
u32 cmd_src;
@@ -249,7 +252,8 @@ static inline void append_store(u32 *desc, dma_addr_t ptr, unsigned int len,
}
#define APPEND_SEQ_PTR_INTLEN(cmd, op) \
-static inline void append_seq_##cmd##_ptr_intlen(u32 *desc, dma_addr_t ptr, \
+static inline void append_seq_##cmd##_ptr_intlen(u32 * const desc, \
+ dma_addr_t ptr, \
unsigned int len, \
u32 options) \
{ \
@@ -263,7 +267,7 @@ APPEND_SEQ_PTR_INTLEN(in, IN)
APPEND_SEQ_PTR_INTLEN(out, OUT)
#define APPEND_CMD_PTR_TO_IMM(cmd, op) \
-static inline void append_##cmd##_as_imm(u32 *desc, void *data, \
+static inline void append_##cmd##_as_imm(u32 * const desc, void *data, \
unsigned int len, u32 options) \
{ \
PRINT_POS; \
@@ -273,7 +277,7 @@ APPEND_CMD_PTR_TO_IMM(load, LOAD);
APPEND_CMD_PTR_TO_IMM(fifo_load, FIFO_LOAD);
#define APPEND_CMD_PTR_EXTLEN(cmd, op) \
-static inline void append_##cmd##_extlen(u32 *desc, dma_addr_t ptr, \
+static inline void append_##cmd##_extlen(u32 * const desc, dma_addr_t ptr, \
unsigned int len, u32 options) \
{ \
PRINT_POS; \
@@ -287,7 +291,7 @@ APPEND_CMD_PTR_EXTLEN(seq_out_ptr, SEQ_OUT_PTR)
* the size of its type
*/
#define APPEND_CMD_PTR_LEN(cmd, op, type) \
-static inline void append_##cmd(u32 *desc, dma_addr_t ptr, \
+static inline void append_##cmd(u32 * const desc, dma_addr_t ptr, \
type len, u32 options) \
{ \
PRINT_POS; \
@@ -304,7 +308,7 @@ APPEND_CMD_PTR_LEN(seq_out_ptr, SEQ_OUT_PTR, u32)
* from length of immediate data provided, e.g., split keys
*/
#define APPEND_CMD_PTR_TO_IMM2(cmd, op) \
-static inline void append_##cmd##_as_imm(u32 *desc, void *data, \
+static inline void append_##cmd##_as_imm(u32 * const desc, void *data, \
unsigned int data_len, \
unsigned int len, u32 options) \
{ \
@@ -315,7 +319,7 @@ static inline void append_##cmd##_as_imm(u32 *desc, void *data, \
APPEND_CMD_PTR_TO_IMM2(key, KEY);
#define APPEND_CMD_RAW_IMM(cmd, op, type) \
-static inline void append_##cmd##_imm_##type(u32 *desc, type immediate, \
+static inline void append_##cmd##_imm_##type(u32 * const desc, type immediate, \
u32 options) \
{ \
PRINT_POS; \
--
2.4.4
^ permalink raw reply related
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