]>
Commit | Line | Data |
---|---|---|
a4b7cc35 GKH |
1 | From 7e92e1717e3eaf6b322c252947c696b3059f05be Mon Sep 17 00:00:00 2001 |
2 | From: Christian Lamparter <chunkeey@gmail.com> | |
3 | Date: Mon, 22 Apr 2019 13:25:59 +0200 | |
4 | Subject: crypto: crypto4xx - fix cfb and ofb "overran dst buffer" issues | |
5 | ||
6 | From: Christian Lamparter <chunkeey@gmail.com> | |
7 | ||
8 | commit 7e92e1717e3eaf6b322c252947c696b3059f05be upstream. | |
9 | ||
10 | Currently, crypto4xx CFB and OFB AES ciphers are | |
11 | failing testmgr's test vectors. | |
12 | ||
13 | |cfb-aes-ppc4xx encryption overran dst buffer on test vector 3, cfg="in-place" | |
14 | |ofb-aes-ppc4xx encryption overran dst buffer on test vector 1, cfg="in-place" | |
15 | ||
16 | This is because of a very subtile "bug" in the hardware that | |
17 | gets indirectly mentioned in 18.1.3.5 Encryption/Decryption | |
18 | of the hardware spec: | |
19 | ||
20 | the OFB and CFB modes for AES are listed there as operation | |
21 | modes for >>> "Block ciphers" <<<. Which kind of makes sense, | |
22 | but we would like them to be considered as stream ciphers just | |
23 | like the CTR mode. | |
24 | ||
25 | To workaround this issue and stop the hardware from causing | |
26 | "overran dst buffer" on crypttexts that are not a multiple | |
27 | of 16 (AES_BLOCK_SIZE), we force the driver to use the scatter | |
28 | buffers as the go-between. | |
29 | ||
30 | As a bonus this patch also kills redundant pd_uinfo->num_gd | |
31 | and pd_uinfo->num_sd setters since the value has already been | |
32 | set before. | |
33 | ||
34 | Cc: stable@vger.kernel.org | |
35 | Fixes: f2a13e7cba9e ("crypto: crypto4xx - enable AES RFC3686, ECB, CFB and OFB offloads") | |
36 | Signed-off-by: Christian Lamparter <chunkeey@gmail.com> | |
37 | Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> | |
38 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
39 | ||
40 | --- | |
41 | drivers/crypto/amcc/crypto4xx_core.c | 31 +++++++++++++++++++++---------- | |
42 | 1 file changed, 21 insertions(+), 10 deletions(-) | |
43 | ||
44 | --- a/drivers/crypto/amcc/crypto4xx_core.c | |
45 | +++ b/drivers/crypto/amcc/crypto4xx_core.c | |
46 | @@ -712,7 +712,23 @@ int crypto4xx_build_pd(struct crypto_asy | |
47 | size_t offset_to_sr_ptr; | |
48 | u32 gd_idx = 0; | |
49 | int tmp; | |
50 | - bool is_busy; | |
51 | + bool is_busy, force_sd; | |
52 | + | |
53 | + /* | |
54 | + * There's a very subtile/disguised "bug" in the hardware that | |
55 | + * gets indirectly mentioned in 18.1.3.5 Encryption/Decryption | |
56 | + * of the hardware spec: | |
57 | + * *drum roll* the AES/(T)DES OFB and CFB modes are listed as | |
58 | + * operation modes for >>> "Block ciphers" <<<. | |
59 | + * | |
60 | + * To workaround this issue and stop the hardware from causing | |
61 | + * "overran dst buffer" on crypttexts that are not a multiple | |
62 | + * of 16 (AES_BLOCK_SIZE), we force the driver to use the | |
63 | + * scatter buffers. | |
64 | + */ | |
65 | + force_sd = (req_sa->sa_command_1.bf.crypto_mode9_8 == CRYPTO_MODE_CFB | |
66 | + || req_sa->sa_command_1.bf.crypto_mode9_8 == CRYPTO_MODE_OFB) | |
67 | + && (datalen % AES_BLOCK_SIZE); | |
68 | ||
69 | /* figure how many gd are needed */ | |
70 | tmp = sg_nents_for_len(src, assoclen + datalen); | |
71 | @@ -730,7 +746,7 @@ int crypto4xx_build_pd(struct crypto_asy | |
72 | } | |
73 | ||
74 | /* figure how many sd are needed */ | |
75 | - if (sg_is_last(dst)) { | |
76 | + if (sg_is_last(dst) && force_sd == false) { | |
77 | num_sd = 0; | |
78 | } else { | |
79 | if (datalen > PPC4XX_SD_BUFFER_SIZE) { | |
80 | @@ -805,9 +821,10 @@ int crypto4xx_build_pd(struct crypto_asy | |
81 | pd->sa_len = sa_len; | |
82 | ||
83 | pd_uinfo = &dev->pdr_uinfo[pd_entry]; | |
84 | - pd_uinfo->async_req = req; | |
85 | pd_uinfo->num_gd = num_gd; | |
86 | pd_uinfo->num_sd = num_sd; | |
87 | + pd_uinfo->dest_va = dst; | |
88 | + pd_uinfo->async_req = req; | |
89 | ||
90 | if (iv_len) | |
91 | memcpy(pd_uinfo->sr_va->save_iv, iv, iv_len); | |
92 | @@ -826,7 +843,6 @@ int crypto4xx_build_pd(struct crypto_asy | |
93 | /* get first gd we are going to use */ | |
94 | gd_idx = fst_gd; | |
95 | pd_uinfo->first_gd = fst_gd; | |
96 | - pd_uinfo->num_gd = num_gd; | |
97 | gd = crypto4xx_get_gdp(dev, &gd_dma, gd_idx); | |
98 | pd->src = gd_dma; | |
99 | /* enable gather */ | |
100 | @@ -863,17 +879,14 @@ int crypto4xx_build_pd(struct crypto_asy | |
101 | * Indicate gather array is not used | |
102 | */ | |
103 | pd_uinfo->first_gd = 0xffffffff; | |
104 | - pd_uinfo->num_gd = 0; | |
105 | } | |
106 | - if (sg_is_last(dst)) { | |
107 | + if (!num_sd) { | |
108 | /* | |
109 | * we know application give us dst a whole piece of memory | |
110 | * no need to use scatter ring. | |
111 | */ | |
112 | pd_uinfo->using_sd = 0; | |
113 | pd_uinfo->first_sd = 0xffffffff; | |
114 | - pd_uinfo->num_sd = 0; | |
115 | - pd_uinfo->dest_va = dst; | |
116 | sa->sa_command_0.bf.scatter = 0; | |
117 | pd->dest = (u32)dma_map_page(dev->core_dev->device, | |
118 | sg_page(dst), dst->offset, | |
119 | @@ -887,9 +900,7 @@ int crypto4xx_build_pd(struct crypto_asy | |
120 | nbytes = datalen; | |
121 | sa->sa_command_0.bf.scatter = 1; | |
122 | pd_uinfo->using_sd = 1; | |
123 | - pd_uinfo->dest_va = dst; | |
124 | pd_uinfo->first_sd = fst_sd; | |
125 | - pd_uinfo->num_sd = num_sd; | |
126 | sd = crypto4xx_get_sdp(dev, &sd_dma, sd_idx); | |
127 | pd->dest = sd_dma; | |
128 | /* setup scatter descriptor */ |