All of lore.kernel.org
 help / color / mirror / Atom feed
From: David Howells <dhowells@redhat.com>
To: Andrey Ryabinin <ryabinin.a.a@gmail.com>
Cc: dhowells@redhat.com, jmorris@namei.org,
	Dmitry Kasatkin <dmitry.kasatkin@gmail.com>,
	LKML <linux-kernel@vger.kernel.org>,
	stable@vger.kernel.org, linux-security-module@vger.kernel.org,
	keyrings@vger.kernel.org, linux-ima-devel@lists.sourceforge.net
Subject: Re: [PATCH 2/2] MPI: Fix mpi_powm() when exponent is 0 and the result has no limbs
Date: Thu, 24 Nov 2016 12:56:09 +0000	[thread overview]
Message-ID: <8467.1479992169@warthog.procyon.org.uk> (raw)
In-Reply-To: <CAPAsAGy55fvBMsf8AUn39bb_qTyVKOe5uvEW+1twAqL0F3Zndw@mail.gmail.com>

I've integrated my patch and yours (see attached) - are you okay with the
result?

> We could even remove RESIZE_IF_NEEDED() to not confuse people, because
> currently it has no users.

Yep, but that'll have to be a separate patch.

David
---
commit cfe384ff418f11db2a3647f79635393ecb723c6f
Author: Andrey Ryabinin <aryabinin@virtuozzo.com>
Date:   Wed Nov 23 16:44:47 2016 +0000

    mpi: Fix NULL ptr dereference in mpi_powm()
    
    This fixes CVE-2016-8650.
    
    If mpi_powm() is given a zero exponent, it wants to immediately return
    either 1 or 0, depending on the modulus.  However, if the result was
    initalised with zero limb space, no limbs space is allocated and a
    NULL-pointer exception ensues.
    
    Fix this by allocating a minimal amount of limb space for the result when
    the 0-exponent case when the result is 1 and not touching the limb space
    when the result is 0.
    
    This affects the use of RSA keys and X.509 certificates that carry them.
    
    BUG: unable to handle kernel NULL pointer dereference at           (null)
    IP: [<ffffffff8138ce5d>] mpi_powm+0x32/0x7e6
    PGD 0
    Oops: 0002 [#1] SMP
    Modules linked in:
    CPU: 3 PID: 3014 Comm: keyctl Not tainted 4.9.0-rc6-fscache+ #278
    Hardware name: ASUS All Series/H97-PLUS, BIOS 2306 10/09/2014
    task: ffff8804011944c0 task.stack: ffff880401294000
    RIP: 0010:[<ffffffff8138ce5d>]  [<ffffffff8138ce5d>] mpi_powm+0x32/0x7e6
    RSP: 0018:ffff880401297ad8  EFLAGS: 00010212
    RAX: 0000000000000000 RBX: ffff88040868bec0 RCX: ffff88040868bba0
    RDX: ffff88040868b260 RSI: ffff88040868bec0 RDI: ffff88040868bee0
    RBP: ffff880401297ba8 R08: 0000000000000000 R09: 0000000000000000
    R10: 0000000000000047 R11: ffffffff8183b210 R12: 0000000000000000
    R13: ffff8804087c7600 R14: 000000000000001f R15: ffff880401297c50
    FS:  00007f7a7918c700(0000) GS:ffff88041fb80000(0000) knlGS:0000000000000000
    CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
    CR2: 0000000000000000 CR3: 0000000401250000 CR4: 00000000001406e0
    Stack:
     ffff88040868bec0 0000000000000020 ffff880401297b00 ffffffff81376cd4
     0000000000000100 ffff880401297b10 ffffffff81376d12 ffff880401297b30
     ffffffff81376f37 0000000000000100 0000000000000000 ffff880401297ba8
    Call Trace:
     [<ffffffff81376cd4>] ? __sg_page_iter_next+0x43/0x66
     [<ffffffff81376d12>] ? sg_miter_get_next_page+0x1b/0x5d
     [<ffffffff81376f37>] ? sg_miter_next+0x17/0xbd
     [<ffffffff8138ba3a>] ? mpi_read_raw_from_sgl+0xf2/0x146
     [<ffffffff8132a95c>] rsa_verify+0x9d/0xee
     [<ffffffff8132acca>] ? pkcs1pad_sg_set_buf+0x2e/0xbb
     [<ffffffff8132af40>] pkcs1pad_verify+0xc0/0xe1
     [<ffffffff8133cb5e>] public_key_verify_signature+0x1b0/0x228
     [<ffffffff8133d974>] x509_check_for_self_signed+0xa1/0xc4
     [<ffffffff8133cdde>] x509_cert_parse+0x167/0x1a1
     [<ffffffff8133d609>] x509_key_preparse+0x21/0x1a1
     [<ffffffff8133c3d7>] asymmetric_key_preparse+0x34/0x61
     [<ffffffff812fc9f3>] key_create_or_update+0x145/0x399
     [<ffffffff812fe227>] SyS_add_key+0x154/0x19e
     [<ffffffff81001c2b>] do_syscall_64+0x80/0x191
     [<ffffffff816825e4>] entry_SYSCALL64_slow_path+0x25/0x25
    Code: 56 41 55 41 54 53 48 81 ec a8 00 00 00 44 8b 71 04 8b 42 04 4c 8b 67 18 45 85 f6 89 45 80 0f 84 b4 06 00 00 85 c0 75 2f 41 ff ce <49> c7 04 24 01 00 00 00 b0 01 75 0b 48 8b 41 18 48 83 38 01 0f
    RIP  [<ffffffff8138ce5d>] mpi_powm+0x32/0x7e6
     RSP <ffff880401297ad8>
    CR2: 0000000000000000
    ---[ end trace d82015255d4a5d8d ]---
    
    Basically, this is a backport of a libgcrypt patch:
    
    	http://git.gnupg.org/cgi-bin/gitweb.cgi?p=libgcrypt.git;a=patch;h=6e1adb05d290aeeb1c230c763970695f4a538526
    
    Fixes: cdec9cb5167a ("crypto: GnuPG based MPI lib - source files (part 1)")
    Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
    Signed-off-by: David Howells <dhowells@redhat.com>
    cc: Dmitry Kasatkin <dmitry.kasatkin@gmail.com>
    cc: linux-ima-devel@lists.sourceforge.net
    cc: stable@vger.kernel.org

diff --git a/lib/mpi/mpi-pow.c b/lib/mpi/mpi-pow.c
index 5464c8744ea9..94c30138bd32 100644
--- a/lib/mpi/mpi-pow.c
+++ b/lib/mpi/mpi-pow.c
@@ -64,6 +64,9 @@ int mpi_powm(MPI res, MPI base, MPI exp, MPI mod)
 	if (!esize) {
 		/* Exponent is zero, result is 1 mod MOD, i.e., 1 or 0
 		 * depending on if MOD equals 1.  */
+		if (RESIZE_IF_NEEDED(res, 1) < 0)
+			return -ENOMEM;
+		rp = res->d;
 		rp[0] = 1;
 		res->nlimbs = (msize == 1 && mod->d[0] == 1) ? 0 : 1;
 		res->sign = 0;

  parent reply	other threads:[~2016-11-24 12:56 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-11-24  8:51 [PATCH 0/2] KEYS: Fixes David Howells
2016-11-24  8:51 ` [PATCH 1/2] X.509: Fix double free in x509_cert_parse() David Howells
2016-11-24  8:51 ` [PATCH 2/2] MPI: Fix mpi_powm() when exponent is 0 and the result has no limbs David Howells
2016-11-24 11:20   ` Andrey Ryabinin
2016-11-24 11:35   ` David Howells
2016-11-24 12:20     ` Andrey Ryabinin
2016-11-24 11:17 ` David Howells
2016-11-24 11:22   ` Andrey Ryabinin
2016-11-24 11:33   ` David Howells
2016-11-24 12:20     ` Andrey Ryabinin
2016-11-24 12:56     ` David Howells [this message]
2016-11-24 12:57     ` David Howells
2016-11-24 13:00       ` Andrey Ryabinin
2016-11-24 22:44 ` [PATCH 0/2] KEYS: Fixes James Morris
2016-11-24 23:16 ` David Howells
2016-11-24 23:27   ` James Morris
2016-11-24 23:39   ` David Howells
2016-11-25  1:33     ` James Morris

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8467.1479992169@warthog.procyon.org.uk \
    --to=dhowells@redhat.com \
    --cc=dmitry.kasatkin@gmail.com \
    --cc=jmorris@namei.org \
    --cc=keyrings@vger.kernel.org \
    --cc=linux-ima-devel@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=ryabinin.a.a@gmail.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.