* [PATCH] wusb: Stop using the stack for sg crypto scratch space
@ 2016-10-06 17:25 Andy Lutomirski
2016-10-16 17:17 ` Andy Lutomirski
0 siblings, 1 reply; 3+ messages in thread
From: Andy Lutomirski @ 2016-10-06 17:25 UTC (permalink / raw)
To: Greg KH; +Cc: Herbert Xu, linux-kernel@vger.kernel.org, USB list,
Andy Lutomirski
Pointing an sg list at the stack is verboten and, with
CONFIG_VMAP_STACK=y, will malfunction. Use kmalloc for the wusb
crypto stack space instead.
Untested -- I'm not entirely convinced that this hardware exists in
the wild.
Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
This is needed for 4.9 for wusb to have a chance of working on default x86
configurations.
drivers/usb/wusbcore/crypto.c | 59 +++++++++++++++++++++++++++----------------
1 file changed, 37 insertions(+), 22 deletions(-)
diff --git a/drivers/usb/wusbcore/crypto.c b/drivers/usb/wusbcore/crypto.c
index 33acd1599e99..2db79d64e66c 100644
--- a/drivers/usb/wusbcore/crypto.c
+++ b/drivers/usb/wusbcore/crypto.c
@@ -133,6 +133,13 @@ static void bytewise_xor(void *_bo, const void *_bi1, const void *_bi2,
bo[itr] = bi1[itr] ^ bi2[itr];
}
+/* Scratch space for MAC calculations. */
+struct wusb_mac_scratch {
+ struct aes_ccm_b0 b0;
+ struct aes_ccm_b1 b1;
+ struct aes_ccm_a ax;
+};
+
/*
* CC-MAC function WUSB1.0[6.5]
*
@@ -197,16 +204,15 @@ static void bytewise_xor(void *_bo, const void *_bi1, const void *_bi2,
* what sg[4] is for. Maybe there is a smarter way to do this.
*/
static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
- struct crypto_cipher *tfm_aes, void *mic,
+ struct crypto_cipher *tfm_aes,
+ struct wusb_mac_scratch *scratch,
+ void *mic,
const struct aes_ccm_nonce *n,
const struct aes_ccm_label *a, const void *b,
size_t blen)
{
int result = 0;
SKCIPHER_REQUEST_ON_STACK(req, tfm_cbc);
- struct aes_ccm_b0 b0;
- struct aes_ccm_b1 b1;
- struct aes_ccm_a ax;
struct scatterlist sg[4], sg_dst;
void *dst_buf;
size_t dst_size;
@@ -218,16 +224,17 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
* These checks should be compile time optimized out
* ensure @a fills b1's mac_header and following fields
*/
- WARN_ON(sizeof(*a) != sizeof(b1) - sizeof(b1.la));
- WARN_ON(sizeof(b0) != sizeof(struct aes_ccm_block));
- WARN_ON(sizeof(b1) != sizeof(struct aes_ccm_block));
- WARN_ON(sizeof(ax) != sizeof(struct aes_ccm_block));
+ WARN_ON(sizeof(*a) != sizeof(scratch->b1) - sizeof(scratch->b1.la));
+ WARN_ON(sizeof(scratch->b0) != sizeof(struct aes_ccm_block));
+ WARN_ON(sizeof(scratch->b1) != sizeof(struct aes_ccm_block));
+ WARN_ON(sizeof(scratch->ax) != sizeof(struct aes_ccm_block));
result = -ENOMEM;
zero_padding = blen % sizeof(struct aes_ccm_block);
if (zero_padding)
zero_padding = sizeof(struct aes_ccm_block) - zero_padding;
- dst_size = blen + sizeof(b0) + sizeof(b1) + zero_padding;
+ dst_size = blen + sizeof(scratch->b0) + sizeof(scratch->b1) +
+ zero_padding;
dst_buf = kzalloc(dst_size, GFP_KERNEL);
if (dst_buf == NULL) {
printk(KERN_ERR "E: can't alloc destination buffer\n");
@@ -237,9 +244,9 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
memset(iv, 0, sizeof(iv));
/* Setup B0 */
- b0.flags = 0x59; /* Format B0 */
- b0.ccm_nonce = *n;
- b0.lm = cpu_to_be16(0); /* WUSB1.0[6.5] sez l(m) is 0 */
+ scratch->b0.flags = 0x59; /* Format B0 */
+ scratch->b0.ccm_nonce = *n;
+ scratch->b0.lm = cpu_to_be16(0); /* WUSB1.0[6.5] sez l(m) is 0 */
/* Setup B1
*
@@ -248,12 +255,12 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
* 14'--after clarification, it means to use A's contents
* for MAC Header, EO, sec reserved and padding.
*/
- b1.la = cpu_to_be16(blen + 14);
- memcpy(&b1.mac_header, a, sizeof(*a));
+ scratch->b1.la = cpu_to_be16(blen + 14);
+ memcpy(&scratch->b1.mac_header, a, sizeof(*a));
sg_init_table(sg, ARRAY_SIZE(sg));
- sg_set_buf(&sg[0], &b0, sizeof(b0));
- sg_set_buf(&sg[1], &b1, sizeof(b1));
+ sg_set_buf(&sg[0], &scratch->b0, sizeof(scratch->b0));
+ sg_set_buf(&sg[1], &scratch->b1, sizeof(scratch->b1));
sg_set_buf(&sg[2], b, blen);
/* 0 if well behaved :) */
sg_set_buf(&sg[3], bzero, zero_padding);
@@ -278,11 +285,12 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
* POS Crypto API: size is assumed to be AES's block size.
* Thanks for documenting it -- tip taken from airo.c
*/
- ax.flags = 0x01; /* as per WUSB 1.0 spec */
- ax.ccm_nonce = *n;
- ax.counter = 0;
- crypto_cipher_encrypt_one(tfm_aes, (void *)&ax, (void *)&ax);
- bytewise_xor(mic, &ax, iv, 8);
+ scratch->ax.flags = 0x01; /* as per WUSB 1.0 spec */
+ scratch->ax.ccm_nonce = *n;
+ scratch->ax.counter = 0;
+ crypto_cipher_encrypt_one(tfm_aes, (void *)&scratch->ax,
+ (void *)&scratch->ax);
+ bytewise_xor(mic, &scratch->ax, iv, 8);
result = 8;
error_cbc_crypt:
kfree(dst_buf);
@@ -305,6 +313,7 @@ ssize_t wusb_prf(void *out, size_t out_size,
struct aes_ccm_nonce n = *_n;
struct crypto_skcipher *tfm_cbc;
struct crypto_cipher *tfm_aes;
+ struct wusb_mac_scratch *scratch;
u64 sfn = 0;
__le64 sfn_le;
@@ -331,17 +340,23 @@ ssize_t wusb_prf(void *out, size_t out_size,
printk(KERN_ERR "E: can't set AES key: %d\n", (int)result);
goto error_setkey_aes;
}
+ scratch = kmalloc(sizeof(*scratch), GFP_KERNEL);
+ if (!scratch)
+ goto error_alloc_scratch;
for (bitr = 0; bitr < (len + 63) / 64; bitr++) {
sfn_le = cpu_to_le64(sfn++);
memcpy(&n.sfn, &sfn_le, sizeof(n.sfn)); /* n.sfn++... */
- result = wusb_ccm_mac(tfm_cbc, tfm_aes, out + bytes,
+ result = wusb_ccm_mac(tfm_cbc, tfm_aes, scratch, out + bytes,
&n, a, b, blen);
if (result < 0)
goto error_ccm_mac;
bytes += result;
}
result = bytes;
+
+ kfree(scratch);
+error_alloc_scratch:
error_ccm_mac:
error_setkey_aes:
crypto_free_cipher(tfm_aes);
--
2.7.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] wusb: Stop using the stack for sg crypto scratch space
2016-10-06 17:25 [PATCH] wusb: Stop using the stack for sg crypto scratch space Andy Lutomirski
@ 2016-10-16 17:17 ` Andy Lutomirski
2016-10-17 7:05 ` Greg KH
0 siblings, 1 reply; 3+ messages in thread
From: Andy Lutomirski @ 2016-10-16 17:17 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Greg KH, Herbert Xu, linux-kernel@vger.kernel.org, USB list
On Thu, Oct 6, 2016 at 10:25 AM, Andy Lutomirski <luto@kernel.org> wrote:
> Pointing an sg list at the stack is verboten and, with
> CONFIG_VMAP_STACK=y, will malfunction. Use kmalloc for the wusb
> crypto stack space instead.
>
> Untested -- I'm not entirely convinced that this hardware exists in
> the wild.
Greg, could you queue this up for 4.9? I can't guarantee that it
works, but I can pretty much guarantee that the driver is busted
without it.
>
> Signed-off-by: Andy Lutomirski <luto@kernel.org>
> ---
>
> This is needed for 4.9 for wusb to have a chance of working on default x86
> configurations.
>
> drivers/usb/wusbcore/crypto.c | 59 +++++++++++++++++++++++++++----------------
> 1 file changed, 37 insertions(+), 22 deletions(-)
>
> diff --git a/drivers/usb/wusbcore/crypto.c b/drivers/usb/wusbcore/crypto.c
> index 33acd1599e99..2db79d64e66c 100644
> --- a/drivers/usb/wusbcore/crypto.c
> +++ b/drivers/usb/wusbcore/crypto.c
> @@ -133,6 +133,13 @@ static void bytewise_xor(void *_bo, const void *_bi1, const void *_bi2,
> bo[itr] = bi1[itr] ^ bi2[itr];
> }
>
> +/* Scratch space for MAC calculations. */
> +struct wusb_mac_scratch {
> + struct aes_ccm_b0 b0;
> + struct aes_ccm_b1 b1;
> + struct aes_ccm_a ax;
> +};
> +
> /*
> * CC-MAC function WUSB1.0[6.5]
> *
> @@ -197,16 +204,15 @@ static void bytewise_xor(void *_bo, const void *_bi1, const void *_bi2,
> * what sg[4] is for. Maybe there is a smarter way to do this.
> */
> static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
> - struct crypto_cipher *tfm_aes, void *mic,
> + struct crypto_cipher *tfm_aes,
> + struct wusb_mac_scratch *scratch,
> + void *mic,
> const struct aes_ccm_nonce *n,
> const struct aes_ccm_label *a, const void *b,
> size_t blen)
> {
> int result = 0;
> SKCIPHER_REQUEST_ON_STACK(req, tfm_cbc);
> - struct aes_ccm_b0 b0;
> - struct aes_ccm_b1 b1;
> - struct aes_ccm_a ax;
> struct scatterlist sg[4], sg_dst;
> void *dst_buf;
> size_t dst_size;
> @@ -218,16 +224,17 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
> * These checks should be compile time optimized out
> * ensure @a fills b1's mac_header and following fields
> */
> - WARN_ON(sizeof(*a) != sizeof(b1) - sizeof(b1.la));
> - WARN_ON(sizeof(b0) != sizeof(struct aes_ccm_block));
> - WARN_ON(sizeof(b1) != sizeof(struct aes_ccm_block));
> - WARN_ON(sizeof(ax) != sizeof(struct aes_ccm_block));
> + WARN_ON(sizeof(*a) != sizeof(scratch->b1) - sizeof(scratch->b1.la));
> + WARN_ON(sizeof(scratch->b0) != sizeof(struct aes_ccm_block));
> + WARN_ON(sizeof(scratch->b1) != sizeof(struct aes_ccm_block));
> + WARN_ON(sizeof(scratch->ax) != sizeof(struct aes_ccm_block));
>
> result = -ENOMEM;
> zero_padding = blen % sizeof(struct aes_ccm_block);
> if (zero_padding)
> zero_padding = sizeof(struct aes_ccm_block) - zero_padding;
> - dst_size = blen + sizeof(b0) + sizeof(b1) + zero_padding;
> + dst_size = blen + sizeof(scratch->b0) + sizeof(scratch->b1) +
> + zero_padding;
> dst_buf = kzalloc(dst_size, GFP_KERNEL);
> if (dst_buf == NULL) {
> printk(KERN_ERR "E: can't alloc destination buffer\n");
> @@ -237,9 +244,9 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
> memset(iv, 0, sizeof(iv));
>
> /* Setup B0 */
> - b0.flags = 0x59; /* Format B0 */
> - b0.ccm_nonce = *n;
> - b0.lm = cpu_to_be16(0); /* WUSB1.0[6.5] sez l(m) is 0 */
> + scratch->b0.flags = 0x59; /* Format B0 */
> + scratch->b0.ccm_nonce = *n;
> + scratch->b0.lm = cpu_to_be16(0); /* WUSB1.0[6.5] sez l(m) is 0 */
>
> /* Setup B1
> *
> @@ -248,12 +255,12 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
> * 14'--after clarification, it means to use A's contents
> * for MAC Header, EO, sec reserved and padding.
> */
> - b1.la = cpu_to_be16(blen + 14);
> - memcpy(&b1.mac_header, a, sizeof(*a));
> + scratch->b1.la = cpu_to_be16(blen + 14);
> + memcpy(&scratch->b1.mac_header, a, sizeof(*a));
>
> sg_init_table(sg, ARRAY_SIZE(sg));
> - sg_set_buf(&sg[0], &b0, sizeof(b0));
> - sg_set_buf(&sg[1], &b1, sizeof(b1));
> + sg_set_buf(&sg[0], &scratch->b0, sizeof(scratch->b0));
> + sg_set_buf(&sg[1], &scratch->b1, sizeof(scratch->b1));
> sg_set_buf(&sg[2], b, blen);
> /* 0 if well behaved :) */
> sg_set_buf(&sg[3], bzero, zero_padding);
> @@ -278,11 +285,12 @@ static int wusb_ccm_mac(struct crypto_skcipher *tfm_cbc,
> * POS Crypto API: size is assumed to be AES's block size.
> * Thanks for documenting it -- tip taken from airo.c
> */
> - ax.flags = 0x01; /* as per WUSB 1.0 spec */
> - ax.ccm_nonce = *n;
> - ax.counter = 0;
> - crypto_cipher_encrypt_one(tfm_aes, (void *)&ax, (void *)&ax);
> - bytewise_xor(mic, &ax, iv, 8);
> + scratch->ax.flags = 0x01; /* as per WUSB 1.0 spec */
> + scratch->ax.ccm_nonce = *n;
> + scratch->ax.counter = 0;
> + crypto_cipher_encrypt_one(tfm_aes, (void *)&scratch->ax,
> + (void *)&scratch->ax);
> + bytewise_xor(mic, &scratch->ax, iv, 8);
> result = 8;
> error_cbc_crypt:
> kfree(dst_buf);
> @@ -305,6 +313,7 @@ ssize_t wusb_prf(void *out, size_t out_size,
> struct aes_ccm_nonce n = *_n;
> struct crypto_skcipher *tfm_cbc;
> struct crypto_cipher *tfm_aes;
> + struct wusb_mac_scratch *scratch;
> u64 sfn = 0;
> __le64 sfn_le;
>
> @@ -331,17 +340,23 @@ ssize_t wusb_prf(void *out, size_t out_size,
> printk(KERN_ERR "E: can't set AES key: %d\n", (int)result);
> goto error_setkey_aes;
> }
> + scratch = kmalloc(sizeof(*scratch), GFP_KERNEL);
> + if (!scratch)
> + goto error_alloc_scratch;
>
> for (bitr = 0; bitr < (len + 63) / 64; bitr++) {
> sfn_le = cpu_to_le64(sfn++);
> memcpy(&n.sfn, &sfn_le, sizeof(n.sfn)); /* n.sfn++... */
> - result = wusb_ccm_mac(tfm_cbc, tfm_aes, out + bytes,
> + result = wusb_ccm_mac(tfm_cbc, tfm_aes, scratch, out + bytes,
> &n, a, b, blen);
> if (result < 0)
> goto error_ccm_mac;
> bytes += result;
> }
> result = bytes;
> +
> + kfree(scratch);
> +error_alloc_scratch:
> error_ccm_mac:
> error_setkey_aes:
> crypto_free_cipher(tfm_aes);
> --
> 2.7.4
>
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] wusb: Stop using the stack for sg crypto scratch space
2016-10-16 17:17 ` Andy Lutomirski
@ 2016-10-17 7:05 ` Greg KH
0 siblings, 0 replies; 3+ messages in thread
From: Greg KH @ 2016-10-17 7:05 UTC (permalink / raw)
To: Andy Lutomirski
Cc: Andy Lutomirski, Herbert Xu, linux-kernel@vger.kernel.org,
USB list
On Sun, Oct 16, 2016 at 10:17:53AM -0700, Andy Lutomirski wrote:
> On Thu, Oct 6, 2016 at 10:25 AM, Andy Lutomirski <luto@kernel.org> wrote:
> > Pointing an sg list at the stack is verboten and, with
> > CONFIG_VMAP_STACK=y, will malfunction. Use kmalloc for the wusb
> > crypto stack space instead.
> >
> > Untested -- I'm not entirely convinced that this hardware exists in
> > the wild.
>
> Greg, could you queue this up for 4.9? I can't guarantee that it
> works, but I can pretty much guarantee that the driver is busted
> without it.
Yes, it's in my queue, couldn't do anything until after -rc1 came out.
I'll catch up on it this week.
thanks,
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-10-17 7:05 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-10-06 17:25 [PATCH] wusb: Stop using the stack for sg crypto scratch space Andy Lutomirski
2016-10-16 17:17 ` Andy Lutomirski
2016-10-17 7:05 ` Greg KH
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).