| Message ID | E1r9H1M-00612B-10@formenos.hmeau.com (mailing list archive) |
|---|---|
| State | Accepted |
| Delegated to: | Herbert Xu |
| Headers | show |
| Series | crypto: Fix chaining support for stream ciphers (arc4 only for now) | expand |
Hello,
kernel test robot noticed "WARNING:at_net/core/sock.c:#sock_kzfree_s" on:
commit: 29531d406c4f2b0f07b1d9eb4e24f5ac6b44bc05 ("[v3 PATCH 4/4] crypto: algif_skcipher - Fix stream cipher chaining")
url: https://github.com/intel-lab-lkp/linux/commits/Herbert-Xu/crypto-skcipher-Add-internal-state-support/20231202-123508
base: https://git.kernel.org/cgit/linux/kernel/git/herbert/cryptodev-2.6.git master
patch link: https://lore.kernel.org/all/E1r9H1M-00612B-10@formenos.hmeau.com/
patch subject: [v3 PATCH 4/4] crypto: algif_skcipher - Fix stream cipher chaining
in testcase: ltp
version: ltp-x86_64-14c1f76-1_20230715
with following parameters:
test: crypto
compiler: gcc-12
test machine: 8 threads 1 sockets Intel(R) Core(TM) i7-3770K CPU @ 3.50GHz (Ivy Bridge) with 16G memory
(please refer to attached dmesg/kmsg for entire log/backtrace)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <oliver.sang@intel.com>
| Closes: https://lore.kernel.org/oe-lkp/202312101716.7cbf38c4-oliver.sang@intel.com
kern :warn : [ 242.028749] ------------[ cut here ]------------
kern :warn : [ 242.029073] WARNING: CPU: 3 PID: 3733 at net/core/sock.c:2697 sock_kzfree_s+0x38/0x40
kern :warn : [ 242.030906] Modules linked in: sm4_generic sm4 vmac poly1305_generic libpoly1305 poly1305_x86_64 chacha_generic chacha_x86_64 libchacha chacha20poly1305 sm3_generic sm3 netconsole btrfs blake2b_generic xor raid6_pq zstd_compress libcrc32c intel_rapl_msr intel_rapl_common x86_pkg_temp_thermal sd_mod intel_powerclamp t10_pi coretemp crc64_rocksoft_generic crc64_rocksoft crc64 kvm_intel sg ipmi_devintf ipmi_msghandler i915 kvm irqbypass crct10dif_pclmul crc32_pclmul crc32c_intel ghash_clmulni_intel sha512_ssse3 rapl drm_buddy mxm_wmi intel_gtt intel_cstate ahci drm_display_helper firewire_ohci libahci ttm i2c_i801 firewire_core intel_uncore drm_kms_helper crc_itu_t libata lpc_ich video i2c_smbus wmi binfmt_misc drm fuse ip_tables
kern :warn : [ 242.032427] CPU: 3 PID: 3733 Comm: af_alg05 Not tainted 6.7.0-rc1-00040-g29531d406c4f #1
kern :warn : [ 242.033686] Hardware name: /DZ77BH-55K, BIOS BHZ7710H.86A.0097.2012.1228.1346 12/28/2012
kern :warn : [ 242.033949] RIP: 0010:sock_kzfree_s+0x38/0x40
kern :warn : [ 242.034146] Code: 55 89 d5 53 48 89 fb 48 89 f7 e8 53 8b 82 fe 48 8d bb 48 01 00 00 be 04 00 00 00 e8 22 ad 97 fe f0 29 ab 48 01 00 00 5b 5d c3 <0f> 0b c3 0f 1f 44 00 00 f3 0f 1e fa 0f 1f 44 00 00 55 53 48 89 fb
kern :warn : [ 242.034731] RSP: 0018:ffffc900011bfde8 EFLAGS: 00010246
kern :warn : [ 242.034997] RAX: dffffc0000000000 RBX: ffff8881ad1d5000 RCX: 1ffff110377659a3
kern :warn : [ 242.035308] RDX: 0000000000000000 RSI: 0000000000000000 RDI: ffff8881ad1d5000
kern :warn : [ 242.035614] RBP: ffff8881bbb2cd00 R08: 0000000000000001 R09: ffffed1035a3aa29
kern :warn : [ 242.035913] R10: ffff8881ad1d514b R11: ffffffff83a0009f R12: ffff8881ad1d3048
kern :warn : [ 242.036153] R13: ffff8881a7c089a0 R14: ffff8881ad1d3048 R15: ffff88840eb21900
kern :warn : [ 242.036455] FS: 00007f207e42c740(0000) GS:ffff888348180000(0000) knlGS:0000000000000000
kern :warn : [ 242.036732] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
kern :warn : [ 242.036985] CR2: 00007f89c0a5c2f0 CR3: 0000000403ede005 CR4: 00000000001706f0
kern :warn : [ 242.037288] Call Trace:
kern :warn : [ 242.037496] <TASK>
kern :warn : [ 242.037651] ? __warn+0xcd/0x260
kern :warn : [ 242.037828] ? sock_kzfree_s+0x38/0x40
kern :warn : [ 242.038013] ? report_bug+0x267/0x2d0
kern :warn : [ 242.038199] ? handle_bug+0x3c/0x70
kern :warn : [ 242.038461] ? exc_invalid_op+0x17/0x40
kern :warn : [ 242.038644] ? asm_exc_invalid_op+0x1a/0x20
kern :warn : [ 242.038854] ? entry_SYSCALL_64_after_hwframe+0x63/0x6b
kern :warn : [ 242.039131] ? sock_kzfree_s+0x38/0x40
kern :warn : [ 242.039391] skcipher_sock_destruct+0x1af/0x280
kern :warn : [ 242.039657] __sk_destruct+0x46/0x4e0
kern :warn : [ 242.039862] af_alg_release+0x90/0xc0
kern :warn : [ 242.040074] __sock_release+0xa0/0x250
kern :warn : [ 242.040435] sock_close+0x15/0x20
kern :warn : [ 242.040650] __fput+0x213/0xad0
kern :warn : [ 242.040846] __x64_sys_close+0x7d/0xd0
kern :warn : [ 242.041044] do_syscall_64+0x3f/0xe0
kern :warn : [ 242.041260] entry_SYSCALL_64_after_hwframe+0x63/0x6b
kern :warn : [ 242.041496] RIP: 0033:0x7f207e527780
kern :warn : [ 242.042582] Code: 0d 00 00 00 eb b2 e8 ef f6 01 00 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 44 00 00 80 3d 61 1e 0e 00 00 74 17 b8 03 00 00 00 0f 05 <48> 3d 00 f0 ff ff 77 48 c3 0f 1f 80 00 00 00 00 48 83 ec 18 89 7c
kern :warn : [ 242.043051] RSP: 002b:00007ffef7aefff8 EFLAGS: 00000202 ORIG_RAX: 0000000000000003
kern :warn : [ 242.043430] RAX: ffffffffffffffda RBX: 0000000000000005 RCX: 00007f207e527780
kern :warn : [ 242.043766] RDX: 000055dda9c55b00 RSI: 00007ffef7aefad0 RDI: 0000000000000005
kern :warn : [ 242.044067] RBP: 00007ffef7af2ff0 R08: 0000000000000000 R09: 00007ffef7aeff20
kern :warn : [ 242.044415] R10: 00007ffef7aefae6 R11: 0000000000000202 R12: 00007f207e42c6c0
kern :warn : [ 242.044763] R13: 00007ffef7af0000 R14: 000055dda9c6b01e R15: 0000000000000000
kern :warn : [ 242.045069] </TASK>
kern :warn : [ 242.045310] ---[ end trace 0000000000000000 ]---
The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20231210/202312101716.7cbf38c4-oliver.sang@intel.com
diff --git a/crypto/algif_skcipher.c b/crypto/algif_skcipher.c index 9ada9b741af8..59dcc6fc74a2 100644 --- a/crypto/algif_skcipher.c +++ b/crypto/algif_skcipher.c @@ -47,6 +47,52 @@ static int skcipher_sendmsg(struct socket *sock, struct msghdr *msg, return af_alg_sendmsg(sock, msg, size, ivsize); } +static int algif_skcipher_export(struct sock *sk, struct skcipher_request *req) +{ + struct alg_sock *ask = alg_sk(sk); + struct crypto_skcipher *tfm; + struct af_alg_ctx *ctx; + struct alg_sock *pask; + unsigned statesize; + struct sock *psk; + int err; + + if (!(req->base.flags & CRYPTO_SKCIPHER_REQ_NOTFINAL)) + return 0; + + ctx = ask->private; + psk = ask->parent; + pask = alg_sk(psk); + tfm = pask->private; + + statesize = crypto_skcipher_statesize(tfm); + ctx->state = sock_kmalloc(sk, statesize, GFP_ATOMIC); + if (!ctx->state) + return -ENOMEM; + + err = crypto_skcipher_export(req, ctx->state); + if (err) { + sock_kzfree_s(sk, ctx->state, statesize); + ctx->state = NULL; + } + + return err; +} + +static void algif_skcipher_done(void *data, int err) +{ + struct af_alg_async_req *areq = data; + struct sock *sk = areq->sk; + + if (err) + goto out; + + err = algif_skcipher_export(sk, &areq->cra_u.skcipher_req); + +out: + af_alg_async_cb(data, err); +} + static int _skcipher_recvmsg(struct socket *sock, struct msghdr *msg, size_t ignored, int flags) { @@ -58,6 +104,7 @@ static int _skcipher_recvmsg(struct socket *sock, struct msghdr *msg, struct crypto_skcipher *tfm = pask->private; unsigned int bs = crypto_skcipher_chunksize(tfm); struct af_alg_async_req *areq; + unsigned cflags = 0; int err = 0; size_t len = 0; @@ -82,8 +129,10 @@ static int _skcipher_recvmsg(struct socket *sock, struct msghdr *msg, * If more buffers are to be expected to be processed, process only * full block size buffers. */ - if (ctx->more || len < ctx->used) + if (ctx->more || len < ctx->used) { len -= len % bs; + cflags |= CRYPTO_SKCIPHER_REQ_NOTFINAL; + } /* * Create a per request TX SGL for this request which tracks the @@ -107,6 +156,16 @@ static int _skcipher_recvmsg(struct socket *sock, struct msghdr *msg, skcipher_request_set_crypt(&areq->cra_u.skcipher_req, areq->tsgl, areq->first_rsgl.sgl.sgt.sgl, len, ctx->iv); + if (ctx->state) { + err = crypto_skcipher_import(&areq->cra_u.skcipher_req, + ctx->state); + sock_kzfree_s(sk, ctx->state, crypto_skcipher_statesize(tfm)); + ctx->state = NULL; + if (err) + goto free; + cflags |= CRYPTO_SKCIPHER_REQ_CONT; + } + if (msg->msg_iocb && !is_sync_kiocb(msg->msg_iocb)) { /* AIO operation */ sock_hold(sk); @@ -116,8 +175,9 @@ static int _skcipher_recvmsg(struct socket *sock, struct msghdr *msg, areq->outlen = len; skcipher_request_set_callback(&areq->cra_u.skcipher_req, + cflags | CRYPTO_TFM_REQ_MAY_SLEEP, - af_alg_async_cb, areq); + algif_skcipher_done, areq); err = ctx->enc ? crypto_skcipher_encrypt(&areq->cra_u.skcipher_req) : crypto_skcipher_decrypt(&areq->cra_u.skcipher_req); @@ -130,6 +190,7 @@ static int _skcipher_recvmsg(struct socket *sock, struct msghdr *msg, } else { /* Synchronous operation */ skcipher_request_set_callback(&areq->cra_u.skcipher_req, + cflags | CRYPTO_TFM_REQ_MAY_SLEEP | CRYPTO_TFM_REQ_MAY_BACKLOG, crypto_req_done, &ctx->wait); @@ -137,8 +198,11 @@ static int _skcipher_recvmsg(struct socket *sock, struct msghdr *msg, crypto_skcipher_encrypt(&areq->cra_u.skcipher_req) : crypto_skcipher_decrypt(&areq->cra_u.skcipher_req), &ctx->wait); - } + if (!err) + err = algif_skcipher_export( + sk, &areq->cra_u.skcipher_req); + } free: af_alg_free_resources(areq); @@ -301,6 +365,7 @@ static void skcipher_sock_destruct(struct sock *sk) af_alg_pull_tsgl(sk, ctx->used, NULL, 0); sock_kzfree_s(sk, ctx->iv, crypto_skcipher_ivsize(tfm)); + sock_kzfree_s(sk, ctx->state, crypto_skcipher_statesize(tfm)); sock_kfree_s(sk, ctx, ctx->len); af_alg_release_parent(sk); } diff --git a/include/crypto/if_alg.h b/include/crypto/if_alg.h index 08b803a4fcde..78ecaf5db04c 100644 --- a/include/crypto/if_alg.h +++ b/include/crypto/if_alg.h @@ -121,6 +121,7 @@ struct af_alg_async_req { * * @tsgl_list: Link to TX SGL * @iv: IV for cipher operation + * @state: Existing state for continuing operation * @aead_assoclen: Length of AAD for AEAD cipher operations * @completion: Work queue for synchronous operation * @used: TX bytes sent to kernel. This variable is used to @@ -142,6 +143,7 @@ struct af_alg_ctx { struct list_head tsgl_list; void *iv; + void *state; size_t aead_assoclen; struct crypto_wait wait;
Unlike algif_aead which is always issued in one go (thus limiting the maximum size of the request), algif_skcipher has always allowed unlimited input data by cutting them up as necessary and feeding the fragments to the underlying algorithm one at a time. However, because of deficiencies in the API, this has been broken for most stream ciphers such as arc4 or chacha. This is because they have an internal state in addition to the IV that must be preserved in order to continue processing. Fix this by using the new skcipher state API. Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> --- crypto/algif_skcipher.c | 71 +++++++++++++++++++++++++++++++++++++++++++++--- include/crypto/if_alg.h | 2 + 2 files changed, 70 insertions(+), 3 deletions(-)