Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

crypto: arm64/aes-ccm - Rewrite skcipher walker loop

An often overlooked aspect of the skcipher walker API is that an
error is not just indicated by a non-zero return value, but by the
fact that walk->nbytes is zero.

Thus it is an error to call skcipher_walk_done after getting back
walk->nbytes == 0 from the previous interaction with the walker.

This is because when walk->nbytes is zero the walker is left in
an undefined state and any further calls to it may try to free
uninitialised stack memory.

The arm64 ccm code has to deal with zero-length messages, and
it needs to process data even when walk->nbytes == 0 is returned.
It doesn't have this bug because there is an explicit check for
walk->nbytes != 0 prior to the skcipher_walk_done call.

However, the loop is still sufficiently different from the usual
layout and it appears to have been copied into other code which
then ended up with this bug. This patch rewrites it to follow the
usual convention of checking walk->nbytes.

Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Tested-by: Ard Biesheuvel <ardb@kernel.org>
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

+26 -31
+26 -31
arch/arm64/crypto/aes-ce-ccm-glue.c
··· 161 161 memcpy(buf, req->iv, AES_BLOCK_SIZE); 162 162 163 163 err = skcipher_walk_aead_encrypt(&walk, req, false); 164 - if (unlikely(err)) 165 - return err; 166 164 167 165 kernel_neon_begin(); 168 166 169 167 if (req->assoclen) 170 168 ccm_calculate_auth_mac(req, mac); 171 169 172 - do { 170 + while (walk.nbytes) { 173 171 u32 tail = walk.nbytes % AES_BLOCK_SIZE; 172 + bool final = walk.nbytes == walk.total; 174 173 175 - if (walk.nbytes == walk.total) 174 + if (final) 176 175 tail = 0; 177 176 178 177 ce_aes_ccm_encrypt(walk.dst.virt.addr, walk.src.virt.addr, 179 178 walk.nbytes - tail, ctx->key_enc, 180 179 num_rounds(ctx), mac, walk.iv); 181 180 182 - if (walk.nbytes == walk.total) 183 - ce_aes_ccm_final(mac, buf, ctx->key_enc, num_rounds(ctx)); 181 + if (!final) 182 + kernel_neon_end(); 183 + err = skcipher_walk_done(&walk, tail); 184 + if (!final) 185 + kernel_neon_begin(); 186 + } 184 187 185 - kernel_neon_end(); 188 + ce_aes_ccm_final(mac, buf, ctx->key_enc, num_rounds(ctx)); 186 189 187 - if (walk.nbytes) { 188 - err = skcipher_walk_done(&walk, tail); 189 - if (unlikely(err)) 190 - return err; 191 - if (unlikely(walk.nbytes)) 192 - kernel_neon_begin(); 193 - } 194 - } while (walk.nbytes); 190 + kernel_neon_end(); 195 191 196 192 /* copy authtag to end of dst */ 197 193 scatterwalk_map_and_copy(mac, req->dst, req->assoclen + req->cryptlen, 198 194 crypto_aead_authsize(aead), 1); 199 195 200 - return 0; 196 + return err; 201 197 } 202 198 203 199 static int ccm_decrypt(struct aead_request *req) ··· 215 219 memcpy(buf, req->iv, AES_BLOCK_SIZE); 216 220 217 221 err = skcipher_walk_aead_decrypt(&walk, req, false); 218 - if (unlikely(err)) 219 - return err; 220 222 221 223 kernel_neon_begin(); 222 224 223 225 if (req->assoclen) 224 226 ccm_calculate_auth_mac(req, mac); 225 227 226 - do { 228 + while (walk.nbytes) { 227 229 u32 tail = walk.nbytes % AES_BLOCK_SIZE; 230 + bool final = walk.nbytes == walk.total; 228 231 229 - if (walk.nbytes == walk.total) 232 + if (final) 230 233 tail = 0; 231 234 232 235 ce_aes_ccm_decrypt(walk.dst.virt.addr, walk.src.virt.addr, 233 236 walk.nbytes - tail, ctx->key_enc, 234 237 num_rounds(ctx), mac, walk.iv); 235 238 236 - if (walk.nbytes == walk.total) 237 - ce_aes_ccm_final(mac, buf, ctx->key_enc, num_rounds(ctx)); 239 + if (!final) 240 + kernel_neon_end(); 241 + err = skcipher_walk_done(&walk, tail); 242 + if (!final) 243 + kernel_neon_begin(); 244 + } 238 245 239 - kernel_neon_end(); 246 + ce_aes_ccm_final(mac, buf, ctx->key_enc, num_rounds(ctx)); 240 247 241 - if (walk.nbytes) { 242 - err = skcipher_walk_done(&walk, tail); 243 - if (unlikely(err)) 244 - return err; 245 - if (unlikely(walk.nbytes)) 246 - kernel_neon_begin(); 247 - } 248 - } while (walk.nbytes); 248 + kernel_neon_end(); 249 + 250 + if (unlikely(err)) 251 + return err; 249 252 250 253 /* compare calculated auth tag with the stored one */ 251 254 scatterwalk_map_and_copy(buf, req->src,