[PATCH] crashdump-ppc64: crashkernel-base and crashkernel-size are big-endian

2020-03-04 Thread Thadeu Lima de Souza Cascardo 
When reading the device-tree exported crashkernel-base and
crashkernel-size, their values should be converted from big-endian to the
CPU byte order.

These is the output of running kexec --print-ckr-size on a little-endian
ppc64 box.

$ kexec --print-ckr-size
137438953472
$ kexec --print-ckr-size
536870912

Signed-off-by: Thadeu Lima de Souza Cascardo 
---
 kexec/arch/ppc64/crashdump-ppc64.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/kexec/arch/ppc64/crashdump-ppc64.c 
b/kexec/arch/ppc64/crashdump-ppc64.c
index 50e3853dbdcf..b2787d5135bd 100644
--- a/kexec/arch/ppc64/crashdump-ppc64.c
+++ b/kexec/arch/ppc64/crashdump-ppc64.c
@@ -612,12 +612,12 @@ int get_crash_kernel_load_range(uint64_t *start, uint64_t 
*end)
unsigned long long value;
 
if (!get_devtree_value(DEVTREE_CRASHKERNEL_BASE, ))
-   *start = value;
+   *start = be64_to_cpu(value);
else
return -1;
 
if (!get_devtree_value(DEVTREE_CRASHKERNEL_SIZE, ))
-   *end = *start + value - 1;
+   *end = *start + be64_to_cpu(value) - 1;
else
return -1;
 
-- 
2.17.1


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] makedumpfile: cope with not-present mem section

2020-02-20 Thread Thadeu Lima de Souza Cascardo 
On Thu, Feb 20, 2020 at 03:40:12PM +, HAGIO KAZUHITO(萩尾 一仁) wrote:
> -Original Message-
> > On 02/20/2020 04:12 AM, HAGIO KAZUHITO wrote:
> > > Hi Cascardo,
> > >
> > > Do you have any solution or detailed information on the failure on your 
> > > kernel?
> > > or could you try this branch?  It has an additional patch on top of 
> > > Pingfan's
> > > one to avoid the false positive failure that I'm suspecting:
> > > https://github.com/k-hagio/makedumpfile/tree/modify-mem_section-validation
> > >
> > > Pingfan,
> > > Do you have an output of makedumpfile when the original failure occurs?
> > > If you don't and it's hard to get it, no need to do so.  I just would 
> > > like to
> > > add it to your patch if available.
> > I did the test on a PowerVM. After hot removing the memory, save a raw
> > vmcore by "cp", then run makedumpfile against the "cp" vmcore, and it
> > will get the following error message:
> > # makedumpfile -x vmlinux -l -d 31 vmcore vmcore.dump
> > get_mem_section: Could not validate mem_section.
> > get_mm_sparsemem: Can't get the address of mem_section.
> > 
> > makedumpfile Failed.
> 
> Thank you, will add it to your patch.
> 
> and a bit of explanation for the branch above..
> 
> > >>>> But from the logic of this patch, it just does the following changes:
> > >>>> -1. After memory hot-removed, either mem_section.section_mem_map = NULL
> > >>>> or mem_section.section_mem_map without SECTION_MARKED_PRESENT, we will
> > >>>> have mem_maps[section_nr] = mem_map = NOT_MEMMAP_ADDR, so later it will
> > >>>> be skipped.
> > >>>> -2. If not populated, mem_section.section_mem_map = NULL. It can follow
> > >>>> the same handling of hot-removed, and be skipped during parsing.
> > >>>>
> > >>>> And in v4.4 sparse_remove_one_section() just assigns 
> > >>>> ms->section_mem_map
> > >>>> = 0, which can not be violated by the above changes.
> > >> Ping. As all of us can not reproduce this bug by v4.4 kernel, further
> > >> more, there is no code analysis, which persuades us this patch will
> > >> break the makedumpfile on any kernel version.
> 
> I'm not clear what causes it on the 4.4 kernel because of lack of information.
> But at least your patch relaxes the condition to recognize data of an address
> as a valid mem_section.  So I'm concerned that both of the first and second
> validate_mem_section() may return true by accident or something.  If it is not
> caused by a patch in the 4.4 kernel, for example, just data location causes 
> it,
> it may occur on upstream kernels.  Although we cannot reproduce it so far.
> 
> Whatever causes it, in the first place, upstream kernels with vmcoreinfo don't
> need the second validate_mem_section().  It's almost for some downstream
> kernels and has a risk that causes problem like this.  So I'm thinking that
> it might be safer to change it to the if(!ret) way on the branch above
> with your patch.
> 
> Thanks,
> Kazu
> 

Hi, Kazu.

For now, I would keep Pingfan's patch as is. I decided to test some other
kernels, like 3.16 and 4.9 without commit
83e3c48729d9ebb7af5a31a504f3fd6aff0348c4.

So, they would be valid for the first iteration and invalid for the second,
just like Ubuntu's 4.4 kernel. As I couldn't reproduce the problem, I
investigated further and realized I was testing without your commit for
makedumpfile: 8f1aafa1643532ece86cba22f2187d0f42fb7ca3 ("PATCH Fix
validate_mem_section()").

With that one, all of Ubuntu's 4.4 kernel and Debian's 3.16 and 4.9 kernels
dump correctly. Without that one, all fail, and I suppose it would be easily
reproducible.

So, in effect, we are looking for any entry with the present bit, and then
checking it is valid kernel address. That seems to work just fine.

As you mention, the only case we do the second check is for some downstream
kernels (though I would argue we should care about those the most), but at
least from the Ubuntu side, those should not be around in the field anymore,
and, by default, those should be the rare exception anyway. So, I agree with
your follow-up commit in your branch, as it also simplifies the code a lot.

If you care, feel free to add my Ack to the two patches.

Acked-by: Thadeu Lima de Souza Cascardo 

Thanks for your patience.
Thadeu Cascardo.

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] makedumpfile: cope with not-present mem section

