Project

General

Profile

Bug #10806

mnode_range_setup() makes assumptions about mnodes

Added by John Levon 7 months ago. Updated 7 months ago.

Status:
Closed
Priority:
Normal
Assignee:
Category:
-
Start date:
2019-04-16
Due date:
% Done:

100%

Estimated time:
Difficulty:
Medium
Tags:
needs-triage

History

#1

Updated by John Levon 7 months ago

SmartOS OS-7644

Some EFI-booted systems do not have a memnode covering pfn 0:

> mem_node_config::print
[
    {
        exists = 0x1
        physbase = 0x10
        physmax = 0xc3ffff
    },
    {
        exists = 0x1
        physbase = 0xc40000
        physmax = 0x183ffff
    },

However, mnode_range_setup() presumes that we do, and flies into hyperspace. We need to fix this assumption and instead just qsort() the mnoderange array.

#2

Updated by John Levon 7 months ago

A little more detailed example from the downstream bug:

[0]> mem_node_config::print
[
    {
        exists = 0x1
        physbase = 0x4000
        physmax = 0xc3ffff
    }, 
    {
        exists = 0x1
        physbase = 0xc40000
        physmax = 0x183ffff
    }...

1393 void                                                                             
1394 mnode_range_setup(mnoderange_t *mnoderanges)                                     
1395 {                                                                                
...
1430         /*                                                                       
1431          * For now do a simple sort of the mnoderanges array to fill in          
1432          * the mnr_next fields.  Since mindex is expected to be relatively       
1433          * small, using a simple O(N^2) algorithm.                               
1434          */                                                                      
1435         for (i = 0; i < mindex; i++) {                                           
1436                 if (mp[i].mnr_pfnlo == 0)       /* find lowest */                
1437                         break;                                                   
1438         }                                                                        
1439         ASSERT(i < mindex);                                                      
1440         last = i;                                                                
1441         mtype16m = last;                                                         

Since we do not have an mnr_pfnlo == 0 mrange, we end up one past the end of the array, which happens to be within the page stuff, and we end up corrupting memory allocated past the mnoderanges.

#3

Updated by Electric Monk 7 months ago

  • Status changed from New to Closed
  • % Done changed from 0 to 100

git commit dddac438d5629d72879f2701924b62774cb4eaf0

commit  dddac438d5629d72879f2701924b62774cb4eaf0
Author: John Levon <john.levon@joyent.com>
Date:   2019-04-30T15:09:06.000Z

    10806 mnode_range_setup() makes assumptions about mnodes
    Reviewed by: Robert Mustacchi <rm@joyent.com>
    Reviewed by: Jerry Jelinek <jerry.jelinek@joyent.com>
    Reviewed by: Toomas Soome <tsoome@me.com>
    Approved by: Dan McDonald <danmcd@joyent.com>

Also available in: Atom PDF