Bug 266548 - malicious gpt can cause kernel page fault during tasting
Summary: malicious gpt can cause kernel page fault during tasting
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: Mark Johnston
URL:
Keywords: crash
Depends on:
Blocks:
 
Reported: 2022-09-22 15:11 UTC by Robert Morris
Modified: 2023-09-06 05:04 UTC (History)
4 users (show)

See Also:
zlei: mfc-stable13+
zlei: mfc-stable12+


Attachments
a disk image that causes a crash in the GPT taste code (8.00 KB, application/octet-stream)
2022-09-22 15:11 UTC, Robert Morris
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Robert Morris 2022-09-22 15:11:00 UTC
Created attachment 236747 [details]
a disk image that causes a crash in the GPT taste code

I've attached a disk image that, if it's on a USB thumb drive and
inserted (but not mounted), will cause a kernel page fault (or
assertion failure for an INVARIANTS kernel).

This code in gpt_read_tbl():

        tbl = g_malloc(hdr->hdr_entries * sizeof(struct gpt_ent),
            M_WAITOK | M_ZERO);

The disk image contains the crazy value hdr_entries=4294967295;
g_malloc()'s size argument is a 32-bit signed int, and the result is
to call g_malloc(-128).

Here's a backtrace from a CURRENT amd64 machine with INVARIANTS:

# uname -a
FreeBSD stock14 14.0-CURRENT FreeBSD 14.0-CURRENT #3 main-n258027-c9baa974717a: Thu Sep 15 20:02:51 AST 2022     root@stock14:/usr/obj/usr/src/amd64.amd64/sys/GENERIC amd64
# mdconfig -f taste18a.img
panic: Assertion size > 0 failed at /usr/src/sys/kern/subr_vmem.c:1332
cpuid = 0
time = 1663856658
KDB: stack backtrace:
db_trace_self_wrapper() at db_trace_self_wrapper+0x2b/frame 0xfffffe0063aa0c00
vpanic() at vpanic+0x151/frame 0xfffffe0063aa0c50
panic() at panic+0x43/frame 0xfffffe0063aa0cb0
vmem_alloc() at vmem_alloc+0xf1/frame 0xfffffe0063aa0ce0
kmem_malloc_domainset() at kmem_malloc_domainset+0x92/frame 0xfffffe0063aa0d50
malloc_large() at malloc_large+0x2f/frame 0xfffffe0063aa0d80
gpt_read_tbl() at gpt_read_tbl+0x25f/frame 0xfffffe0063aa0e00
g_part_gpt_read() at g_part_gpt_read+0xfb/frame 0xfffffe0063aa0e60
g_part_taste() at g_part_taste+0x172/frame 0xfffffe0063aa0ea0
g_new_provider_event() at g_new_provider_event+0x9a/frame 0xfffffe0063aa0ec0
g_run_events() at g_run_events+0x104/frame 0xfffffe0063aa0ef0
fork_exit() at fork_exit+0x80/frame 0xfffffe0063aa0f30
fork_trampoline() at fork_trampoline+0xe/frame 0xfffffe0063aa0f30

Without INVARIANTS, the kernel gets a bit farther before a page fault.
Comment 1 Zhenlei Huang freebsd_committer freebsd_triage 2022-09-23 08:34:28 UTC
I'm not familiar with GPT GEOM, but since hdr->hdr_entries is from untrusted source then definitely hdr->hdr_entries should be checked firstly before it being passed to g_malloc(int size, int flags) .
Comment 2 Robert Morris 2022-09-27 17:12:44 UTC
hdr_entsz ought also to be checked, due to the "p += hdr->hdr_entsz"
farther along in gpt_read_tbl().
Comment 3 Zhenlei Huang freebsd_committer freebsd_triage 2022-09-28 02:00:05 UTC
Some related differential that try to resolve this issue:
1. https://reviews.freebsd.org/D36709
2. https://reviews.freebsd.org/D36717

Suggestions are welcomed :)
Comment 4 commit-hook freebsd_committer freebsd_triage 2022-10-18 15:20:35 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=5be5d0d5cb2657d7668f4ca0f8543198cf8d759b