2020-02-19 Thread Thadeu Lima de Souza Cascardo 
On Wed, Feb 19, 2020 at 08:12:41PM +, HAGIO KAZUHITO(萩尾 一仁) wrote:
> Hi Cascardo,
> 
> Do you have any solution or detailed information on the failure on your 
> kernel?
> or could you try this branch?  It has an additional patch on top of Pingfan's
> one to avoid the false positive failure that I'm suspecting:
> https://github.com/k-hagio/makedumpfile/tree/modify-mem_section-validation
> 
> Pingfan,
> Do you have an output of makedumpfile when the original failure occurs?
> If you don't and it's hard to get it, no need to do so.  I just would like to
> add it to your patch if available.
> 
> Thanks,
> Kazu

Will try the said branch. Sorry that I couldn't work this out before. I was
trying to reproduce this today, but end up in a rabbit hole when qemu+KVM
started failing for unrelated reasons after an upgrade.

I'll try to come up with some new results by tomorrow later in the day.

Thanks.
Cascardo.

> 
> -Original Message-
> > On 02/12/2020 12:11 PM, piliu wrote:
> > >
> > >
> > > On 02/06/2020 11:46 AM, piliu wrote:
> > >>
> > >>
> > >> On 02/05/2020 05:18 AM, HAGIO KAZUHITO wrote:
> >  -Original Message-
> >  On Tue, Feb 04, 2020 at 02:24:17PM +0800, piliu wrote:
> > > Hi,
> > >
> > > Sorry to reply late due to a long festival.
> > >
> > > I have tested this patch against v4.15 and latest kernel with small
> > > modification to meet the situation we discussed here. Both work fine.
> > >
> > > The below is the modification of two kernel
> > >
> > > test1. latest kernel with two extra modification to expose the problem
> > > -1.1 reverts commit 1f503443e7df8dc8366608b4d810ce2d6669827c
> > > (mm/sparse.c: reset section's mem_map when fully deactivated), this
> > > commit work around this bug
> > > -1.2. reverts commit a0b1280368d1e91ab72f849ef095b4f07a39bbf1 ("kdump:
> > > write correct address of mem_section into vmcoreinfo"). This will 
> > > create
> > > a buggy situation as we discussed here.
> > > -1.3. fix building bug due to revert
> > > a0b1280368d1e91ab72f849ef095b4f07a39bbf1
> > >
> > > test2. v4.15, which include both commit 83e3c48729d9 and a0b1280368d1.
> > > -2.1. revert commit a0b1280368d1e91ab72f849ef095b4f07a39bbf1 ("kdump:
> > > write correct address of mem_section into vmcoreinfo")
> > >
> > > So I can not see any problem with my patch.
> > > Maybe I misunderstand the discussion, but I can not see my original
> > > patch will break the kernel which have 83e3c48729d9 but not 
> > > a0b1280368d1.
> > >
> > > Thanks,
> > > Pingfan
> > >
> > 
> >  You also need to test the case where 83e3c48729d9 is not present at 
> >  all. Can
> >  you test on a 4.4 kernel, for example? As far as I understand, a 
> >  vanilla 4.4
> >  kernel would not be dumpable with your patch.
> > >>>
> > >>> As far as I've tested this patch with SPARSEMEM_EXTREME vmcores below, 
> > >>> it's OK:
> > >>>   - 51 vmcores of vanilla kernels (each from 2.6.36 through 5.5) on hand
> > >>>   - one more vanilla 4.4.0 kernel with a different config from the above
> > >>>
> > >>> So apparently not all vanilla 4.4 kernels are affected by the patch.
> > >>>
> > >> Sorry, due to touch hardware resource in our lab, I can not have a test
> > >> on v4.4 on a system with hotplug memory yet. I still try to find some
> > >> resource.
> > >>
> > >> But from the logic of this patch, it just does the following changes:
> > >> -1. After memory hot-removed, either mem_section.section_mem_map = NULL
> > >> or mem_section.section_mem_map without SECTION_MARKED_PRESENT, we will
> > >> have mem_maps[section_nr] = mem_map = NOT_MEMMAP_ADDR, so later it will
> > >> be skipped.
> > >> -2. If not populated, mem_section.section_mem_map = NULL. It can follow
> > >> the same handling of hot-removed, and be skipped during parsing.
> > >>
> > >> And in v4.4 sparse_remove_one_section() just assigns ms->section_mem_map
> > >> = 0, which can not be violated by the above changes.
> > Ping. As all of us can not reproduce this bug by v4.4 kernel, further
> > more, there is no code analysis, which persuades us this patch will
> > break the makedumpfile on any kernel version.
> > 
> > Could this better-to-have patch be accepted?
> > 
> > Thanks,
> > Pingfan
> > > Last night, I got a machine to test this scene. After applying my patch
> > > makedumpfile can still work with v4.4 kernel.
> > >
> > > Thanks,
> > > Pingfan
> > >
> 

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] makedumpfile: cope with not-present mem section

2020-02-04 Thread Thadeu Lima de Souza Cascardo 
On Tue, Feb 04, 2020 at 02:24:17PM +0800, piliu wrote:
> Hi,
> 
> Sorry to reply late due to a long festival.
> 
> I have tested this patch against v4.15 and latest kernel with small
> modification to meet the situation we discussed here. Both work fine.
> 
> The below is the modification of two kernel
> 
> test1. latest kernel with two extra modification to expose the problem
> -1.1 reverts commit 1f503443e7df8dc8366608b4d810ce2d6669827c
> (mm/sparse.c: reset section's mem_map when fully deactivated), this
> commit work around this bug
> -1.2. reverts commit a0b1280368d1e91ab72f849ef095b4f07a39bbf1 ("kdump:
> write correct address of mem_section into vmcoreinfo"). This will create
> a buggy situation as we discussed here.
> -1.3. fix building bug due to revert
> a0b1280368d1e91ab72f849ef095b4f07a39bbf1
> 
> test2. v4.15, which include both commit 83e3c48729d9 and a0b1280368d1.
> -2.1. revert commit a0b1280368d1e91ab72f849ef095b4f07a39bbf1 ("kdump:
> write correct address of mem_section into vmcoreinfo")
> 
> So I can not see any problem with my patch.
> Maybe I misunderstand the discussion, but I can not see my original
> patch will break the kernel which have 83e3c48729d9 but not a0b1280368d1.
> 
> Thanks,
> Pingfan
> 

You also need to test the case where 83e3c48729d9 is not present at all. Can
you test on a 4.4 kernel, for example? As far as I understand, a vanilla 4.4
kernel would not be dumpable with your patch.

Thanks.
Cascardo.

> On 01/29/2020 03:33 AM, Thadeu Lima de Souza Cascardo wrote:
> > On Tue, Jan 28, 2020 at 05:03:12PM +, HAGIO KAZUHITO(萩尾 一仁) wrote:
> >> Hi Cascardo,
> >>
> >>> -Original Message-
> >>> On Mon, Jan 27, 2020 at 02:04:54PM -0300, Thadeu Lima de Souza Cascardo 
> >>> wrote:
> >>>> Sorry for taking too long to respond, as I was on vacation.
> >>>>
> >>>> The kernels that had commit 83e3c48729d9, but not commit a0b1280368d1, 
> >>>> are
> >>>> not supported anymore. In a way that it's even hard for me to test them.
> >>>>
> >>>> However, I managed to test it, and those two lines are definitively 
> >>>> needed
> >>>> to dump a VM running such a kernel. Is removing them really needed to fix
> >>>> this issue?
> >>>>
> >>>> Otherwise, I would rather keep them.
> >>>>
> >>>> Thanks.
> >>>> Cascardo.
> >>>
> >>> By the way, I was too fast in sending this. We really need to keep those 
> >>> lines
> >>> as makedumpfile will fail to dump a 4.4 kernel with this patch as is.
> >>
> >> Is that Ubuntu 4.4 kernel which has 83e3c48729d9 and not a0b1280368d1?
> >> Could you elaborate on how it fails?
> > 
> > No, it doesn't have either, so my guess is it would fail on upstream 4.4 as
> > well, so anything that doesn't have 83e3c48729d9.
> > 
> > That's what I get on that 4.4 kernel (4.4.0-171-generic):
> > 
> > # ./makedumpfile /proc/vmcore ../dump
> > get_mem_section: Could not validate mem_section.
> > get_mm_sparsemem: Can't get the address of mem_section.
> > 
> > makedumpfile Failed.
> > #
> > 
> > So, now, I have a better grasp of the whole logic, and understand why it 
> > fails
> > with this patch.
> > 
> > So, we need to either interpret the mem_section as a pointer to the array 
> > of a
> > pointer to the pointer to the array. The only case the second option is 
> > valid
> > is when sparse_extreme is on, so we don't even need to check the second case
> > when it's off.
> > 
> > Then, we check that interpreting it either way is valid. If it's valid in 
> > both
> > interpretations, we can't decide which to use, and will fail. So far, we
> > haven't seen any case in the field where that would accidentally happen. 
> > But in
> > case it does, we should rather fail to dump and fallback to copying, than
> > creating a bogus compressed dump.
> > 
> > When this patch is applied, a kernel which does not have 83e3c48729d9, and
> > thus, has mem_section as a direct pointer to the array, it so happens that 
> > we
> > don't detect the pointer to pointer to the array case as invalid, thus 
> > failing
> > to dump.
> > 
> > The way we validate is that the mem_maps should either have the PRESENT bit 
> > or
> > be NULL. Now, that assumption may be invalid, and we would need to do the
> > validation some other way. I can test the cases where that assumption is
> > invalid in a 4.4 kernel and see how to fix this in a satisfactory way.
> > 
> > Going through the code once again, I don't see how the second section of the
> > patch would be correct by itself too. I will think it through a little more 
> > and
> > see if I can come up with a solution.
> > 
> > Regards.
> > Cascardo.
> > 
> >>
> >> I'm thinking that Pingfan's thought may help:
> >>>> I think it could be if/else, no need to call twice.
> >>
> >> Thanks,
> >> Kazu
> >>
> >>>
> >>> Cascardo.
> > 
> 

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] makedumpfile: cope with not-present mem section

2020-01-28 Thread Thadeu Lima de Souza Cascardo 
On Tue, Jan 28, 2020 at 05:03:12PM +, HAGIO KAZUHITO(萩尾 一仁) wrote:
> Hi Cascardo,
> 
> > -Original Message-
> > On Mon, Jan 27, 2020 at 02:04:54PM -0300, Thadeu Lima de Souza Cascardo 
> > wrote:
> > > Sorry for taking too long to respond, as I was on vacation.
> > >
> > > The kernels that had commit 83e3c48729d9, but not commit a0b1280368d1, are
> > > not supported anymore. In a way that it's even hard for me to test them.
> > >
> > > However, I managed to test it, and those two lines are definitively needed
> > > to dump a VM running such a kernel. Is removing them really needed to fix
> > > this issue?
> > >
> > > Otherwise, I would rather keep them.
> > >
> > > Thanks.
> > > Cascardo.
> > 
> > By the way, I was too fast in sending this. We really need to keep those 
> > lines
> > as makedumpfile will fail to dump a 4.4 kernel with this patch as is.
> 
> Is that Ubuntu 4.4 kernel which has 83e3c48729d9 and not a0b1280368d1?
> Could you elaborate on how it fails?

No, it doesn't have either, so my guess is it would fail on upstream 4.4 as
well, so anything that doesn't have 83e3c48729d9.

That's what I get on that 4.4 kernel (4.4.0-171-generic):

# ./makedumpfile /proc/vmcore ../dump
get_mem_section: Could not validate mem_section.
get_mm_sparsemem: Can't get the address of mem_section.

makedumpfile Failed.
#

So, now, I have a better grasp of the whole logic, and understand why it fails
with this patch.

So, we need to either interpret the mem_section as a pointer to the array of a
pointer to the pointer to the array. The only case the second option is valid
is when sparse_extreme is on, so we don't even need to check the second case
when it's off.

Then, we check that interpreting it either way is valid. If it's valid in both
interpretations, we can't decide which to use, and will fail. So far, we
haven't seen any case in the field where that would accidentally happen. But in
case it does, we should rather fail to dump and fallback to copying, than
creating a bogus compressed dump.

When this patch is applied, a kernel which does not have 83e3c48729d9, and
thus, has mem_section as a direct pointer to the array, it so happens that we
don't detect the pointer to pointer to the array case as invalid, thus failing
to dump.

The way we validate is that the mem_maps should either have the PRESENT bit or
be NULL. Now, that assumption may be invalid, and we would need to do the
validation some other way. I can test the cases where that assumption is
invalid in a 4.4 kernel and see how to fix this in a satisfactory way.

Going through the code once again, I don't see how the second section of the
patch would be correct by itself too. I will think it through a little more and
see if I can come up with a solution.

Regards.
Cascardo.

> 
> I'm thinking that Pingfan's thought may help:
> >> I think it could be if/else, no need to call twice.
> 
> Thanks,
> Kazu
> 
> > 
> > Cascardo.

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] makedumpfile: cope with not-present mem section

2020-01-27 Thread Thadeu Lima de Souza Cascardo 
On Mon, Jan 27, 2020 at 02:04:54PM -0300, Thadeu Lima de Souza Cascardo wrote:
> Sorry for taking too long to respond, as I was on vacation.
> 
> The kernels that had commit 83e3c48729d9, but not commit a0b1280368d1, are
> not supported anymore. In a way that it's even hard for me to test them.
> 
> However, I managed to test it, and those two lines are definitively needed
> to dump a VM running such a kernel. Is removing them really needed to fix
> this issue?
> 
> Otherwise, I would rather keep them.
> 
> Thanks.
> Cascardo.

By the way, I was too fast in sending this. We really need to keep those lines
as makedumpfile will fail to dump a 4.4 kernel with this patch as is.

Cascardo.

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH] makedumpfile: cope with not-present mem section

2020-01-27 Thread Thadeu Lima de Souza Cascardo 
On Thu, Jan 23, 2020 at 05:07:50PM +, HAGIO KAZUHITO(萩尾 一仁) wrote:
> Hi Pingfan,
> 
> Sorry, I had been busy, then was looking into its history a bit.
> 
> > -Original Message-
> > On 01/20/2020 04:59 PM, Baoquan He wrote:
> > > On 01/20/20 at 09:33am, David Hildenbrand wrote:
> > >>
> > >>
> > >>> Am 20.01.2020 um 03:25 schrieb Pingfan Liu :
> > >>>
> > >>> After kernel commit ba72b4c8cf60 ("mm/sparsemem: support sub-section
> > >>> hotplug"), when hot-removed, section_mem_map is still encoded with 
> > >>> section
> > >>> start pfn, not NULL. This break the current makedumpfile.
> 
> Could you add kernel version which this started and error message,
> if possible, for someone hitting this issue to find the patch.
> 
> > >>>
> > >>> Whatever section_mem_map coding info after hot-removed, it is reliable
> > >>> just to work on SECTION_MARKED_PRESENT bit. Fixing makedumpfile by this
> > >>> way.
> > >>>
> > >>> Signed-off-by: Pingfan Liu 
> > >>> To: kexec@lists.infradead.org
> > >>> Cc: Kazuhito Hagio 
> > >>> Cc: Baoquan He 
> > >>> Cc: David Hildenbrand 
> > >>> Cc: Andrew Morton 
> > >>> Cc: Dan Williams 
> > >>> Cc: Oscar Salvador 
> > >>> Cc: Michal Hocko 
> > >>> Cc: Qian Cai 
> > >>> ---
> > >>> makedumpfile.c | 6 +-
> > >>> 1 file changed, 1 insertion(+), 5 deletions(-)
> > >>>
> > >>> diff --git a/makedumpfile.c b/makedumpfile.c
> > >>> index e290fbd..ab40a58 100644
> > >>> --- a/makedumpfile.c
> > >>> +++ b/makedumpfile.c
> > >>> @@ -3406,8 +3406,6 @@ section_mem_map_addr(unsigned long addr, unsigned 
> > >>> long *map_mask)
> > >>>map = ULONG(mem_section + OFFSET(mem_section.section_mem_map));
> > >>>mask = SECTION_MAP_MASK;
> > >>>*map_mask = map & ~mask;
> > >>> -if (map == 0x0)
> > >>> -*map_mask |= SECTION_MARKED_PRESENT;
> > This should be a trick to let map==0x0 survive the logic
> > if (!(map_mask & SECTION_MARKED_PRESENT)) {
> > return FALSE;
> > }
> > in validate_mem_section().
> > >>
> > >> Why was that added in the first place? This looks like dome compat 
> > >> handling to me. Are we sure we can
> > remove it?
> > The logic introduced by commit 14876c45aef ("[PATCH makedumpfile] handle
> > mem_section as either a pointer or an array")
> > Combining section_mem_map_addr() and the following logic in
> > validate_mem_section()
> > if (mem_map == 0) {
> > mem_map = NOT_MEMMAP_ADDR;
> > }
> > 
> > I reached the same conclusion as Baoquan's.
> > >
> > >
> > > The original code assumes that there are only two kinds of mem_section,
> > > one is empty value, the second is a present one. This removing behaves
> > > the same as the old code, I don't see a problem with it.
> > >
> > > I tried to understand the code, but I don't know why it need call
> > > validate_mem_section() twice and compare the returned value.
> > I think it could be if/else, no need to call twice.
> 
> It looks to me that commit 14876c45aef ("[PATCH makedumpfile] handle 
> mem_section
> as either a pointer or an array") was intended to support kernels which had
>   83e3c48729d9 ("mm/sparsemem: Allocate mem_section at runtime for 
> CONFIG_SPARSEMEM_EXTREME=y")
> and did not have
>   a0b1280368d1 ("kdump: write correct address of mem_section into 
> vmcoreinfo").
> 
> Apparently there were some released Ubuntu kernels like this.
> 
> So, if we need that logic, your patch seems
> - support kernels which section_mem_map is non-NULL for hot-removed memory,
> - but might increase the possiblity of false-positive.
> 
> I think this is a tradeoff between the above two, but the latter would be
> small enough.  And I'm testing the patch and OK with 10 vmcores so far.
> 
> > Cc Thadeu Lima de Souza Cascardo, who contributes the original code, and
> > may have some idea about it.
> 
> So if Cascardo doesn't have any objection, I will accept.

Sorry for taking too long to respond, as I was on vacation.

The kernels that had commit 83e3c48729d9, but not commit a0b1280368d1, are
not supported anymore. In a way that it's even hard for me to test them.

However, I managed to test it, and those two lines are definitively needed
to dump a VM running such a kernel. Is removing them really needed to fix
this issue?

Otherwise, I would rather keep them.

Thanks.
Cascardo.

> 
> Thanks,
> Kazu
> 
> P.S. My email address has been changed to k-hagio...@nec.com.
> Please send email to this address in the future.  Thanks.


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH v2] lockdown: allow kexec_file of unsigned images when not under lockdown

2018-11-05 Thread Thadeu Lima de Souza Cascardo 
If CONFIG_KEXEC_VERIFY_SIG is enabled, kexec -s with an unsigned image will
fail requiring an image signed with a trusted key. However, that same
kernel will allow kexec to load and boot a kernel, if kexec_file_load is
not used.

Now, lockdown brings a solution to this inconsistency. However, as it is,
it will still prevent an unsigned image to be loaded with kexec -s when the
system is not under lockdown, while still allowing kexec to work.

At the same time, with lockdown, kexec_file_load would still work when
CONFIG_KEXEC_VERIFY_SIG is disabled.

Signed-off-by: Thadeu Lima de Souza Cascardo 
---

v2:
fixed build failure, s/#elif/#else/

---
 kernel/kexec_file.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index e5bcd94c1efb..b1f0373014c1 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -140,10 +140,17 @@ kimage_file_prepare_segments(struct kimage *image, int 
kernel_fd, int initrd_fd,
   image->kernel_buf_len);
if (ret) {
pr_debug("kernel signature verification failed.\n");
-   goto out;
+   } else {
+   pr_debug("kernel signature verification successful.\n");
}
-   pr_debug("kernel signature verification successful.\n");
+#else
+   ret = -EPERM;
 #endif
+   if (ret && kernel_is_locked_down("kexec of unsigned images"))
+   goto out;
+   else
+   ret = 0;
+
/* It is possible that there no initramfs is being loaded */
if (!(flags & KEXEC_FILE_NO_INITRAMFS)) {
ret = kernel_read_file_from_fd(initrd_fd, >initrd_buf,
-- 
2.19.1


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

[PATCH] lockdown: allow kexec_file of unsigned images when not under lockdown

2018-11-01 Thread Thadeu Lima de Souza Cascardo 
If CONFIG_KEXEC_VERIFY_SIG is enabled, kexec -s with an unsigned image will
fail requiring an image signed with a trusted key. However, that same
kernel will allow kexec to load and boot a kernel, if kexec_file_load is
not used.

Now, lockdown brings a solution to this inconsistency. However, as it is,
it will still prevent an unsigned image to be loaded with kexec -s when the
system is not under lockdown, while still allowing kexec to work.

At the same time, with lockdown, kexec_file_load would still work when
CONFIG_KEXEC_VERIFY_SIG is disabled.

Signed-off-by: Thadeu Lima de Souza Cascardo 
---
 kernel/kexec_file.c | 11 +--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/kernel/kexec_file.c b/kernel/kexec_file.c
index 35cf0ad29718..b64f32fda9ca 100644
--- a/kernel/kexec_file.c
+++ b/kernel/kexec_file.c
@@ -211,10 +211,17 @@ kimage_file_prepare_segments(struct kimage *image, int 
kernel_fd, int initrd_fd,
   image->kernel_buf_len);
if (ret) {
pr_debug("kernel signature verification failed.\n");
-   goto out;
+   } else {
+   pr_debug("kernel signature verification successful.\n");
}
-   pr_debug("kernel signature verification successful.\n");
+#elif
+   ret = -EPERM;
 #endif
+   if (ret && kernel_is_locked_down("kexec of unsigned images"))
+   goto out;
+   else
+   ret = 0;
+
/* It is possible that there no initramfs is being loaded */
if (!(flags & KEXEC_FILE_NO_INITRAMFS)) {
ret = kernel_read_file_from_fd(initrd_fd, >initrd_buf,
-- 
2.19.1


___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH makedumpfile] handle mem_section as either a pointer or an array

2018-05-14 Thread Thadeu Lima de Souza Cascardo 
On Mon, May 14, 2018 at 07:40:21AM +, Masaki Tachibana wrote:
> Hi Thadeu,
> 
> So sorry for the late reply.
> I will merge the patch into V1.6.4 with the following modifying 
> because I have already accepted "Always use bigger SECTION_MAP_MASK" patch.
> 
> 63,68c63,64
> <   if (info->kernel_version < KERNEL_VERSION(4, 13, 0))
> < - map &= SECTION_MAP_MASK_4_12;
> < + mask = SECTION_MAP_MASK_4_12;
> <   else
> < - map &= SECTION_MAP_MASK;
> < + mask = SECTION_MAP_MASK;
> ---
> > - map &= SECTION_MAP_MASK;
> > + mask = SECTION_MAP_MASK;
> 

Ack. That should be enough for a fixup.
Thanks.
Cascardo.

> 
> Thanks
> Tachibana
> 
> > -Original Message-
> > From: kexec [mailto:kexec-boun...@lists.infradead.org] On Behalf Of Masaki 
> > Tachibana
> > Sent: Monday, March 05, 2018 6:16 PM
> > To: Thadeu Lima de Souza Cascardo <casca...@canonical.com>
> > Cc: Hayashi Masahiko() <mas-haya...@tg.jp.nec.com>; 
> > kexec@lists.infradead.org
> > Subject: RE: [PATCH makedumpfile] handle mem_section as either a pointer or 
> > an array
> > 
> > Hi Thadeu,
> > 
> > Sorry for the late reply.
> > "handle mem_section as either a pointer or an array" patch and
> > "Always use bigger SECTION_MAP_MASK" patch modify the same line.
> > I would like to reply about both patches by the end of the next week.
> > 
> > 
> > Thanks
> > Tachibana
> > 
> > 
> > > -Original Message-
> > > From: kexec [mailto:kexec-boun...@lists.infradead.org] On Behalf Of 
> > > Thadeu Lima de Souza Cascardo
> > > Sent: Friday, March 02, 2018 11:33 PM
> > > To: kexec@lists.infradead.org
> > > Subject: Re: [PATCH makedumpfile] handle mem_section as either a pointer 
> > > or an array
> > >
> > > Any comments or reviews on the patch below?
> > >
> > > Thanks.
> > > Cascardo.
> > >
> > > On Mon, Feb 19, 2018 at 05:04:59PM -0300, Thadeu Lima de Souza Cascardo 
> > > wrote:
> > > > Some kernel versions that have been recently shipped have mem_section 
> > > > point to
> > > > a pointer to the array, instead of pointing directly to the array. That 
> > > > only
> > > > happens on SPARSEMEM_EXTREME configurations.
> > > >
> > > > As dwarf information might not be present that would have allowed us to 
> > > > detect
> > > > which type it is, we need to try it either as an array or as the 
> > > > pointer to the
> > > > array. Then, we validate all section_mem_map: they must either be 
> > > > present or
> > > > null. If any problems are found when traversing it, consider it 
> > > > invalid. Only
> > > > one way may be valid. Otherwise, fail.
> > > >
> > > > This has been tested with both types of kernels and succeeded in 
> > > > producing a
> > > > compressed dump that could be analyzed with crash 7.2.1.
> > > >
> > > > Signed-off-by: Thadeu Lima de Souza Cascardo <casca...@canonical.com>
> > > > ---
> > > >  makedumpfile.c | 153 
> > > > +++--
> > > >  makedumpfile.h |   1 +
> > > >  2 files changed, 118 insertions(+), 36 deletions(-)
> > > >
> > > > diff --git a/makedumpfile.c b/makedumpfile.c
> > > > index ed138d3..cd3fa4d 100644
> > > > --- a/makedumpfile.c
> > > > +++ b/makedumpfile.c
> > > > @@ -3297,7 +3297,7 @@ get_mm_discontigmem(void)
> > > > return TRUE;
> > > >  }
> > > >
> > > > -unsigned long
> > > > +static unsigned long
> > > >  nr_to_section(unsigned long nr, unsigned long *mem_sec)
> > > >  {
> > > > unsigned long addr;
> > > > @@ -3311,17 +3311,17 @@ nr_to_section(unsigned long nr, unsigned long 
> > > > *mem_sec)
> > > > addr = SYMBOL(mem_section) + (nr * SIZE(mem_section));
> > > > }
> > > >
> > > > -   if (!is_kvaddr(addr))
> > > > -   return NOT_KV_ADDR;
> > > > -
> > > > return addr;
> > > >  }
> > > >
> > > > -unsigned long
> > > > -section_mem_map_addr(unsigned long ad

Re: [PATCH net-next v2 2/2] cxgb4: collect hardware dump in second kernel

2018-03-24 Thread Thadeu Lima de Souza Cascardo 
On Sat, Mar 24, 2018 at 04:26:34PM +0530, Rahul Lakkireddy wrote:
> Register callback to collect hardware/firmware dumps in second kernel
> before hardware/firmware is initialized.  The dumps for each device
> will be available under /sys/kernel/crashdd/cxgb4/ directory in second
> kernel.
> 
> Signed-off-by: Rahul Lakkireddy 
> Signed-off-by: Ganesh Goudar 
> ---
> v2:
> - No Changes.
> 
> Changes since rfc v2:
> - Update comments and commit message for sysfs change.
> 
> rfc v2:
> - Updated dump registration to the new API in patch 1.
[...]
> diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c 
> b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> index e880be8e3c45..265cb026f868 100644
> --- a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c
> @@ -5527,6 +5527,18 @@ static int init_one(struct pci_dev *pdev, const struct 
> pci_device_id *ent)
>   if (err)
>   goto out_free_adapter;
>  
> + if (is_kdump_kernel()) {
> + /* Collect hardware state and append to
> +  * /sys/kernel/crashdd/cxgb4/ directory
> +  */
> + err = cxgb4_cudbg_crashdd_add_dump(adapter);
> + if (err) {
> + dev_warn(adapter->pdev_dev,
> +  "Fail collecting crash device dump, err: %d. 
> Continuing\n",
> +  err);
> + err = 0;
> + }
> + }
>  

The problem I see with this approach is that you require that the driver
is built into the kdump kernel (or present as a module in the kdump
initramfs), and that you will probe the device during the collection of
the dumps.

IMHO, if you are going to require the device to be probed by the same
driver during kdump, you might just as well use the device object itself
to present the crash data. I think that's what Stephen Hemminger meant
when he said to use sysfs. No need at all for any special crashdd. Just
add an attribute or attribute group to the device object.

Otherwise, as Eric Biederman pointed out, you should just add that data
into the vmcore before you kexec, so you don't even need to look at a
different file, and the driver does not even need to be present in the
kdump kernel.

Cascardo.

>   if (!is_t4(adapter->params.chip)) {
>   s_qpp = (QUEUESPERPAGEPF0_S +
> -- 
> 2.14.1

___
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

Re: [PATCH makedumpfile] handle mem_section as either a pointer or an array

2018-03-02 Thread Thadeu Lima de Souza Cascardo 
Any comments or reviews on the patch below?

Thanks.
Cascardo.

On Mon, Feb 19, 2018 at 05:04:59PM -0300, Thadeu Lima de Souza Cascardo wrote:
> Some kernel versions that have been recently shipped have mem_section point to
> a pointer to the array, instead of pointing directly to the array. That only
> happens on SPARSEMEM_EXTREME configurations.
> 
> As dwarf information might not be present that would have allowed us to detect
> which type it is, we need to try it either as an array or as the pointer to 
> the
> array. Then, we validate all section_mem_map: they must either be present or
> null. If any problems are found when traversing it, consider it invalid. Only
> one way may be valid. Otherwise, fail.
> 
> This has been tested with both types of kernels and succeeded in producing a
> compressed dump that could be analyzed with crash 7.2.1.
> 
> Signed-off-by: Thadeu Lima de Souza Cascardo <casca...@canonical.com>
> ---
>  makedumpfile.c | 153 
> +++--
>  makedumpfile.h |   1 +
>  2 files changed, 118 insertions(+), 36 deletions(-)
> 
> diff --git a/makedumpfile.c b/makedumpfile.c
> index ed138d3..cd3fa4d 100644
> --- a/makedumpfile.c
> +++ b/makedumpfile.c
> @@ -3297,7 +3297,7 @@ get_mm_discontigmem(void)
>   return TRUE;
>  }
>  
> -unsigned long
> +static unsigned long
>  nr_to_section(unsigned long nr, unsigned long *mem_sec)
>  {
>   unsigned long addr;
> @@ -3311,17 +3311,17 @@ nr_to_section(unsigned long nr, unsigned long 
> *mem_sec)
>   addr = SYMBOL(mem_section) + (nr * SIZE(mem_section));
>   }
>  
> - if (!is_kvaddr(addr))
> - return NOT_KV_ADDR;
> -
>   return addr;
>  }
>  
> -unsigned long
> -section_mem_map_addr(unsigned long addr)
> +static unsigned long
> +section_mem_map_addr(unsigned long addr, unsigned long *map_mask)
>  {
>   char *mem_section;
>   unsigned long map;
> + unsigned long mask;
> +
> + *map_mask = 0;
>  
>   if (!is_kvaddr(addr))
>   return NOT_KV_ADDR;
> @@ -3338,15 +3338,19 @@ section_mem_map_addr(unsigned long addr)
>   }
>   map = ULONG(mem_section + OFFSET(mem_section.section_mem_map));
>   if (info->kernel_version < KERNEL_VERSION(4, 13, 0))
> - map &= SECTION_MAP_MASK_4_12;
> + mask = SECTION_MAP_MASK_4_12;
>   else
> - map &= SECTION_MAP_MASK;
> + mask = SECTION_MAP_MASK;
> + *map_mask = map & ~mask;
> + if (map == 0x0)
> + *map_mask |= SECTION_MARKED_PRESENT;
> + map &= mask;
>   free(mem_section);
>  
>   return map;
>  }
>  
> -unsigned long
> +static unsigned long
>  sparse_decode_mem_map(unsigned long coded_mem_map, unsigned long section_nr)
>  {
>   unsigned long mem_map;
> @@ -3354,17 +3358,110 @@ sparse_decode_mem_map(unsigned long coded_mem_map, 
> unsigned long section_nr)
>   mem_map =  coded_mem_map +
>   (SECTION_NR_TO_PFN(section_nr) * SIZE(page));
>  
> - if (!is_kvaddr(mem_map))
> - return NOT_KV_ADDR;
>   return mem_map;
>  }
> +
> +/*
> + * On some kernels, mem_section may be a pointer or an array, when
> + * SPARSEMEM_EXTREME is on.
> + *
> + * We assume that section_mem_map is either 0 or has the present bit set.
> + *
> + */
> +
> +static int
> +validate_mem_section(unsigned long *mem_sec,
> +  unsigned long mem_section_ptr, unsigned int 
> mem_section_size,
> +  unsigned long *mem_maps, unsigned int num_section)
> +{
> + unsigned int section_nr;
> + unsigned long map_mask;
> + unsigned long section, mem_map;
> + if (!readmem(VADDR, mem_section_ptr, mem_sec, mem_section_size)) {
> + ERRMSG("Can't read mem_section array.\n");
> + return FALSE;
> + }
> + for (section_nr = 0; section_nr < num_section; section_nr++) {
> + section = nr_to_section(section_nr, mem_sec);
> + if (section == NOT_KV_ADDR) {
> + mem_map = NOT_MEMMAP_ADDR;
> + } else {
> + mem_map = section_mem_map_addr(section, _mask);
> + if (!(map_mask & SECTION_MARKED_PRESENT)) {
> + return FALSE;
> + }
> + if (mem_map == 0) {
> + mem_map = NOT_MEMMAP_ADDR;
> + } else {
> + mem_map = sparse_decode_mem_map(mem_map,
> +

[PATCH makedumpfile] handle mem_section as either a pointer or an array

2018-02-19 Thread Thadeu Lima de Souza Cascardo 
Some kernel versions that have been recently shipped have mem_section point to
a pointer to the array, instead of pointing directly to the array. That only
happens on SPARSEMEM_EXTREME configurations.

As dwarf information might not be present that would have allowed us to detect
which type it is, we need to try it either as an array or as the pointer to the
array. Then, we validate all section_mem_map: they must either be present or
null. If any problems are found when traversing it, consider it invalid. Only
one way may be valid. Otherwise, fail.

This has been tested with both types of kernels and succeeded in producing a
compressed dump that could be analyzed with crash 7.2.1.

Signed-off-by: Thadeu Lima de Souza Cascardo <casca...@canonical.com>
---
 makedumpfile.c | 153 +++--
 makedumpfile.h |   1 +
 2 files changed, 118 insertions(+), 36 deletions(-)

diff --git a/makedumpfile.c b/makedumpfile.c
index ed138d3..cd3fa4d 100644
--- a/makedumpfile.c
+++ b/makedumpfile.c
@@ -3297,7 +3297,7 @@ get_mm_discontigmem(void)
return TRUE;
 }
 
-unsigned long
+static unsigned long
 nr_to_section(unsigned long nr, unsigned long *mem_sec)
 {
unsigned long addr;
@@ -3311,17 +3311,17 @@ nr_to_section(unsigned long nr, unsigned long *mem_sec)
addr = SYMBOL(mem_section) + (nr * SIZE(mem_section));
}
 
-   if (!is_kvaddr(addr))
-   return NOT_KV_ADDR;
-
return addr;
 }
 
-unsigned long
-section_mem_map_addr(unsigned long addr)
+static unsigned long
+section_mem_map_addr(unsigned long addr, unsigned long *map_mask)
 {
char *mem_section;
unsigned long map;
+   unsigned long mask;
+
+   *map_mask = 0;
 
if (!is_kvaddr(addr))
return NOT_KV_ADDR;
@@ -3338,15 +3338,19 @@ section_mem_map_addr(unsigned long addr)
}
map = ULONG(mem_section + OFFSET(mem_section.section_mem_map));
if (info->kernel_version < KERNEL_VERSION(4, 13, 0))
-   map &= SECTION_MAP_MASK_4_12;
+   mask = SECTION_MAP_MASK_4_12;
else
-   map &= SECTION_MAP_MASK;
+   mask = SECTION_MAP_MASK;
+   *map_mask = map & ~mask;
+   if (map == 0x0)
+   *map_mask |= SECTION_MARKED_PRESENT;
+   map &= mask;
free(mem_section);
 
return map;
 }
 
-unsigned long
+static unsigned long
 sparse_decode_mem_map(unsigned long coded_mem_map, unsigned long section_nr)
 {
unsigned long mem_map;
@@ -3354,17 +3358,110 @@ sparse_decode_mem_map(unsigned long coded_mem_map, 
unsigned long section_nr)
mem_map =  coded_mem_map +
(SECTION_NR_TO_PFN(section_nr) * SIZE(page));
 
-   if (!is_kvaddr(mem_map))
-   return NOT_KV_ADDR;
return mem_map;
 }
+
+/*
+ * On some kernels, mem_section may be a pointer or an array, when
+ * SPARSEMEM_EXTREME is on.
+ *
+ * We assume that section_mem_map is either 0 or has the present bit set.
+ *
+ */
+
+static int
+validate_mem_section(unsigned long *mem_sec,
+unsigned long mem_section_ptr, unsigned int 
mem_section_size,
+unsigned long *mem_maps, unsigned int num_section)
+{
+   unsigned int section_nr;
+   unsigned long map_mask;
+   unsigned long section, mem_map;
+   if (!readmem(VADDR, mem_section_ptr, mem_sec, mem_section_size)) {
+   ERRMSG("Can't read mem_section array.\n");
+   return FALSE;
+   }
+   for (section_nr = 0; section_nr < num_section; section_nr++) {
+   section = nr_to_section(section_nr, mem_sec);
+   if (section == NOT_KV_ADDR) {
+   mem_map = NOT_MEMMAP_ADDR;
+   } else {
+   mem_map = section_mem_map_addr(section, _mask);
+   if (!(map_mask & SECTION_MARKED_PRESENT)) {
+   return FALSE;
+   }
+   if (mem_map == 0) {
+   mem_map = NOT_MEMMAP_ADDR;
+   } else {
+   mem_map = sparse_decode_mem_map(mem_map,
+   section_nr);
+   if (!is_kvaddr(mem_map)) {
+   return FALSE;
+   }
+   }
+   }
+   mem_maps[section_nr] = mem_map;
+   }
+   return TRUE;
+}
+
+static int
+get_mem_section(unsigned int mem_section_size, unsigned long *mem_maps,
+   unsigned int num_section)
+{
+   unsigned long mem_section_ptr;
+   int ret = FALSE;
+   unsigned long *mem_sec = NULL;
+
+   if ((mem_sec = malloc(mem_section_size)) == NULL) {
+   ERRMSG("Can't allocate memory for the mem_section. %s\n",
+