Message ID | 20230203084302.589017-1-zyytlz.wz@163.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:adf:eb09:0:0:0:0:0 with SMTP id s9csp722129wrn; Fri, 3 Feb 2023 00:58:13 -0800 (PST) X-Google-Smtp-Source: AK7set/7c7nDvMB3T+0UFK3dfRcuKnloiyHUbDCQRYvfanaAuiWmtVEwhsPRSa5fKfVlDGkq44OI X-Received: by 2002:a05:6402:54b:b0:499:b4ce:b26e with SMTP id i11-20020a056402054b00b00499b4ceb26emr10533580edx.40.1675414693372; Fri, 03 Feb 2023 00:58:13 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1675414693; cv=none; d=google.com; s=arc-20160816; b=VwFtKleXW2fAVAPPFCPq0AHRzbq8j02UH6pUuJK7+wCYNjA/Iii9lV8J67JoX8cVao IoFKPXL0TpOd3tbkoowR24H9j48gUG4QpfYDUPTQb11g9Hc63c/0CkVYkQyV8tpE4oMz 5FkgbWUBetRteAPx9XhmmK/TENIO6S3vDG9qR5LQbi7+f1l+jwytIjjn6EMN6mb2u7oU xEYSp9qecwocoGYjQmzDeS6GLQXKNfGPzgBqazahoXgMpOrigXgPQ03XrdxMt7C9UeLf kbVZJQ1wFbSTo5EOSsCDGzWG5g3WEQDSaWEnRRIXvgEpyVkPx2S3cV2ywO3xcc56ow4i X2TQ== 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:dkim-signature; bh=2/KzqscKzGfSW5bIOswCBQ7pZPgpX1NYp6RGAiZBDks=; b=cDxiD5jJCdhL6v5FI8ygHAkSaAm3tv5O9ewSJ2lwtm1mh+ML8iQSMonDT2Cd/XQpdT rCeSdmRalU9sMN5Wo/odooHhrwdI+CYmye0SMWf+G7jk+ExvS6xElQ9/NzB68727Tge0 oPcxYxWjAiFC0qtvtLDZQaYWpKOfCHdbMGzWa59K39GyhUO4v4YYXMxZ0R76Pwfr5efV r0XAayOSIQamV7WGHxBgnM2pknvnG8Ajg/fK1F/pX1PkUzOGNy2PJ9EIhi4gbpDvJ+Nf Lph1Olrzq7fv38iJPs+q17i3n/IdJAhtQLOB7zYiYveZHNrETUUeG8SHlER6bXCWNkQS VdqQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@163.com header.s=s110527 header.b=j3Hfi+TF; 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; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=163.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id er18-20020a056402449200b0046a0331778dsi2261980edb.118.2023.02.03.00.57.49; Fri, 03 Feb 2023 00:58:13 -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; dkim=pass header.i=@163.com header.s=s110527 header.b=j3Hfi+TF; 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; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=163.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231871AbjBCInM (ORCPT <rfc822;il.mystafa@gmail.com> + 99 others); Fri, 3 Feb 2023 03:43:12 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:46982 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231302AbjBCInK (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Fri, 3 Feb 2023 03:43:10 -0500 Received: from m12.mail.163.com (m12.mail.163.com [220.181.12.215]) by lindbergh.monkeyblade.net (Postfix) with ESMTP id 1F5EC126E6 for <linux-kernel@vger.kernel.org>; Fri, 3 Feb 2023 00:43:07 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=163.com; s=s110527; h=From:Subject:Date:Message-Id:MIME-Version; bh=2/Kzq scKzGfSW5bIOswCBQ7pZPgpX1NYp6RGAiZBDks=; b=j3Hfi+TFwZ1Vomk6cshx8 i50+Jcq9XTnwEZP7eB2CbcSeOHxVGkYfpbNt8uLhlr8RTP9PTCk73OyDKGy55DBN 6+RJZWrQE5EifU7seUaDPHL1WQnhvs+wxUpuiSnDoAYc/FEocoRsf7fkMCJ6Zh+p Q3a2psgcMI8yzOIlcc/dC0= Received: from leanderwang-LC2.localdomain (unknown [111.206.145.21]) by zwqz-smtp-mta-g2-3 (Coremail) with SMTP id _____wCHiLIXydxjUofKCg--.60360S2; Fri, 03 Feb 2023 16:43:03 +0800 (CST) From: Zheng Wang <zyytlz.wz@163.com> To: tianjia.zhang@linux.alibaba.com Cc: linux-kernel@vger.kernel.org, hackerzheng666@gmail.com, alex000young@gmail.com, Zheng Wang <zyytlz.wz@163.com> Subject: [PATCH] lib/mpi: Fix poential NULL pointer dereference in mpi_fdiv_q Date: Fri, 3 Feb 2023 16:43:02 +0800 Message-Id: <20230203084302.589017-1-zyytlz.wz@163.com> X-Mailer: git-send-email 2.25.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-CM-TRANSID: _____wCHiLIXydxjUofKCg--.60360S2 X-Coremail-Antispam: 1Uf129KBjvdXoWrKrW5Zw1DZry8ur15Gr4xtFb_yoWkurbEkF yIqr1UAr1Uur109w15CF40gr4DA3Z8ur42kF42gw12grWDWFZ3WFWqg393tFZrCF4ak3yU Cr1rZrW5Xw42vjkaLaAFLSUrUUUUUb8apTn2vfkv8UJUUUU8Yxn0WfASr-VFAUDa7-sFnT 9fnUUvcSsGvfC2KfnxnUUI43ZEXa7sRtl1kDUUUUU== X-Originating-IP: [111.206.145.21] X-CM-SenderInfo: h2113zf2oz6qqrwthudrp/1tbiXBgLU1Xl5gbdbgAAs- X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,FREEMAIL_FROM,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?1756799637516125969?= X-GMAIL-MSGID: =?utf-8?q?1756799637516125969?= |
Series |
lib/mpi: Fix poential NULL pointer dereference in mpi_fdiv_q
|
|
Commit Message
Zheng Wang
Feb. 3, 2023, 8:43 a.m. UTC
in lib/mpi, there is multiple function that not check the return
value of mpi_alloc. One case is mpi_fdiv_q, if tmp == NULL,
tmp->nlimbs in mpi_fdiv_qr will cause NULL pointer dereference.
As the code is too much, here I only fix one of them. Other
function like mpi_barrett_init mpi_copy mpi_alloc_like mpi_set
mpi_set_ui mpi_alloc_set_ui has the same problem.
Please let me know if there is a better way to fix the
problem.
Note that, as a bug found by static analysis, it can be a false
positive or hard to trigger.
Fixes: a8ea8bdd9df9 ("lib/mpi: Extend the MPI library")
Signed-off-by: Zheng Wang <zyytlz.wz@163.com>
---
lib/mpi/mpi-div.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Comments
Hi Zheng Wang, On 2/3/23 4:43 PM, Zheng Wang wrote: > in lib/mpi, there is multiple function that not check the return > value of mpi_alloc. One case is mpi_fdiv_q, if tmp == NULL, > tmp->nlimbs in mpi_fdiv_qr will cause NULL pointer dereference. > As the code is too much, here I only fix one of them. Other > function like mpi_barrett_init mpi_copy mpi_alloc_like mpi_set > mpi_set_ui mpi_alloc_set_ui has the same problem. > > Please let me know if there is a better way to fix the > problem. > > Note that, as a bug found by static analysis, it can be a false > positive or hard to trigger. Thanks for your patch, lib/mpi is ported from libgcrypt, as an application layer library, such error handling is no problem, but for the kernel, these return values should be checked, even if these errors are difficult to trigger. As you said, there are many places where this check is ignored, and even the error return value needs to be passed upwards, which should be upgraded and fixed uniformly, even some callers need such a check, such as the SM2 algorithm. These checks were initially ignored in the interest of code brevity and rapid development, and it looks like it's time to fix them. > > Fixes: a8ea8bdd9df9 ("lib/mpi: Extend the MPI library") > Signed-off-by: Zheng Wang <zyytlz.wz@163.com> > --- > lib/mpi/mpi-div.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/lib/mpi/mpi-div.c b/lib/mpi/mpi-div.c > index 45beab8b9e9e..e8642265d7d4 100644 > --- a/lib/mpi/mpi-div.c > +++ b/lib/mpi/mpi-div.c > @@ -43,7 +43,8 @@ void mpi_fdiv_r(MPI rem, MPI dividend, MPI divisor) > void mpi_fdiv_q(MPI quot, MPI dividend, MPI divisor) > { > MPI tmp = mpi_alloc(mpi_get_nlimbs(quot)); > - mpi_fdiv_qr(quot, tmp, dividend, divisor); > + if (tmp) > + mpi_fdiv_qr(quot, tmp, dividend, divisor); > mpi_free(tmp); > } > As far as this is concerned, the allocation failure should pass the ENOMEM error upwards, not the void type. Best regards, Tianjia
At 2023-02-07 09:41:35, "Tianjia Zhang" <tianjia.zhang@linux.alibaba.com> wrote: >Hi Zheng Wang, > >On 2/3/23 4:43 PM, Zheng Wang wrote: >> in lib/mpi, there is multiple function that not check the return >> value of mpi_alloc. One case is mpi_fdiv_q, if tmp == NULL, >> tmp->nlimbs in mpi_fdiv_qr will cause NULL pointer dereference. >> As the code is too much, here I only fix one of them. Other >> function like mpi_barrett_init mpi_copy mpi_alloc_like mpi_set >> mpi_set_ui mpi_alloc_set_ui has the same problem. >> >> Please let me know if there is a better way to fix the >> problem. >> >> Note that, as a bug found by static analysis, it can be a false >> positive or hard to trigger. > >Thanks for your patch, lib/mpi is ported from libgcrypt, as an >application layer library, such error handling is no problem, but for >the kernel, these return values should be checked, even if these errors >are difficult to trigger. > >As you said, there are many places where this check is ignored, and even >the error return value needs to be passed upwards, which should be >upgraded and fixed uniformly, even some callers need such a check, such >as the SM2 algorithm. These checks were initially ignored in the >interest of code brevity and rapid development, and it looks like it's >time to fix them. > Thank you for taking the time to consider my question. Hope it can be fixed quickly. >> >> Fixes: a8ea8bdd9df9 ("lib/mpi: Extend the MPI library") >> Signed-off-by: Zheng Wang <zyytlz.wz@163.com> >> --- >> lib/mpi/mpi-div.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) >> >> diff --git a/lib/mpi/mpi-div.c b/lib/mpi/mpi-div.c >> index 45beab8b9e9e..e8642265d7d4 100644 >> --- a/lib/mpi/mpi-div.c >> +++ b/lib/mpi/mpi-div.c >> @@ -43,7 +43,8 @@ void mpi_fdiv_r(MPI rem, MPI dividend, MPI divisor) >> void mpi_fdiv_q(MPI quot, MPI dividend, MPI divisor) >> { >> MPI tmp = mpi_alloc(mpi_get_nlimbs(quot)); >> - mpi_fdiv_qr(quot, tmp, dividend, divisor); >> + if (tmp) >> + mpi_fdiv_qr(quot, tmp, dividend, divisor); >> mpi_free(tmp); >> } >> > >As far as this is concerned, the allocation failure should pass the >ENOMEM error upwards, not the void type. Yes, I think the good way is to refactor the prototype of function, making it available to receiver error code and pass it to the caller function, until there is an error handler to handle it. Best regards, Zheng Wang
diff --git a/lib/mpi/mpi-div.c b/lib/mpi/mpi-div.c index 45beab8b9e9e..e8642265d7d4 100644 --- a/lib/mpi/mpi-div.c +++ b/lib/mpi/mpi-div.c @@ -43,7 +43,8 @@ void mpi_fdiv_r(MPI rem, MPI dividend, MPI divisor) void mpi_fdiv_q(MPI quot, MPI dividend, MPI divisor) { MPI tmp = mpi_alloc(mpi_get_nlimbs(quot)); - mpi_fdiv_qr(quot, tmp, dividend, divisor); + if (tmp) + mpi_fdiv_qr(quot, tmp, dividend, divisor); mpi_free(tmp); }