commit 5be5d0d5cb2657d7668f4ca0f8543198cf8d759b
Author:     Zhenlei Huang <zlei.huang@gmail.com>
AuthorDate: 2022-10-18 15:03:02 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2022-10-18 15:03:02 +0000

    geom_part: Check number of GPT entries and size of GPT entry

    Current specification does not have upper limit of the number of
    partition entries and the size of partition entry. In
    799eac8c3df597179bbb3b078362f3ff03993a1a Andrey V. Elsukov introduced a
    limit maximum number of GPT entries to 4k, but that is for write routine
    (gpart create) only. When attaching disks that have large number of GPT
    entries exceeding the limit, or disks with large size of partition
    entry, it is still possible to exhaust kernel memory.

    1. Reuse the limit of the maximum number of partition entries.
    2. Limit the maximum size of GPT entry to 1k.

    In current specification (2.10) the size of GPT entry is 128 *
    2^n while n >= 0, and the size - 128 is reserved. 1k should be
    sufficient enough for foreseen future.

    PR:             266548
    Discussed with: imp
    Reviewed by:    markj
    MFC after:      1 month
    Differential Revision:  https://reviews.freebsd.org/D36717

 sys/geom/part/g_part_gpt.c | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)
Comment 5 Kubilay Kocak freebsd_committer freebsd_triage 2022-10-18 22:29:53 UTC
^Triage: Assign to committer resolving
Comment 6 commit-hook freebsd_committer freebsd_triage 2022-11-21 13:53:48 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=e8c769b22d42fd66d0916e2a04af32ad9d306db1

commit e8c769b22d42fd66d0916e2a04af32ad9d306db1
Author:     Zhenlei Huang <zlei.huang@gmail.com>
AuthorDate: 2022-10-18 15:03:02 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2022-11-21 13:49:25 +0000

    geom_part: Check number of GPT entries and size of GPT entry

    Current specification does not have upper limit of the number of
    partition entries and the size of partition entry. In
    799eac8c3df597179bbb3b078362f3ff03993a1a Andrey V. Elsukov introduced a
    limit maximum number of GPT entries to 4k, but that is for write routine
    (gpart create) only. When attaching disks that have large number of GPT
    entries exceeding the limit, or disks with large size of partition
    entry, it is still possible to exhaust kernel memory.

    1. Reuse the limit of the maximum number of partition entries.
    2. Limit the maximum size of GPT entry to 1k.

    In current specification (2.10) the size of GPT entry is 128 *
    2^n while n >= 0, and the size - 128 is reserved. 1k should be
    sufficient enough for foreseen future.

    PR:             266548
    Discussed with: imp
    Reviewed by:    markj
    MFC after:      1 month
    Differential Revision:  https://reviews.freebsd.org/D36717

    (cherry picked from commit 5be5d0d5cb2657d7668f4ca0f8543198cf8d759b)

 sys/geom/part/g_part_gpt.c | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)
Comment 7 commit-hook freebsd_committer freebsd_triage 2022-12-21 01:18:38 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=2e543af13ab3746c7626c53293c007c8747eff9d

commit 2e543af13ab3746c7626c53293c007c8747eff9d
Author:     Zhenlei Huang <zlei@FreeBSD.org>
AuthorDate: 2022-12-21 01:04:30 +0000
Commit:     Zhenlei Huang <zlei@FreeBSD.org>
CommitDate: 2022-12-21 01:04:30 +0000

    geom_part: Fix potential integer overflow when checking size of the table

    `hdr_entries` and `hdr_entsz` are both uint32_t as defined in UEFI spec.
    Current spec does not have upper limit of the number of partition
    entries and the size of partition entry, it is potential that malicious
    or corrupted GPT header read from untrusted source contains large size of
    entry number or size.

    PR:             266548
    Reviewed by:    oshogbo, cem, imp, markj
    Approved by:    kp (mentor)
    MFC after:      2 weeks
    Differential Revision:  https://reviews.freebsd.org/D36709

 sys/geom/part/g_part_gpt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Comment 8 commit-hook freebsd_committer freebsd_triage 2023-01-11 10:40:25 UTC
A commit in branch stable/13 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=3070bedd3dc54196f48645966eb34bd3a9bf131d

