Message ID | Y6Kthn+rIUnCEJWz@gondor.apana.org.au |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:e747:0:0:0:0:0 with SMTP id c7csp3380012wrn; Tue, 20 Dec 2022 23:13:12 -0800 (PST) X-Google-Smtp-Source: AMrXdXuekz7XhsIeVgAlAmAxNfzh6zGj4rGoi+krWZjFAWfAKch5inhgEQQoBfs0xFW6FrmMhoBJ X-Received: by 2002:a17:906:aed6:b0:7af:170f:512 with SMTP id me22-20020a170906aed600b007af170f0512mr476997ejb.38.1671606792281; Tue, 20 Dec 2022 23:13:12 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1671606792; cv=none; d=google.com; s=arc-20160816; b=kNWJ4QZOnkYUkkf8LL2r54YUAnR6aRPVRmKIL3cVTfWfkxZXwlPsMGjIzXUDgMVwGf P/tbN5uxXkLqTS0hrDVfaRMS0nqNm9UgHzM8FUHr0qa9enZe/CeE7/sXCW1SgrO310j9 J1LL+58wklpWToE8fcCijlBTxfi9kB82PtMxqcl9BmkmGLWdy4yr6qfLImOGH2w0TZ8b nW+1zSGiFQpa63usXNCw1SgAPMMq9kTd3bZBO9Bm1bMNnnJX8m0JmGCi7UUME6HCnQY/ pFxPrsnabR9wLz81+L+3PyE8aaPTdkk9BQflyjEScCGDLUv4XexOjQGXSG8UzLxy1K4J 8vSQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:in-reply-to:content-disposition:mime-version :references:message-id:subject:cc:to:from:date; bh=tXBUWCJSW8vq3pLC3iRsSzqtTuHxkwSpE1wcwXb1XUA=; b=YeSfn+njFKgcCcl9NIih6c2PtTMcrdAXuDgizcQuYKDB0rHqwkC7XJraN21MxubMF4 rYs9iyGSf9pSDYiRJn1vPTMqf1j1PXuljaE9A6nS+Hg5ZQRlLbu+ybWK+sjRqMrDgTi6 V1VikCZaqMLUJSUSrDg8le/vILSWvQfE5eKCBFJ3uRbnFAe7T9EN279A2GX3PKG0iRkI 6Hcjzo1K7PCvQY5E5VCCCu2wReLFeLDNL2/zdYT6cKUGRzxRzb2sQ5Xg8jT6G1axT/yA h2JVYFYxjKB3Cfbnct87uIs4qH+/LEQjm/aALsLR4hXevMHRkxZq7+9eNGWqYqC4tAO1 oIzg== 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 ga41-20020a1709070c2900b0078e1d1d6005si2974586ejc.23.2022.12.20.23.12.48; Tue, 20 Dec 2022 23:13:11 -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 S234100AbiLUGyf (ORCPT <rfc822;pacteraone@gmail.com> + 99 others); Wed, 21 Dec 2022 01:54:35 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:49796 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234371AbiLUGy3 (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 21 Dec 2022 01:54:29 -0500 Received: from formenos.hmeau.com (helcar.hmeau.com [216.24.177.18]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id CA2C51F2F4; Tue, 20 Dec 2022 22:54:27 -0800 (PST) Received: from loth.rohan.me.apana.org.au ([192.168.167.2]) by formenos.hmeau.com with smtp (Exim 4.94.2 #2 (Debian)) id 1p7szG-00989G-S1; Wed, 21 Dec 2022 14:53:59 +0800 Received: by loth.rohan.me.apana.org.au (sSMTP sendmail emulation); Wed, 21 Dec 2022 14:53:58 +0800 Date: Wed, 21 Dec 2022 14:53:58 +0800 From: Herbert Xu <herbert@gondor.apana.org.au> To: Eric Biggers <ebiggers@kernel.org> Cc: Roberto Sassu <roberto.sassu@huaweicloud.com>, dhowells@redhat.com, davem@davemloft.net, zohar@linux.ibm.com, dmitry.kasatkin@gmail.com, paul@paul-moore.com, jmorris@namei.org, serge@hallyn.com, 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>, Tadeusz Struk <tadeusz.struk@intel.com> Subject: [v2 PATCH] lib/mpi: Fix buffer overrun when SG is too long Message-ID: <Y6Kthn+rIUnCEJWz@gondor.apana.org.au> References: <20221209150633.1033556-1-roberto.sassu@huaweicloud.com> <Y5OGr59A9wo86rYY@sol.localdomain> <fa8a307541735ec9258353d8ccb75c20bb22aafe.camel@huaweicloud.com> <Y5bxJ5UZNPzxwtoy@gondor.apana.org.au> <0f80852578436dbba7a0fce03d86c3fa2d38c571.camel@huaweicloud.com> <Y6FjQPZiJYTEG1zI@gondor.apana.org.au> <a04e6458-6814-97fc-f03a-617809e2e6ce@huaweicloud.com> <Y6IbWA5aZeBnn4n2@gmail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <Y6IbWA5aZeBnn4n2@gmail.com> 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?1752717102517925974?= X-GMAIL-MSGID: =?utf-8?q?1752806763888798488?= |
Series |
[v2] lib/mpi: Fix buffer overrun when SG is too long
|
|
Commit Message
Herbert Xu
Dec. 21, 2022, 6:53 a.m. UTC
On Tue, Dec 20, 2022 at 08:30:16PM +0000, Eric Biggers wrote: > > > Tried, could not boot the UML kernel. > > > > After looking, it seems we have to call sg_miter_stop(). Or alternatively, > > we could let sg_miter_next() be called but not writing anything inside the > > loop. > > > > With either of those fixes, the tests pass (using one scatterlist). Thanks for the quick feedback Roberto! > I think it should look like: > > while (nbytes) { > sg_miter_next(&miter); > ... > } > sg_miter_stop(&miter); You're right Eric. However, we could also do it by simply not checking nbytes since we already set nents according to nbytes at the top of the function. ---8<--- The helper mpi_read_raw_from_sgl sets the number of entries in the SG list according to nbytes. However, if the last entry in the SG list contains more data than nbytes, then it may overrun the buffer because it only allocates enough memory for nbytes. Fixes: 2d4d1eea540b ("lib/mpi: Add mpi sgl helpers") Reported-by: Roberto Sassu <roberto.sassu@huaweicloud.com> Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>
Comments
On Wed, Dec 21, 2022 at 02:53:58PM +0800, Herbert Xu wrote: > On Tue, Dec 20, 2022 at 08:30:16PM +0000, Eric Biggers wrote: > > > > > Tried, could not boot the UML kernel. > > > > > > After looking, it seems we have to call sg_miter_stop(). Or alternatively, > > > we could let sg_miter_next() be called but not writing anything inside the > > > loop. > > > > > > With either of those fixes, the tests pass (using one scatterlist). > > Thanks for the quick feedback Roberto! > > > I think it should look like: > > > > while (nbytes) { > > sg_miter_next(&miter); > > ... > > } > > sg_miter_stop(&miter); > > You're right Eric. However, we could also do it by simply not > checking nbytes since we already set nents according to nbytes > at the top of the function. > > ---8<--- > The helper mpi_read_raw_from_sgl sets the number of entries in > the SG list according to nbytes. However, if the last entry > in the SG list contains more data than nbytes, then it may overrun > the buffer because it only allocates enough memory for nbytes. > > Fixes: 2d4d1eea540b ("lib/mpi: Add mpi sgl helpers") > Reported-by: Roberto Sassu <roberto.sassu@huaweicloud.com> > Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au> > > diff --git a/lib/mpi/mpicoder.c b/lib/mpi/mpicoder.c > index 39c4c6731094..157ef532a6a2 100644 > --- a/lib/mpi/mpicoder.c > +++ b/lib/mpi/mpicoder.c > @@ -504,7 +501,8 @@ MPI mpi_read_raw_from_sgl(struct scatterlist *sgl, unsigned int nbytes) > > while (sg_miter_next(&miter)) { > buff = miter.addr; > - len = miter.length; > + len = min_t(unsigned, miter.length, nbytes); > + nbytes -= len; > > for (x = 0; x < len; x++) { > a <<= 8; That's fine, I guess. One quirk of the above approach is that if the last needed element of the scatterlist has a lot of extra pages, this will iterate through all those extra pages, processing 0 bytes from each. It could just stop when done. I suppose it's not worth worrying about that case, though. - Eric
On Wed, Dec 21, 2022 at 12:53:29PM -0800, Eric Biggers wrote: > > That's fine, I guess. One quirk of the above approach is that if the last > needed element of the scatterlist has a lot of extra pages, this will iterate > through all those extra pages, processing 0 bytes from each. It could just stop > when done. I suppose it's not worth worrying about that case, though. Ideally this should be handled in the sg_miter interface, IOW, it should allow us to cap the SG list at a certain number of bytes as opposed to a certain number of entries. Cheers,
diff --git a/lib/mpi/mpicoder.c b/lib/mpi/mpicoder.c index 39c4c6731094..157ef532a6a2 100644 --- a/lib/mpi/mpicoder.c +++ b/lib/mpi/mpicoder.c @@ -504,7 +501,8 @@ MPI mpi_read_raw_from_sgl(struct scatterlist *sgl, unsigned int nbytes) while (sg_miter_next(&miter)) { buff = miter.addr; - len = miter.length; + len = min_t(unsigned, miter.length, nbytes); + nbytes -= len; for (x = 0; x < len; x++) { a <<= 8;