Re: [PATCH 1/2] x86: Check return values from early_memremap calls

From: Juergen Gross
Date: Wed Oct 12 2022 - 11:26:18 EST


On 12.10.22 17:13, Ross Philipson wrote:
On 10/8/22 11:12, Borislav Petkov wrote:
Adding Xen and Jailhouse people and MLs to Cc.

Folks, thread starts here:

https://lore.kernel.org/r/1650035401-22855-1-git-send-email-ross.philipson@xxxxxxxxxx

On Fri, Apr 15, 2022 at 11:10:00AM -0400, Ross Philipson wrote:
There are a number of places where early_memremap is called
but the return pointer is not checked for NULL. The call
can result in a NULL being returned so the checks must
be added.

Signed-off-by: Ross Philipson <ross.philipson@xxxxxxxxxx>
---
  arch/x86/kernel/devicetree.c | 10 ++++++++++
  arch/x86/kernel/e820.c       |  5 +++++
  arch/x86/kernel/jailhouse.c  |  6 ++++++
  arch/x86/kernel/mpparse.c    | 23 +++++++++++++++++++++++
  arch/x86/kernel/setup.c      |  5 +++++
  arch/x86/xen/enlighten_hvm.c |  2 ++
  arch/x86/xen/mmu_pv.c        |  8 ++++++++
  arch/x86/xen/setup.c         |  2 ++
  8 files changed, 61 insertions(+)

Ok, a couple of notes:

1. the pr_*("<prefix>:" ... )

thing is done using pr_fmt() - grep the tree for examples.

I am already using the pr_* macros in the patches. Are you asking me to do something or is this just informational?


2. I think you should not panic() the machine but issue a the
warning/error and let the machine die a painful death anyway. But Xen
folks will know better what would be the optimal thing to do.

When I was working on the patches I asked Andrew Cooper at Citrix what action I should take if any of the calls in the Xen code failed. I believe he told me it was basically fatal and that panic() would be fine there.

panic() is the way to go. Everything else would make the error harder
to analyze.

BTW, CC-ing the maintainers of the modified code is good practice.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature