Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1630010 > unrolled thread

Re: KASLR causes intermittent boot failures on some systems

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: KASLR causes intermittent boot failures on some systems Dan Williams <dan.j.williams@intel.com> - 2017-04-24 23:00 +0200
    Re: KASLR causes intermittent boot failures on some systems Baoquan He <bhe@redhat.com> - 2017-04-25 01:10 +0200
      Re: KASLR causes intermittent boot failures on some systems Dan Williams <dan.j.williams@intel.com> - 2017-04-25 01:30 +0200
        Re: KASLR causes intermittent boot failures on some systems Baoquan He <bhe@redhat.com> - 2017-04-25 02:00 +0200
      Re: KASLR causes intermittent boot failures on some systems Thomas Garnier <thgarnie@google.com> - 2017-04-25 02:50 +0200
        Re: KASLR causes intermittent boot failures on some systems Baoquan He <bhe@redhat.com> - 2017-04-25 03:20 +0200

#1630010 — Re: KASLR causes intermittent boot failures on some systems

On Mon, Apr 24, 2017 at 1:37 PM, Thomas Garnier <thgarnie@google.com> wrote:
>  )
>
> On Thu, Apr 20, 2017 at 6:26 AM, Baoquan He <bhe@redhat.com> wrote:
>> On 04/19/17 at 07:27am, Thomas Garnier wrote:
>>> On Wed, Apr 19, 2017 at 6:36 AM, Baoquan He <bhe@redhat.com> wrote:
>>> > Hi all,
>>> >
>>> > I login in Jeff's system, and added debug code, no clue found. However
>>> > DaveY found he disabled page_offset randomization only and the efi issue
>>> > won't be seen on his system with kaslr enabled. I did it too on Jeff's
>>> > pmem system, it has the same result. I have rebooted several times, all
>>> > boot successfully. In the current code, no __PAGE_OFFSET_BASE is used
>>> > directly, don't know why it failed.
>>>
>>> Great! I still cannot repro it.
>>>
>>> >
>>> > Does anyone have any idea or hint I can try? I read pmem code about
>>> > the devm_nsio_enable/pmem_attach_disk/arch_add_memory, have no idea yet.
>>>
>>> I would test couple things:
>>>  - Set page_offset_base to 0 by default and set it to
>>> __PAGE_OFFSET_BASE in kernel_randomize_memory (without randomizing
>>> it). If it crashes on a low address, it might be due to using __va or
>>> PAGE_OFFSET in general before randomization is done.
>>>  - Does any change in __PAGE_OFFSET lead to a crash? Or only when
>>> __PAGE_OFFSET is on a specific range. Given that you may have to
>>> reboot multiple times to get a crash, I assume that a specific range
>>> is the problem but might be worth checking.
>>
>> I added debug code and collected boot logs about failure cases and
>> success cases, seems it's related to crossing pgd entry issue. Below
>> code change is part of my debugging code, I added printing anywhere,
>> just abstract this for better understanding of the printed information
>> below it. The emulated pmem memory is [1TB, 1TB+192G], namely
>> [0x10000000000, 0x13000000000). If the left pud entries indexed from 1TB
>> is smaller than 192, it will fail. init_memory_mapping might have
>> handled direct mapping well, I am not sure if __add_pages is OK.
>>
>> diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
>> index 5b536be..f3f8d43 100644
>> --- a/drivers/nvdimm/pmem.c
>> +++ b/drivers/nvdimm/pmem.c
>> @@ -87,6 +87,8 @@ static int read_pmem(struct page *page, unsigned int off,
>>  {
>>         int rc;
>>         void *mem = kmap_atomic(page);
>> +       pr_info("pfn:0x%llu, off=0x%lx, pmem_addr:0x%llx, len:0x%lx\n",
>> +               page_to_pfn(page), off, pmem_addr, len);
>>
>>         rc = memcpy_from_pmem(mem + off, pmem_addr, len);
>>         kunmap_atomic(mem);
>> @@ -312,6 +318,8 @@ static int pmem_attach_disk(struct device *dev,
>>         if (IS_ERR(addr))
>>                 return PTR_ERR(addr);
>>         pmem->virt_addr = addr;
>> +       pr_info("pmem->virt_addr:0x%llx, pmem->phys_addr:0x%llx, pmem->size:0x%llx\n",
>> +               pmem->virt_addr, pmem->phys_addr, pmem->size);
>>
>>         blk_queue_write_cache(q, true, true);
>>         blk_queue_make_request(q, pmem_make_request);
>>
>>
>
> Super useful. I can see that the virt_addr field can be set in three
> locations (http://lxr.free-electrons.com/source/drivers/nvdimm/pmem.c#L288).
> Can you check which one is used for the faulting addresses?
>
> Also the two functions used (devm_memremap_pages and devm_memremap)
> seem to check if the region intersects with IORESOURCE_SYSTEM_RAM, if
> it does then the mapping is not done and the __va is returned. I would
> be interested to know if this is what's happening. Basically logging
> the VA on these lines:
>
>  - http://lxr.free-electrons.com/source/kernel/memremap.c#L307
>  - http://lxr.free-electrons.com/source/kernel/memremap.c#L98
>
> This way, we can get closer to which code does not handle PG boundary correctly.
>
> Thanks!
>

When using the memmap= parameter we're using this call by default:

        } else if (pmem_should_map_pages(dev)) {
                addr = devm_memremap_pages(dev, &nsio->res,
                                &q->q_usage_counter, NULL);
                pmem->pfn_flags |= PFN_MAP;
        } else

...where we are assuming that the memmap= parameter does not specify a
range-size that will exhaust all of system-memory just to hold the
struct page array.

[toc] | [next] | [standalone]

#1630105

On 04/24/17 at 01:52pm, Dan Williams wrote:
> On Mon, Apr 24, 2017 at 1:37 PM, Thomas Garnier <thgarnie@google.com> wrote:
> >  )
> >
> > On Thu, Apr 20, 2017 at 6:26 AM, Baoquan He <bhe@redhat.com> wrote:
> >> On 04/19/17 at 07:27am, Thomas Garnier wrote:
> >>> On Wed, Apr 19, 2017 at 6:36 AM, Baoquan He <bhe@redhat.com> wrote:
> >>> > Hi all,
> >>> >
> >>> > I login in Jeff's system, and added debug code, no clue found. However
> >>> > DaveY found he disabled page_offset randomization only and the efi issue
> >>> > won't be seen on his system with kaslr enabled. I did it too on Jeff's
> >>> > pmem system, it has the same result. I have rebooted several times, all
> >>> > boot successfully. In the current code, no __PAGE_OFFSET_BASE is used
> >>> > directly, don't know why it failed.
> >>>
> >>> Great! I still cannot repro it.
> >>>
> >>> >
> >>> > Does anyone have any idea or hint I can try? I read pmem code about
> >>> > the devm_nsio_enable/pmem_attach_disk/arch_add_memory, have no idea yet.
> >>>
> >>> I would test couple things:
> >>>  - Set page_offset_base to 0 by default and set it to
> >>> __PAGE_OFFSET_BASE in kernel_randomize_memory (without randomizing
> >>> it). If it crashes on a low address, it might be due to using __va or
> >>> PAGE_OFFSET in general before randomization is done.
> >>>  - Does any change in __PAGE_OFFSET lead to a crash? Or only when
> >>> __PAGE_OFFSET is on a specific range. Given that you may have to
> >>> reboot multiple times to get a crash, I assume that a specific range
> >>> is the problem but might be worth checking.
> >>
> >> I added debug code and collected boot logs about failure cases and
> >> success cases, seems it's related to crossing pgd entry issue. Below
> >> code change is part of my debugging code, I added printing anywhere,
> >> just abstract this for better understanding of the printed information
> >> below it. The emulated pmem memory is [1TB, 1TB+192G], namely
> >> [0x10000000000, 0x13000000000). If the left pud entries indexed from 1TB
> >> is smaller than 192, it will fail. init_memory_mapping might have
> >> handled direct mapping well, I am not sure if __add_pages is OK.
> >>
> >> diff --git a/drivers/nvdimm/pmem.c b/drivers/nvdimm/pmem.c
> >> index 5b536be..f3f8d43 100644
> >> --- a/drivers/nvdimm/pmem.c
> >> +++ b/drivers/nvdimm/pmem.c
> >> @@ -87,6 +87,8 @@ static int read_pmem(struct page *page, unsigned int off,
> >>  {
> >>         int rc;
> >>         void *mem = kmap_atomic(page);
> >> +       pr_info("pfn:0x%llu, off=0x%lx, pmem_addr:0x%llx, len:0x%lx\n",
> >> +               page_to_pfn(page), off, pmem_addr, len);
> >>
> >>         rc = memcpy_from_pmem(mem + off, pmem_addr, len);
> >>         kunmap_atomic(mem);
> >> @@ -312,6 +318,8 @@ static int pmem_attach_disk(struct device *dev,
> >>         if (IS_ERR(addr))
> >>                 return PTR_ERR(addr);
> >>         pmem->virt_addr = addr;
> >> +       pr_info("pmem->virt_addr:0x%llx, pmem->phys_addr:0x%llx, pmem->size:0x%llx\n",
> >> +               pmem->virt_addr, pmem->phys_addr, pmem->size);
> >>
> >>         blk_queue_write_cache(q, true, true);
> >>         blk_queue_make_request(q, pmem_make_request);
> >>
> >>
> >
> > Super useful. I can see that the virt_addr field can be set in three
> > locations (http://lxr.free-electrons.com/source/drivers/nvdimm/pmem.c#L288).
> > Can you check which one is used for the faulting addresses?
> >
> > Also the two functions used (devm_memremap_pages and devm_memremap)
> > seem to check if the region intersects with IORESOURCE_SYSTEM_RAM, if
> > it does then the mapping is not done and the __va is returned. I would
> > be interested to know if this is what's happening. Basically logging
> > the VA on these lines:
> >
> >  - http://lxr.free-electrons.com/source/kernel/memremap.c#L307
> >  - http://lxr.free-electrons.com/source/kernel/memremap.c#L98
> >
> > This way, we can get closer to which code does not handle PG boundary correctly.
> >
> > Thanks!
> >
> 
> When using the memmap= parameter we're using this call by default:
> 
>         } else if (pmem_should_map_pages(dev)) {
>                 addr = devm_memremap_pages(dev, &nsio->res,
>                                 &q->q_usage_counter, NULL);
>                 pmem->pfn_flags |= PFN_MAP;
>         } else
> 
> ...where we are assuming that the memmap= parameter does not specify a
> range-size that will exhaust all of system-memory just to hold the
> struct page array.

Yeah, according to my debugging tracking, it goes as Dan said. And the
is_ram is REGION_DISJOINT. And till arch_add_memory, the parameters
passed to arch_add_memory are "arch_add_memory, align_start:0x10000000000, align_size:0x3000000000",
seems it's going well.

Hi Dan,

I am always confused that in devm_memremap_pages, the passed in
parameter altmap is NULL, while it used devres_alloc_node to allocate a
page_map and that page_map contained a altmap instance, not pointer.
Then the addr range were inserted into pgmap_radix with value of
page_map. Why later in __add_pages, to_vmem_altmap() return NULL
according to my debugging code?

Thanks
Baoquan

[toc] | [prev] | [next] | [standalone]

#1630119

On Mon, Apr 24, 2017 at 4:07 PM, Baoquan He <bhe@redhat.com> wrote:
> On 04/24/17 at 01:52pm, Dan Williams wrote:
[..]
>> When using the memmap= parameter we're using this call by default:
>>
>>         } else if (pmem_should_map_pages(dev)) {
>>                 addr = devm_memremap_pages(dev, &nsio->res,
>>                                 &q->q_usage_counter, NULL);
>>                 pmem->pfn_flags |= PFN_MAP;
>>         } else
>>
>> ...where we are assuming that the memmap= parameter does not specify a
>> range-size that will exhaust all of system-memory just to hold the
>> struct page array.
>
> Yeah, according to my debugging tracking, it goes as Dan said. And the
> is_ram is REGION_DISJOINT. And till arch_add_memory, the parameters
> passed to arch_add_memory are "arch_add_memory, align_start:0x10000000000, align_size:0x3000000000",
> seems it's going well.
>
> Hi Dan,
>
> I am always confused that in devm_memremap_pages, the passed in
> parameter altmap is NULL, while it used devres_alloc_node to allocate a
> page_map and that page_map contained a altmap instance, not pointer.
> Then the addr range were inserted into pgmap_radix with value of
> page_map. Why later in __add_pages, to_vmem_altmap() return NULL
> according to my debugging code?

We expect altmap to always be NULL in this case. The only time it is
not NULL is when the namespace is configured to allocate the struct
page array from capacity on the namespace itself. I.e. instead of
allocating struct page from page allocator pages the driver creates an
altmap and vmemmap_populate_hugepages() uses that to allocate the
array from "alternate" capacity.

You can force this by running:

    ndctl create-namespace -f -e namespace0.0 -m memory -M dev

...which says "put the struct page map on the device".

[toc] | [prev] | [next] | [standalone]

#1630126

On 04/24/17 at 04:18pm, Dan Williams wrote:
> On Mon, Apr 24, 2017 at 4:07 PM, Baoquan He <bhe@redhat.com> wrote:
> > On 04/24/17 at 01:52pm, Dan Williams wrote:
> [..]
> >> When using the memmap= parameter we're using this call by default:
> >>
> >>         } else if (pmem_should_map_pages(dev)) {
> >>                 addr = devm_memremap_pages(dev, &nsio->res,
> >>                                 &q->q_usage_counter, NULL);
> >>                 pmem->pfn_flags |= PFN_MAP;
> >>         } else
> >>
> >> ...where we are assuming that the memmap= parameter does not specify a
> >> range-size that will exhaust all of system-memory just to hold the
> >> struct page array.
> >
> > Yeah, according to my debugging tracking, it goes as Dan said. And the
> > is_ram is REGION_DISJOINT. And till arch_add_memory, the parameters
> > passed to arch_add_memory are "arch_add_memory, align_start:0x10000000000, align_size:0x3000000000",
> > seems it's going well.
> >
> > Hi Dan,
> >
> > I am always confused that in devm_memremap_pages, the passed in
> > parameter altmap is NULL, while it used devres_alloc_node to allocate a
> > page_map and that page_map contained a altmap instance, not pointer.
> > Then the addr range were inserted into pgmap_radix with value of
> > page_map. Why later in __add_pages, to_vmem_altmap() return NULL
> > according to my debugging code?
> 
> We expect altmap to always be NULL in this case. The only time it is
> not NULL is when the namespace is configured to allocate the struct
> page array from capacity on the namespace itself. I.e. instead of
> allocating struct page from page allocator pages the driver creates an
> altmap and vmemmap_populate_hugepages() uses that to allocate the
> array from "alternate" capacity.
> 
> You can force this by running:
> 
>     ndctl create-namespace -f -e namespace0.0 -m memory -M dev
> 
> ...which says "put the struct page map on the device".

Ah, got. It only assigned page_map->altmap to pgmap->altmap when the
passed in altmap is not NULL. 

pgmap->altmap = &page_map->altmap;

I didn't notice this. Thanks for telling. So in this case without
alternate capacity, it will allocate from normal ram memory struct page
array.

Thanks
Baoquan

[toc] | [prev] | [next] | [standalone]

#1630133

On Mon, Apr 24, 2017 at 4:07 PM, Baoquan He <bhe@redhat.com> wrote:
> Yeah, according to my debugging tracking, it goes as Dan said. And the
> is_ram is REGION_DISJOINT. And till arch_add_memory, the parameters
> passed to arch_add_memory are "arch_add_memory, align_start:0x10000000000, align_size:0x3000000000",
> seems it's going well.

Hum ok, I hope it was another path given it seems the page table is
correctly setup on this one.

Maybe looking at the PTEs with and without crash might help? (setting
this if statement to 1:
http://lxr.free-electrons.com/source/arch/x86/mm/init_64.c#L360)

Thanks Baoquan,

-- 
Thomas

[toc] | [prev] | [next] | [standalone]

#1630138

On 04/24/17 at 05:41pm, Thomas Garnier wrote:
> On Mon, Apr 24, 2017 at 4:07 PM, Baoquan He <bhe@redhat.com> wrote:
> > Yeah, according to my debugging tracking, it goes as Dan said. And the
> > is_ram is REGION_DISJOINT. And till arch_add_memory, the parameters
> > passed to arch_add_memory are "arch_add_memory, align_start:0x10000000000, align_size:0x3000000000",
> > seems it's going well.
> 
> Hum ok, I hope it was another path given it seems the page table is
> correctly setup on this one.
> 
> Maybe looking at the PTEs with and without crash might help? (setting
> this if statement to 1:
> http://lxr.free-electrons.com/source/arch/x86/mm/init_64.c#L360)

The page_size_mask is PG_LEVEL_1G, it should only enter into
phys_pud_init and return.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web