commit 3070bedd3dc54196f48645966eb34bd3a9bf131d
Author:     Zhenlei Huang <zlei@FreeBSD.org>
AuthorDate: 2022-12-21 01:04:30 +0000
Commit:     Zhenlei Huang <zlei@FreeBSD.org>
CommitDate: 2023-01-11 10:35:59 +0000

    geom_part: Fix potential integer overflow when checking size of the table

    `hdr_entries` and `hdr_entsz` are both uint32_t as defined in UEFI spec.
    Current spec does not have upper limit of the number of partition
    entries and the size of partition entry, it is potential that malicious
    or corrupted GPT header read from untrusted source contains large size of
    entry number or size.

    PR:             266548
    Reviewed by:    oshogbo, cem, imp, markj
    Approved by:    kp (mentor)
    MFC after:      2 weeks
    Differential Revision:  https://reviews.freebsd.org/D36709

    (cherry picked from commit 2e543af13ab3746c7626c53293c007c8747eff9d)

 sys/geom/part/g_part_gpt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Comment 9 commit-hook freebsd_committer freebsd_triage 2023-09-06 04:53:29 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=61b426a0b6a822c6281016a23f28443bb26b891f

commit 61b426a0b6a822c6281016a23f28443bb26b891f
Author:     Zhenlei Huang <zlei.huang@gmail.com>
AuthorDate: 2022-10-18 15:03:02 +0000
Commit:     Zhenlei Huang <zlei@FreeBSD.org>
CommitDate: 2023-09-06 04:32:56 +0000

    geom_part: Check number of GPT entries and size of GPT entry

    Current specification does not have upper limit of the number of
    partition entries and the size of partition entry. In
    799eac8c3df597179bbb3b078362f3ff03993a1a Andrey V. Elsukov introduced a
    limit maximum number of GPT entries to 4k, but that is for write routine
    (gpart create) only. When attaching disks that have large number of GPT
    entries exceeding the limit, or disks with large size of partition
    entry, it is still possible to exhaust kernel memory.

    1. Reuse the limit of the maximum number of partition entries.
    2. Limit the maximum size of GPT entry to 1k.

    In current specification (2.10) the size of GPT entry is 128 *
    2^n while n >= 0, and the size - 128 is reserved. 1k should be
    sufficient enough for foreseen future.

    PR:             266548
    Discussed with: imp
    Reviewed by:    markj
    MFC after:      1 month
    Differential Revision:  https://reviews.freebsd.org/D36717

    (cherry picked from commit 5be5d0d5cb2657d7668f4ca0f8543198cf8d759b)
    (cherry picked from commit e8c769b22d42fd66d0916e2a04af32ad9d306db1)

 sys/geom/part/g_part_gpt.c | 31 +++++++++++++++++++++++++++----
 1 file changed, 27 insertions(+), 4 deletions(-)
Comment 10 commit-hook freebsd_committer freebsd_triage 2023-09-06 04:53:31 UTC
A commit in branch stable/12 references this bug:

URL: https://cgit.FreeBSD.org/src/commit/?id=8fdb1181ab8d28cbf62b1917b602028e34c8c9cc

commit 8fdb1181ab8d28cbf62b1917b602028e34c8c9cc
Author:     Zhenlei Huang <zlei@FreeBSD.org>
AuthorDate: 2022-12-21 01:04:30 +0000
Commit:     Zhenlei Huang <zlei@FreeBSD.org>
CommitDate: 2023-09-06 04:32:56 +0000

    geom_part: Fix potential integer overflow when checking size of the table

    `hdr_entries` and `hdr_entsz` are both uint32_t as defined in UEFI spec.
    Current spec does not have upper limit of the number of partition
    entries and the size of partition entry, it is potential that malicious
    or corrupted GPT header read from untrusted source contains large size of
    entry number or size.

    PR:             266548
    Reviewed by:    oshogbo, cem, imp, markj
    Approved by:    kp (mentor)
    MFC after:      2 weeks
    Differential Revision:  https://reviews.freebsd.org/D36709

    (cherry picked from commit 2e543af13ab3746c7626c53293c007c8747eff9d)
    (cherry picked from commit 3070bedd3dc54196f48645966eb34bd3a9bf131d)

 sys/geom/part/g_part_gpt.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Comment 11 Zhenlei Huang freebsd_committer freebsd_triage 2023-09-06 05:04:39 UTC
Also MFCed the fix to stable/12 .