| Message ID | 20221208164610.867747-1-roberto.sassu@huaweicloud.com |
|---|---|
| State | New |
| Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org>
Delivered-To: ouuuleilei@gmail.com
Received: by 2002:adf:f944:0:0:0:0:0 with SMTP id q4csp303919wrr;
Thu, 8 Dec 2022 08:50:37 -0800 (PST)
X-Google-Smtp-Source:
AA0mqf4O1pHoBJJhQvF6zlA9yC6Vnw72AJqfFmLoEPap038Q3dHDrRu8ZezJApkR3od9K7/HA0CQ
X-Received: by 2002:a17:906:9c8a:b0:7bf:6698:d444 with SMTP id
fj10-20020a1709069c8a00b007bf6698d444mr40110771ejc.548.1670518237355;
Thu, 08 Dec 2022 08:50:37 -0800 (PST)
ARC-Seal: i=1; a=rsa-sha256; t=1670518237; cv=none;
d=google.com; s=arc-20160816;
b=tQpvVvaCreXFJB8L9d7oIjBgoO/mQ7CX56k2vDgRukssM+/E5yWnXEMDKROiErpJ3V
ab03gki19eCpDahaylQlTI+jfj9rWhSBUSzVhFXPJQ5xGmEYm48raGLe5/7gBpcUPLn0
wcFL7owvkuRfl0hEnAGiebMAR7c7nm6ZeMQAmC6AcEn95qTM5yVVMrQOZa54KVL/QyyH
tt9jiszdEyWOGV+Y4fpLZo86tIkzcSgMHFMIv9auluyMt7r3iUdAMGxEAny9z+fJukb5
GDYpKwGm3GJPOe9oyiiqUE9P8a5r0KqOHg8nNkNVnIQLOp4ehzqFZIRTlL1fvs8WiYWP
xT1Q==
ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com;
s=arc-20160816;
h=list-id:precedence:content-transfer-encoding:mime-version
:message-id:date:subject:cc:to:from;
bh=W0Oq4dOUL/VpHkA8YIuvJYigHWIpszdym+Ir/XWr4+M=;
b=WJaYiftNOGzrL4Hc7E4qagK7V3X2BnnK1nlafNBZk/0GRmzsMX6yJIFX4sHzfo37/t
gChHmKB6TUlVcb75XqJO5lesU+gEuwm72+oRAalDpqBmn98Ly+fSeefTF/nhDWJ4rMJn
5EwoY71Q/7l4dS5/gFsjvT6Zw1M7Ap7BYBqvrGRkziSw7q8yyRAzrvs2kfFK0ec/qdoa
jRrnfx5DTu7yn2+OkPrj3EZ8Pb8zh6yW+zvvcIB2zU9LCI02Yj3IbCDgZisWISYjOMDF
qvRTZalZF2PqsSbpUSSq3Z5zKNHH58vLhLLo2y5wGN+EqQKxusRWq1+YbmgMVeVJ5ZbH
niWg==
ARC-Authentication-Results: i=1; mx.google.com;
spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
smtp.mailfrom=linux-kernel-owner@vger.kernel.org
Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20])
by mx.google.com with ESMTP id
l12-20020a056402254c00b0046afbd6c84fsi7781010edb.553.2022.12.08.08.50.14;
Thu, 08 Dec 2022 08:50:37 -0800 (PST)
Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
client-ip=2620:137:e000::1:20;
Authentication-Results: mx.google.com;
spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org
designates 2620:137:e000::1:20 as permitted sender)
smtp.mailfrom=linux-kernel-owner@vger.kernel.org
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
id S230258AbiLHQqw (ORCPT <rfc822;foxyelen666@gmail.com>
+ 99 others); Thu, 8 Dec 2022 11:46:52 -0500
Received: from lindbergh.monkeyblade.net ([23.128.96.19]:52442 "EHLO
lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
with ESMTP id S229646AbiLHQqu (ORCPT
<rfc822;linux-kernel@vger.kernel.org>);
Thu, 8 Dec 2022 11:46:50 -0500
Received: from frasgout13.his.huawei.com (frasgout13.his.huawei.com
[14.137.139.46])
by lindbergh.monkeyblade.net (Postfix) with ESMTPS id E4005AD9B9;
Thu, 8 Dec 2022 08:46:47 -0800 (PST)
Received: from mail02.huawei.com (unknown [172.18.147.228])
by frasgout13.his.huawei.com (SkyGuard) with ESMTP id
4NSfy10j7pz9xG9q;
Fri, 9 Dec 2022 00:39:37 +0800 (CST)
Received: from huaweicloud.com (unknown [10.204.63.22])
by APP1 (Coremail) with SMTP id LxC2BwDHs3DWFJJjUyvOAA--.59467S2;
Thu, 08 Dec 2022 17:46:23 +0100 (CET)
From: Roberto Sassu <roberto.sassu@huaweicloud.com>
To: dhowells@redhat.com, herbert@gondor.apana.org.au,
davem@davemloft.net, zohar@linux.ibm.com,
dmitry.kasatkin@gmail.com, paul@paul-moore.com, jmorris@namei.org,
serge@hallyn.com
Cc: linux-integrity@vger.kernel.org,
linux-security-module@vger.kernel.org, keyrings@vger.kernel.org,
linux-crypto@vger.kernel.org, linux-kernel@vger.kernel.org,
Roberto Sassu <roberto.sassu@huawei.com>,
stable@vger.kernel.org, Eric Biggers <ebiggers@kernel.org>
Subject: [PATCH] KEYS: asymmetric: Make a copy of sig and digest in vmalloced
stack
Date: Thu, 8 Dec 2022 17:46:10 +0100
Message-Id: <20221208164610.867747-1-roberto.sassu@huaweicloud.com>
X-Mailer: git-send-email 2.25.1
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
X-CM-TRANSID: LxC2BwDHs3DWFJJjUyvOAA--.59467S2
X-Coremail-Antispam: 1UD129KBjvJXoWxJF4xCry8tF4DCr43Kr4fuFg_yoW5uFWkpa
95Wr15tryUGr1Ik3y3C3WxK345Aw4vkr17Ww4fZw45CFsxXrW8C3yIva13WFyfJrykXFWx
trWvqws8uF1UXaDanT9S1TB71UUUUU7qnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2
9KBjDU0xBIdaVrnRJUUUvFb4IE77IF4wAFF20E14v26ryj6rWUM7CY07I20VC2zVCF04k2
6cxKx2IYs7xG6rWj6s0DM7CIcVAFz4kK6r1j6r18M28lY4IEw2IIxxk0rwA2F7IY1VAKz4
vEj48ve4kI8wA2z4x0Y4vE2Ix0cI8IcVAFwI0_Jr0_JF4l84ACjcxK6xIIjxv20xvEc7Cj
xVAFwI0_Gr0_Cr1l84ACjcxK6I8E87Iv67AKxVW8JVWxJwA2z4x0Y4vEx4A2jsIEc7CjxV
AFwI0_Gr0_Gr1UM2AIxVAIcxkEcVAq07x20xvEncxIr21l5I8CrVACY4xI64kE6c02F40E
x7xfMcIj6xIIjxv20xvE14v26r106r15McIj6I8E87Iv67AKxVWUJVW8JwAm72CE4IkC6x
0Yz7v_Jr0_Gr1lF7xvr2IYc2Ij64vIr41lFIxGxcIEc7CjxVA2Y2ka0xkIwI1lc7CjxVAa
w2AFwI0_GFv_Wryl42xK82IYc2Ij64vIr41l4I8I3I0E4IkC6x0Yz7v_Jr0_Gr1lx2IqxV
Aqx4xG67AKxVWUJVWUGwC20s026x8GjcxK67AKxVWUGVWUWwC2zVAF1VAY17CE14v26r4a
6rW5MIIYrxkI7VAKI48JMIIF0xvE2Ix0cI8IcVAFwI0_Jr0_JF4lIxAIcVC0I7IYx2IY6x
kF7I0E14v26r4j6F4UMIIF0xvE42xK8VAvwI8IcIk0rVW8JVW3JwCI42IY6I8E87Iv67AK
xVWUJVW8JwCI42IY6I8E87Iv6xkF7I0E14v26r4j6r4UJbIYCTnIWIevJa73UjIFyTuYvj
xUxo7KDUUUU
X-CM-SenderInfo: purev21wro2thvvxqx5xdzvxpfor3voofrz/1tbiAgAABF1jj4JxzwAAsL
X-CFilter-Loop: Reflected
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,SPF_HELO_NONE,
SPF_PASS autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?=
X-GMAIL-THRID: =?utf-8?q?1751665331400262148?=
X-GMAIL-MSGID: =?utf-8?q?1751665331400262148?=
|
| Series |
KEYS: asymmetric: Make a copy of sig and digest in vmalloced stack
|
|
Commit Message
Roberto Sassu
Dec. 8, 2022, 4:46 p.m. UTC
From: Roberto Sassu <roberto.sassu@huawei.com> Commit ac4e97abce9b8 ("scatterlist: sg_set_buf() argument must be in linear mapping") checks that both the signature and the digest reside in the linear mapping area. However, more recently commit ba14a194a434c ("fork: Add generic vmalloced stack support"), made it possible to move the stack in the vmalloc area, which is not contiguous, and thus not suitable for sg_set_buf() which needs adjacent pages. Check if the signature and digest passed to public_key_verify_signature() are in the linear mapping area and, for those which are not, make a copy in the linear mapping area with kmalloc() and adjust the pointer passed to sg_set_buf(). Reuse the existing kmalloc() and increase the allocation size as needed. Minimize the number of copies with the compile-time check of CONFIG_VMAP_STACK and with the run-time check virt_addr_valid(). Cc: stable@vger.kernel.org # 4.9.x Fixes: ba14a194a434 ("fork: Add generic vmalloced stack support") Link: https://lore.kernel.org/linux-integrity/Y4pIpxbjBdajymBJ@sol.localdomain/ Suggested-by: Eric Biggers <ebiggers@kernel.org> Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com> --- crypto/asymmetric_keys/public_key.c | 39 +++++++++++++++++++++++++---- 1 file changed, 34 insertions(+), 5 deletions(-)
Comments
On Thu, Dec 08, 2022 at 05:46:10PM +0100, Roberto Sassu wrote: > diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c > index 2f8352e88860..307799ffbc3e 100644 > --- a/crypto/asymmetric_keys/public_key.c > +++ b/crypto/asymmetric_keys/public_key.c > @@ -363,7 +363,8 @@ int public_key_verify_signature(const struct public_key *pkey, > struct scatterlist src_sg[2]; > char alg_name[CRYPTO_MAX_ALG_NAME]; > char *key, *ptr; > - int ret; > + char *sig_s, *digest; > + int ret, verif_bundle_len; > > pr_devel("==>%s()\n", __func__); > > @@ -400,8 +401,21 @@ int public_key_verify_signature(const struct public_key *pkey, > if (!req) > goto error_free_tfm; > > - key = kmalloc(pkey->keylen + sizeof(u32) * 2 + pkey->paramlen, > - GFP_KERNEL); > + verif_bundle_len = pkey->keylen + sizeof(u32) * 2 + pkey->paramlen; > + > + sig_s = sig->s; > + digest = sig->digest; > + > + if (IS_ENABLED(CONFIG_VMAP_STACK)) { > + if (!virt_addr_valid(sig_s)) > + verif_bundle_len += sig->s_size; > + > + if (!virt_addr_valid(digest)) > + verif_bundle_len += sig->digest_size; > + } > + > + /* key points to a buffer which could contain the sig and digest too. */ > + key = kmalloc(verif_bundle_len, GFP_KERNEL); > if (!key) > goto error_free_req; > > @@ -424,9 +438,24 @@ int public_key_verify_signature(const struct public_key *pkey, > goto error_free_key; > } > > + if (IS_ENABLED(CONFIG_VMAP_STACK)) { > + ptr += pkey->paramlen; > + > + if (!virt_addr_valid(sig_s)) { > + sig_s = ptr; > + memcpy(sig_s, sig->s, sig->s_size); > + ptr += sig->s_size; > + } > + > + if (!virt_addr_valid(digest)) { > + digest = ptr; > + memcpy(digest, sig->digest, sig->digest_size); > + } > + } > + > sg_init_table(src_sg, 2); > - sg_set_buf(&src_sg[0], sig->s, sig->s_size); > - sg_set_buf(&src_sg[1], sig->digest, sig->digest_size); > + sg_set_buf(&src_sg[0], sig_s, sig->s_size); > + sg_set_buf(&src_sg[1], digest, sig->digest_size); > akcipher_request_set_crypt(req, src_sg, NULL, sig->s_size, > sig->digest_size); > crypto_init_wait(&cwait); We should try to avoid adding error-prone special cases. How about just doing the copy of the signature and digest unconditionally? That would be much simpler. It would even mean that the scatterlist would only need one element. Also, the size of buffer needed is only max(pkey->keylen + sizeof(u32) * 2 + pkey->paramlen, sig->s_size + sig->digest_size) ... since the signature and digest aren't needed until the key was already used. - Eric
On Thu, 2022-12-08 at 15:17 -0800, Eric Biggers wrote: > On Thu, Dec 08, 2022 at 05:46:10PM +0100, Roberto Sassu wrote: > > diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c > > index 2f8352e88860..307799ffbc3e 100644 > > --- a/crypto/asymmetric_keys/public_key.c > > +++ b/crypto/asymmetric_keys/public_key.c > > @@ -363,7 +363,8 @@ int public_key_verify_signature(const struct public_key *pkey, > > struct scatterlist src_sg[2]; > > char alg_name[CRYPTO_MAX_ALG_NAME]; > > char *key, *ptr; > > - int ret; > > + char *sig_s, *digest; > > + int ret, verif_bundle_len; > > > > pr_devel("==>%s()\n", __func__); > > > > @@ -400,8 +401,21 @@ int public_key_verify_signature(const struct public_key *pkey, > > if (!req) > > goto error_free_tfm; > > > > - key = kmalloc(pkey->keylen + sizeof(u32) * 2 + pkey->paramlen, > > - GFP_KERNEL); > > + verif_bundle_len = pkey->keylen + sizeof(u32) * 2 + pkey->paramlen; > > + > > + sig_s = sig->s; > > + digest = sig->digest; > > + > > + if (IS_ENABLED(CONFIG_VMAP_STACK)) { > > + if (!virt_addr_valid(sig_s)) > > + verif_bundle_len += sig->s_size; > > + > > + if (!virt_addr_valid(digest)) > > + verif_bundle_len += sig->digest_size; > > + } > > + > > + /* key points to a buffer which could contain the sig and digest too. */ > > + key = kmalloc(verif_bundle_len, GFP_KERNEL); > > if (!key) > > goto error_free_req; > > > > @@ -424,9 +438,24 @@ int public_key_verify_signature(const struct public_key *pkey, > > goto error_free_key; > > } > > > > + if (IS_ENABLED(CONFIG_VMAP_STACK)) { > > + ptr += pkey->paramlen; > > + > > + if (!virt_addr_valid(sig_s)) { > > + sig_s = ptr; > > + memcpy(sig_s, sig->s, sig->s_size); > > + ptr += sig->s_size; > > + } > > + > > + if (!virt_addr_valid(digest)) { > > + digest = ptr; > > + memcpy(digest, sig->digest, sig->digest_size); > > + } > > + } > > + > > sg_init_table(src_sg, 2); > > - sg_set_buf(&src_sg[0], sig->s, sig->s_size); > > - sg_set_buf(&src_sg[1], sig->digest, sig->digest_size); > > + sg_set_buf(&src_sg[0], sig_s, sig->s_size); > > + sg_set_buf(&src_sg[1], digest, sig->digest_size); > > akcipher_request_set_crypt(req, src_sg, NULL, sig->s_size, > > sig->digest_size); > > crypto_init_wait(&cwait); > > We should try to avoid adding error-prone special cases. How about just doing > the copy of the signature and digest unconditionally? That would be much > simpler. It would even mean that the scatterlist would only need one element. Took some time to figure out why Redzone was overwritten. There must be two separate scatterlists. If you set the first only with the sum of the key length and digest length, mpi_read_raw_from_sgl() called by rsa_enc() is going to write before the d pointer in MPI. for (x = 0; x < len; x++) { a <<= 8; a |= *buff++; if (((z + x + 1) % BYTES_PER_MPI_LIMB) == 0) { val->d[j--] = a; a = 0; } } Roberto > Also, the size of buffer needed is only > > max(pkey->keylen + sizeof(u32) * 2 + pkey->paramlen, > sig->s_size + sig->digest_size) > > ... since the signature and digest aren't needed until the key was already used. > > - Eric
On Fri, 2022-12-09 at 15:15 +0100, Roberto Sassu wrote: > On Thu, 2022-12-08 at 15:17 -0800, Eric Biggers wrote: > > On Thu, Dec 08, 2022 at 05:46:10PM +0100, Roberto Sassu wrote: > > > diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c > > > index 2f8352e88860..307799ffbc3e 100644 > > > --- a/crypto/asymmetric_keys/public_key.c > > > +++ b/crypto/asymmetric_keys/public_key.c > > > @@ -363,7 +363,8 @@ int public_key_verify_signature(const struct public_key *pkey, > > > struct scatterlist src_sg[2]; > > > char alg_name[CRYPTO_MAX_ALG_NAME]; > > > char *key, *ptr; > > > - int ret; > > > + char *sig_s, *digest; > > > + int ret, verif_bundle_len; > > > > > > pr_devel("==>%s()\n", __func__); > > > > > > @@ -400,8 +401,21 @@ int public_key_verify_signature(const struct public_key *pkey, > > > if (!req) > > > goto error_free_tfm; > > > > > > - key = kmalloc(pkey->keylen + sizeof(u32) * 2 + pkey->paramlen, > > > - GFP_KERNEL); > > > + verif_bundle_len = pkey->keylen + sizeof(u32) * 2 + pkey->paramlen; > > > + > > > + sig_s = sig->s; > > > + digest = sig->digest; > > > + > > > + if (IS_ENABLED(CONFIG_VMAP_STACK)) { > > > + if (!virt_addr_valid(sig_s)) > > > + verif_bundle_len += sig->s_size; > > > + > > > + if (!virt_addr_valid(digest)) > > > + verif_bundle_len += sig->digest_size; > > > + } > > > + > > > + /* key points to a buffer which could contain the sig and digest too. */ > > > + key = kmalloc(verif_bundle_len, GFP_KERNEL); > > > if (!key) > > > goto error_free_req; > > > > > > @@ -424,9 +438,24 @@ int public_key_verify_signature(const struct public_key *pkey, > > > goto error_free_key; > > > } > > > > > > + if (IS_ENABLED(CONFIG_VMAP_STACK)) { > > > + ptr += pkey->paramlen; > > > + > > > + if (!virt_addr_valid(sig_s)) { > > > + sig_s = ptr; > > > + memcpy(sig_s, sig->s, sig->s_size); > > > + ptr += sig->s_size; > > > + } > > > + > > > + if (!virt_addr_valid(digest)) { > > > + digest = ptr; > > > + memcpy(digest, sig->digest, sig->digest_size); > > > + } > > > + } > > > + > > > sg_init_table(src_sg, 2); > > > - sg_set_buf(&src_sg[0], sig->s, sig->s_size); > > > - sg_set_buf(&src_sg[1], sig->digest, sig->digest_size); > > > + sg_set_buf(&src_sg[0], sig_s, sig->s_size); > > > + sg_set_buf(&src_sg[1], digest, sig->digest_size); > > > akcipher_request_set_crypt(req, src_sg, NULL, sig->s_size, > > > sig->digest_size); > > > crypto_init_wait(&cwait); > > > > We should try to avoid adding error-prone special cases. How about just doing > > the copy of the signature and digest unconditionally? That would be much > > simpler. It would even mean that the scatterlist would only need one element. > > Took some time to figure out why Redzone was overwritten. > > There must be two separate scatterlists. If you set the first only with > the sum of the key length and digest length, mpi_read_raw_from_sgl() Of signature length and digest length. Roberto > called by rsa_enc() is going to write before the d pointer in MPI. > > for (x = 0; x < len; x++) { > a <<= 8; > a |= *buff++; > if (((z + x + 1) % BYTES_PER_MPI_LIMB) == 0) { > val->d[j--] = a; > a = 0; > } > } > > Roberto > > > Also, the size of buffer needed is only > > > > max(pkey->keylen + sizeof(u32) * 2 + pkey->paramlen, > > sig->s_size + sig->digest_size) > > > > ... since the signature and digest aren't needed until the key was already used. > > > > - Eric
diff --git a/crypto/asymmetric_keys/public_key.c b/crypto/asymmetric_keys/public_key.c index 2f8352e88860..307799ffbc3e 100644 --- a/crypto/asymmetric_keys/public_key.c +++ b/crypto/asymmetric_keys/public_key.c @@ -363,7 +363,8 @@ int public_key_verify_signature(const struct public_key *pkey, struct scatterlist src_sg[2]; char alg_name[CRYPTO_MAX_ALG_NAME]; char *key, *ptr; - int ret; + char *sig_s, *digest; + int ret, verif_bundle_len; pr_devel("==>%s()\n", __func__); @@ -400,8 +401,21 @@ int public_key_verify_signature(const struct public_key *pkey, if (!req) goto error_free_tfm; - key = kmalloc(pkey->keylen + sizeof(u32) * 2 + pkey->paramlen, - GFP_KERNEL); + verif_bundle_len = pkey->keylen + sizeof(u32) * 2 + pkey->paramlen; + + sig_s = sig->s; + digest = sig->digest; + + if (IS_ENABLED(CONFIG_VMAP_STACK)) { + if (!virt_addr_valid(sig_s)) + verif_bundle_len += sig->s_size; + + if (!virt_addr_valid(digest)) + verif_bundle_len += sig->digest_size; + } + + /* key points to a buffer which could contain the sig and digest too. */ + key = kmalloc(verif_bundle_len, GFP_KERNEL); if (!key) goto error_free_req; @@ -424,9 +438,24 @@ int public_key_verify_signature(const struct public_key *pkey, goto error_free_key; } + if (IS_ENABLED(CONFIG_VMAP_STACK)) { + ptr += pkey->paramlen; + + if (!virt_addr_valid(sig_s)) { + sig_s = ptr; + memcpy(sig_s, sig->s, sig->s_size); + ptr += sig->s_size; + } + + if (!virt_addr_valid(digest)) { + digest = ptr; + memcpy(digest, sig->digest, sig->digest_size); + } + } + sg_init_table(src_sg, 2); - sg_set_buf(&src_sg[0], sig->s, sig->s_size); - sg_set_buf(&src_sg[1], sig->digest, sig->digest_size); + sg_set_buf(&src_sg[0], sig_s, sig->s_size); + sg_set_buf(&src_sg[1], digest, sig->digest_size); akcipher_request_set_crypt(req, src_sg, NULL, sig->s_size, sig->digest_size); crypto_init_wait(&cwait);