public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
From: "Krzysztof Koch" <krzysztof.koch@arm.com>
To: "devel@edk2.groups.io" <devel@edk2.groups.io>
Cc: "ray.ni@intel.com" <ray.ni@intel.com>,
	"zhichao.gao@intel.com" <zhichao.gao@intel.com>,
	Sami Mujawar <Sami.Mujawar@arm.com>,
	Matteo Carlini <Matteo.Carlini@arm.com>, nd <nd@arm.com>
Subject: Re: [edk2-devel] [PATCH v3 00/11] Test against invalid pointers in acpiview
Date: Mon, 3 Feb 2020 09:07:22 +0000	[thread overview]
Message-ID: <AM5PR0801MB1777091B6BB6A5BBE7B9796C84000@AM5PR0801MB1777.eurprd08.prod.outlook.com> (raw)
In-Reply-To: <15EB9409DB0D0A3E.19926@groups.io>

Hi,

Is there any chance this patch series will get reviewed soon?
The v1 series has already been reviewed a few months ago and the patches which were not ok were re-submitted as v2.
The v3 series is basically a rebase of v1 on latest master with the v2 changes included.

Kind regards,
Krzysztof

-----Original Message-----
From: devel@edk2.groups.io <devel@edk2.groups.io> On Behalf Of Krzysztof Koch via Groups.Io
Sent: Monday, January 20, 2020 11:14
To: devel@edk2.groups.io
Cc: ray.ni@intel.com; zhichao.gao@intel.com; Sami Mujawar <Sami.Mujawar@arm.com>; Matteo Carlini <Matteo.Carlini@arm.com>; nd <nd@arm.com>
Subject: [edk2-devel] [PATCH v3 00/11] Test against invalid pointers in acpiview

Prevent the use of invalid pointers when parsing ACPI tables in the UEFI shell acpiview tool.

The parsing of ACPI tables is often controlled with the values read earlier from the same table. For example, the 'Offset' or 'Count' fields found in a structure are later used to parse the substructures. If such fields lie outside the structure's buffer length provided, then there is a possibility for a wild or dangling pointer.

Currently, if the ParseAcpi() function terminates early because the end of the input table data buffer has been reached, then the pointers which were supposed to be updated by this function are left untouched.
This is a security issue as the values pointed to by these pointers are later used for flow control.

This patch series aims to solve this security issue by explicitly initializing any pointers lying outside the input ACPI data buffer to NULL and testing for NULL whenever these pointers are dereferenced.

Changes can be seet at: https://github.com/KrzysztofKoch1/edk2/tree/612_add_pointer_validation_v3

Notes:
    v3:
    - Rebase on latest master [Krzysztof]

    v2:
    - Do not require FadtMinorRevision and X_DsdtAddress pointers to be
      valid in FADT table parser [Zhichao]

    v1:
    - Validate static pointers in acpiview parsers before use [Krzysztof]

Krzysztof Koch (11):
  ShellPkg: acpiview: Set ItemPtr to NULL for unprocessed table fields
  ShellPkg: acpiview: RSDP: Validate global pointer before use
  ShellPkg: acpiview: FADT: Validate global pointer before use
  ShellPkg: acpiview: SLIT: Validate global pointer before use
  ShellPkg: acpiview: SLIT: Validate System Locality count
  ShellPkg: acpiview: SRAT: Validate global pointers before use
  ShellPkg: acpiview: MADT: Validate global pointers before use
  ShellPkg: acpiview: PPTT: Validate global pointers before use
  ShellPkg: acpiview: IORT: Validate global pointers before use
  ShellPkg: acpiview: GTDT: Validate global pointers before use
  ShellPkg: acpiview: DBG2: Validate global pointers before use

 ShellPkg/Library/UefiShellAcpiViewCommandLib/AcpiParser.c              |  9 ++-
 ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Dbg2/Dbg2Parser.c | 43 ++++++++++++++  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Fadt/FadtParser.c | 21 +++----  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Gtdt/GtdtParser.c | 37 ++++++++++++  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Iort/IortParser.c | 52 +++++++++++++++++  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Madt/MadtParser.c | 13 +++++  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Pptt/PpttParser.c | 25 ++++++++  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Rsdp/RsdpParser.c | 12 ++++  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Slit/SlitParser.c | 61 ++++++++++++++++++--  ShellPkg/Library/UefiShellAcpiViewCommandLib/Parsers/Srat/SratParser.c | 13 +++++
 10 files changed, 269 insertions(+), 17 deletions(-)

--
'Guid(CE165669-3EF3-493F-B85D-6190EE5B9759)'






           reply	other threads:[~2020-02-03  9:07 UTC|newest]

Thread overview: expand[flat|nested]  mbox.gz  Atom feed
 [parent not found: <15EB9409DB0D0A3E.19926@groups.io>]

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-list 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=AM5PR0801MB1777091B6BB6A5BBE7B9796C84000@AM5PR0801MB1777.eurprd08.prod.outlook.com \
    --to=devel@edk2.groups.io \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox