Document

linux Function incorrectly returning (int)255 on failure causes kernel panic

The value 0xff is used to mark pfnnid_map pages that are not physically available. But, since pfnnid_map is an array of unsigned values, 0xff is interpreted as 255 and not as -1. As a result, pfn_to_nid() is returning nid=255 for those pages that are not available in physical memory. This value is being used by pfn_valid() to index the node_data array, thus reading data out of node_data bounds.

Bug fixed by commit 91ea8207168

Type	OutOfBoundsRead
Config	"PARISC && DISCONTIGMEM && PROC_PAGE_MONITOR" (3rd degree)
Fix-in	code
Location	arch/parisc/

#include <assert.h>
#include <string.h>

#ifdef CONFIG_NODES_SHIFT
// NB: parisc's CONFIG_NODES_SHIFT default is 3
#define NODES_SHIFT     CONFIG_NODES_SHIFT
#else
#define NODES_SHIFT     0
#endif

#define MAX_NUMNODES    (1 << NODES_SHIFT)

#define node_end_pfn(nid) NODE_DATA(nid)

#define MAX_PHYSMEM_RANGES 8 /* Fix the size for now (current known max is 3) */

#ifdef CONFIG_DISCONTIGMEM

int node_data[MAX_NUMNODES];

#define NODE_DATA(nid)          (node_data[nid])

#define PFNNID_MAP_MAX  512     /* support 512GB */
unsigned char pfnnid_map[PFNNID_MAP_MAX];
unsigned long max_pfn = PFNNID_MAP_MAX;

int pfn_to_nid(unsigned int pfn)
{
	assert(pfn < PFNNID_MAP_MAX);
	return (int)pfnnid_map[pfn];
}

int pfn_valid(unsigned int pfn)
{
	int nid = pfn_to_nid(pfn);

	if (nid >= 0)
		return (pfn < node_end_pfn(nid)); // ERROR
	return 0;
}

#endif

#ifndef CONFIG_DISCONTIGMEM
#define max_pfn 0
#define pfn_valid(pfn)		(1)
#endif /* CONFIG_DISCONTIGMEM */

void setup_bootmem(void)
{
#ifdef CONFIG_DISCONTIGMEM
  memset(pfnnid_map, 0xff, sizeof(pfnnid_map));
#endif
}

#ifdef CONFIG_PROC_PAGE_MONITOR
int kpageflags_read()
{
  unsigned int pfn = 0;

  for (;pfn<max_pfn;pfn++) {
    if (pfn_valid(pfn))
      ;
  }

  return 0;
}
#endif

int main()
{
  setup_bootmem();
#ifdef CONFIG_PROC_PAGE_MONITOR
  kpageflags_read();
#endif
  return 0;
}

diff --git a/simple/91ea820.c b/simple/91ea820.c
--- a/simple/91ea820.c
+++ b/simple/91ea820.c
@@ -22,13 +22,13 @@
 #define NODE_DATA(nid)          (node_data[nid])
 
 #define PFNNID_MAP_MAX  512     /* support 512GB */
-unsigned char pfnnid_map[PFNNID_MAP_MAX];
+signed char pfnnid_map[PFNNID_MAP_MAX];
 unsigned long max_pfn = PFNNID_MAP_MAX;
 
 int pfn_to_nid(unsigned int pfn)
 {
 	assert(pfn < PFNNID_MAP_MAX);
-	return (int)pfnnid_map[pfn];
+	return pfnnid_map[pfn];
 }
 
 int pfn_valid(unsigned int pfn)

#include <assert.h>
#include <string.h>

#ifdef CONFIG_NODES_SHIFT
// NB: parisc's CONFIG_NODES_SHIFT default is 3
#define NODES_SHIFT     CONFIG_NODES_SHIFT
#else
#define NODES_SHIFT     0
#endif

#define MAX_NUMNODES    (1 << NODES_SHIFT)

#define node_end_pfn(nid) NODE_DATA(nid)

#define MAX_PHYSMEM_RANGES 8 /* Fix the size for now (current known max is 3) */

#ifdef CONFIG_DISCONTIGMEM

int node_data[MAX_NUMNODES];
#define NODE_DATA(nid)          (node_data[nid])
#define PFNNID_MAP_MAX  512     /* support 512GB */
unsigned char pfnnid_map[PFNNID_MAP_MAX];
unsigned long max_pfn = PFNNID_MAP_MAX;

#endif

#ifndef CONFIG_DISCONTIGMEM
#define max_pfn 0
#define pfn_valid(pfn)		(1)
#endif /* CONFIG_DISCONTIGMEM */

int main()
{
//  setup_bootmem();
#ifdef CONFIG_DISCONTIGMEM
  memset(pfnnid_map, 0xff, sizeof(pfnnid_map));
#endif
#ifdef CONFIG_PROC_PAGE_MONITOR
//  kpageflags_read();
  unsigned int pfn = 0;

  for (;pfn<max_pfn;pfn++) {
    assert(pfn < PFNNID_MAP_MAX);
    int nid = (int)pfnnid_map[pfn];
    if (nid >= 0)
      return (pfn < node_end_pfn(nid)); // ERROR
  }
#endif
  return 0;
}

. call arch/parisc/mm/init.c:116:setup_bootmem()
. 273: for (i = 0; i < MAX_PHYSMEM_RANGES; i++) {
// MAX_PHYSMEM_RANGES is hardcoded to 8
. 274: memset(NODE_DATA(i), 0, sizeof(pg_data_t));
. [DISCONTIGMEM] 277: memset(pfnnid_map, 0xff, sizeof(pfnnid_map));
// since pfnnid_map is unsigned char[], this is (unsigned char)255

. call fs/proc/page.c:173:kpageflags_read()
. 188: if (pfn_valid(pfn))
.. call arch/parisc/include/asm/mmzone.h:52:pfn_valid()
.. 54: int nid = pfn_to_nid(pfn);
... call arch/parisc/include/asm/mmzone.h:39:pfn_to_nid()
... 49: return (int)pfnnid_map[i];
// if a page is not physically available we will get (int)255 instead of (int)-1
.. 56: if (nid >= 0)
.. 57: return (pfn < node_end_pfn(nid));
... call include/linux/mmzone.h:759:node_end_pfn()
.... call arch/parisc/include/asm/mmzone.h:16:NODE_DATA()
.... ERROR 16: &node_data[nid].pg_data
// node_data[255] may be reading out of bounds