numa: bound SRAT/SLIT parsing against untrusted ACPI table lengths - #1482
Open
gburd wants to merge 1 commit into
Open
numa: bound SRAT/SLIT parsing against untrusted ACPI table lengths#1482gburd wants to merge 1 commit into
gburd wants to merge 1 commit into
Conversation
parse_srat() and parse_slit() read ACPI tables supplied by firmware, which should be treated as untrusted input. The current walk has two out-of-bounds read exposures on a malformed or truncated table: - The SRAT subtable loop only checks that a subtable *starts* before the end of the table and that its Length is non-zero. A subtable whose Length runs past the table end (or a header shorter than the table struct itself) is still dereferenced, reading past the SRAT buffer. - parse_slit() validates LocalityCount against the SRAT node count but then reads n*n distance bytes from Entry without checking that those bytes fit within the table's declared Header.Length. A truncated SLIT (or one whose LocalityCount matches nr_nodes but whose body is short) reads out of bounds, and n*n itself can overflow. Harden both: - Walk SRAT with an explicit byte cursor and reject any subtable whose declared length is shorter than the subtable header or extends past the end of the table; also reject an SRAT header shorter than the table struct. Using a byte cursor also drops the reliance on non-standard void* pointer arithmetic (a GNU extension). - In parse_slit(), require that offsetof(Entry) + n*n fits within Header.Length and guard against n*n overflow before assigning the distance matrix. No functional change on well-formed firmware tables.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
core/numa.ccparses the ACPI SRAT and SLIT tables during NUMA topologydiscovery. Those tables come from firmware and should be treated as untrusted
input. Two out-of-bounds read exposures exist on a malformed or truncated table:
SRAT subtable walk only checks that a subtable starts before the end of
the table and that its
Lengthis non-zero. A subtable whoseLengthrunspast the table end (or an SRAT header shorter than the table struct itself) is
still dereferenced via
get_parent_from_member, reading past the SRAT buffer.parse_slit()validatesLocalityCountagainst the SRAT node count, butthen reads
n*ndistance bytes fromEntrywithout checking that those bytesfit within the table's declared
Header.Length. A truncated SLIT (or onewhose
LocalityCountmatchesnr_nodesbut whose body is short) reads out ofbounds, and
n*nitself can overflow.Fix
Walk SRAT with an explicit byte cursor and reject any subtable whose declared
length is shorter than the subtable header or extends past the end of the
table; also reject an SRAT header shorter than the table struct. Using a byte
cursor also removes the reliance on non-standard
void*pointer arithmetic (aGNU extension that happens to compile under the kernel's GCC).
In
parse_slit(), require thatoffsetof(Entry) + n*nfits withinHeader.Lengthand guard againstn*noverflow before assigning the distancematrix.
No functional change on well-formed firmware tables; this only rejects
malformed/truncated tables that would otherwise cause an out-of-bounds read. The
bounds arithmetic was validated with a standalone self-check covering the
overrun, short-header, and truncated-SLIT cases.