Skip to content
This repository has been archived by the owner on Sep 24, 2020. It is now read-only.

Commit

Permalink
powerpc/pseries: Avoid NULL pointer dereference when drmem is unavail…
Browse files Browse the repository at this point in the history
…able

commit a83836d upstream.

In guests without hotplugagble memory drmem structure is only zero
initialized. Trying to manipulate DLPAR parameters results in a crash.

  $ echo "memory add count 1" > /sys/kernel/dlpar
  Oops: Kernel access of bad area, sig: 11 [#1]
  LE PAGE_SIZE=64K MMU=Hash SMP NR_CPUS=2048 NUMA pSeries
  ...
  NIP:  c0000000000ff294 LR: c0000000000ff248 CTR: 0000000000000000
  REGS: c0000000fb9d3880 TRAP: 0300   Tainted: G            E      (5.5.0-rc6-2-default)
  MSR:  8000000000009033 <SF,EE,ME,IR,DR,RI,LE>  CR: 28242428  XER: 20000000
  CFAR: c0000000009a6c10 DAR: 0000000000000010 DSISR: 40000000 IRQMASK: 0
  ...
  NIP dlpar_memory+0x6e4/0xd00
  LR  dlpar_memory+0x698/0xd00
  Call Trace:
    dlpar_memory+0x698/0xd00 (unreliable)
    handle_dlpar_errorlog+0xc0/0x190
    dlpar_store+0x198/0x4a0
    kobj_attr_store+0x30/0x50
    sysfs_kf_write+0x64/0x90
    kernfs_fop_write+0x1b0/0x290
    __vfs_write+0x3c/0x70
    vfs_write+0xd0/0x260
    ksys_write+0xdc/0x130
    system_call+0x5c/0x68

Taking closer look at the code, I can see that for_each_drmem_lmb is a
macro expanding into `for (lmb = &drmem_info->lmbs[0]; lmb <=
&drmem_info->lmbs[drmem_info->n_lmbs - 1]; lmb++)`. When drmem_info->lmbs
is NULL, the loop would iterate through the whole address range if it
weren't stopped by the NULL pointer dereference on the next line.

This patch aligns for_each_drmem_lmb and for_each_drmem_lmb_in_range
macro behavior with the common C semantics, where the end marker does
not belong to the scanned range, and alters get_lmb_range() semantics.
As a side effect, the wraparound observed in the crash is prevented.

Fixes: 6c6ea53 ("powerpc/mm: Separate ibm, dynamic-memory data from DT format")
Cc: stable@vger.kernel.org # v4.16+
Signed-off-by: Libor Pechacek <lpechacek@suse.cz>
Signed-off-by: Michal Suchanek <msuchanek@suse.de>
Signed-off-by: Michael Ellerman <mpe@ellerman.id.au>
Link: https://lore.kernel.org/r/20200131132829.10281-1-msuchanek@suse.de
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
Libor Pechacek authored and gregkh committed Apr 17, 2020
1 parent 9c6c459 commit 83dc8f0
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 6 deletions.
4 changes: 2 additions & 2 deletions arch/powerpc/include/asm/drmem.h
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,12 @@ struct drmem_lmb_info {
extern struct drmem_lmb_info *drmem_info;

#define for_each_drmem_lmb_in_range(lmb, start, end) \
for ((lmb) = (start); (lmb) <= (end); (lmb)++)
for ((lmb) = (start); (lmb) < (end); (lmb)++)

#define for_each_drmem_lmb(lmb) \
for_each_drmem_lmb_in_range((lmb), \
&drmem_info->lmbs[0], \
&drmem_info->lmbs[drmem_info->n_lmbs - 1])
&drmem_info->lmbs[drmem_info->n_lmbs])

/*
* The of_drconf_cell_v1 struct defines the layout of the LMB data
Expand Down
8 changes: 4 additions & 4 deletions arch/powerpc/platforms/pseries/hotplug-memory.c
Original file line number Diff line number Diff line change
Expand Up @@ -227,7 +227,7 @@ static int get_lmb_range(u32 drc_index, int n_lmbs,
struct drmem_lmb **end_lmb)
{
struct drmem_lmb *lmb, *start, *end;
struct drmem_lmb *last_lmb;
struct drmem_lmb *limit;

start = NULL;
for_each_drmem_lmb(lmb) {
Expand All @@ -240,10 +240,10 @@ static int get_lmb_range(u32 drc_index, int n_lmbs,
if (!start)
return -EINVAL;

end = &start[n_lmbs - 1];
end = &start[n_lmbs];

last_lmb = &drmem_info->lmbs[drmem_info->n_lmbs - 1];
if (end > last_lmb)
limit = &drmem_info->lmbs[drmem_info->n_lmbs];
if (end > limit)
return -EINVAL;

*start_lmb = start;
Expand Down

0 comments on commit 83dc8f0

Please sign in to comment.