Re: [PATCH RESEND] x86: consider effective protection attributes in W+X check

[Jan Beulich]

>>> On 20.02.18 at 11:17,  wrote:
> I assume the Xen fix got merged meanwhile?

Yes (that's the commit I've referred to in an earlier reply).

Jan

Re: [PATCH RESEND] x86: consider effective protection attributes in W+X check

[Jan Beulich]

>>> On 20.02.18 at 11:17,  wrote:
> I assume the Xen fix got merged meanwhile?

Yes (that's the commit I've referred to in an earlier reply).

Jan

Re: [PATCH RESEND] x86: consider effective protection attributes in W+X check

[Ingo Molnar]

* Jan Beulich  wrote:

> >>> On 20.02.18 at 09:37,  wrote:
> 
> > * Jan Beulich  wrote:
> > 
> >> I'll see what I can do; it's a pity that the change here, which had
> >> been sent weeks ago and is a bug fix, hadn't gone in before that
> >> other change (being more an improvement than a bug fix).
> > 
> > When was it submitted, got a link or Message-ID of the previous submission?
> 
> On Dec 12th (https://patchwork.kernel.org/patch/10106593/).

Indeed! There was a bit of a back and forth in the submission, with v2 and v3 
patches sent for the #1 patch, which combined with the whole Meltdown/PTI mess 
that was in overdrive during the holliday season just made us miss this...

Next time anything like this happens just ping the original thread and I'll 
pick 
it up. (But a resend is fine too, of course.)

I assume the Xen fix got merged meanwhile?

Thanks,

Ingo

Re: [PATCH RESEND] x86: consider effective protection attributes in W+X check

[Ingo Molnar]

* Jan Beulich  wrote:

> >>> On 20.02.18 at 09:37,  wrote:
> 
> > * Jan Beulich  wrote:
> > 
> >> I'll see what I can do; it's a pity that the change here, which had
> >> been sent weeks ago and is a bug fix, hadn't gone in before that
> >> other change (being more an improvement than a bug fix).
> > 
> > When was it submitted, got a link or Message-ID of the previous submission?
> 
> On Dec 12th (https://patchwork.kernel.org/patch/10106593/).

Indeed! There was a bit of a back and forth in the submission, with v2 and v3 
patches sent for the #1 patch, which combined with the whole Meltdown/PTI mess 
that was in overdrive during the holliday season just made us miss this...

Next time anything like this happens just ping the original thread and I'll 
pick 
it up. (But a resend is fine too, of course.)

I assume the Xen fix got merged meanwhile?

Thanks,

Ingo

Re: [PATCH RESEND] x86: consider effective protection attributes in W+X check

[Jan Beulich]

>>> On 20.02.18 at 09:37,  wrote:

> * Jan Beulich  wrote:
> 
>> I'll see what I can do; it's a pity that the change here, which had
>> been sent weeks ago and is a bug fix, hadn't gone in before that
>> other change (being more an improvement than a bug fix).
> 
> When was it submitted, got a link or Message-ID of the previous submission?

On Dec 12th (https://patchwork.kernel.org/patch/10106593/).

Jan

Re: [PATCH RESEND] x86: consider effective protection attributes in W+X check

[Jan Beulich]

>>> On 20.02.18 at 09:37,  wrote:

> * Jan Beulich  wrote:
> 
>> I'll see what I can do; it's a pity that the change here, which had
>> been sent weeks ago and is a bug fix, hadn't gone in before that
>> other change (being more an improvement than a bug fix).
> 
> When was it submitted, got a link or Message-ID of the previous submission?

On Dec 12th (https://patchwork.kernel.org/patch/10106593/).

Jan

Re: [PATCH RESEND] x86: consider effective protection attributes in W+X check

[Ingo Molnar]

* Jan Beulich  wrote:

> I'll see what I can do; it's a pity that the change here, which had
> been sent weeks ago and is a bug fix, hadn't gone in before that
> other change (being more an improvement than a bug fix).

When was it submitted, got a link or Message-ID of the previous submission?

Thanks,

Ingo

Re: [PATCH RESEND] x86: consider effective protection attributes in W+X check

[Ingo Molnar]

* Jan Beulich  wrote:

> I'll see what I can do; it's a pity that the change here, which had
> been sent weeks ago and is a bug fix, hadn't gone in before that
> other change (being more an improvement than a bug fix).

When was it submitted, got a link or Message-ID of the previous submission?

Thanks,

Ingo

Re: [PATCH RESEND] x86: consider effective protection attributes in W+X check

[Ingo Molnar]

* Jan Beulich  wrote:

> >>> On 20.02.18 at 09:10,  wrote:
> > * Jan Beulich  wrote:
> >> Using just the leaf page table entry flags would cause a false warning
> >> in case _PAGE_RW is clear or _PAGE_NX is set in a higher level entry.
> > 
> > Under what circumstances did you see false positive warnings?
> 
> As explained in the 2-patch series this was originally part of, there
> continues to be that W+X warning when running under Xen, as
> commit 2cc42bac1c ("x86-64/Xen: eliminate W+X mappings") has
> to make the necessary adjustment in L2 rather than L1 (the
> reason is explained there). I.e. _PAGE_RW is clear there in L1,
> but _PAGE_NX is set in L2.

This would make an excellent additional paragraph of the v2 changelog.

Thanks,

Ingo

Re: [PATCH RESEND] x86: consider effective protection attributes in W+X check

[Ingo Molnar]

* Jan Beulich  wrote:

> >>> On 20.02.18 at 09:10,  wrote:
> > * Jan Beulich  wrote:
> >> Using just the leaf page table entry flags would cause a false warning
> >> in case _PAGE_RW is clear or _PAGE_NX is set in a higher level entry.
> > 
> > Under what circumstances did you see false positive warnings?
> 
> As explained in the 2-patch series this was originally part of, there
> continues to be that W+X warning when running under Xen, as
> commit 2cc42bac1c ("x86-64/Xen: eliminate W+X mappings") has
> to make the necessary adjustment in L2 rather than L1 (the
> reason is explained there). I.e. _PAGE_RW is clear there in L1,
> but _PAGE_NX is set in L2.

This would make an excellent additional paragraph of the v2 changelog.

Thanks,

Ingo

Re: [PATCH RESEND] x86: consider effective protection attributes in W+X check

[Jan Beulich]

>>> On 20.02.18 at 09:10,  wrote:
> * Jan Beulich  wrote:
>> Using just the leaf page table entry flags would cause a false warning
>> in case _PAGE_RW is clear or _PAGE_NX is set in a higher level entry.
> 
> Under what circumstances did you see false positive warnings?

As explained in the 2-patch series this was originally part of, there
continues to be that W+X warning when running under Xen, as
commit 2cc42bac1c ("x86-64/Xen: eliminate W+X mappings") has
to make the necessary adjustment in L2 rather than L1 (the
reason is explained there). I.e. _PAGE_RW is clear there in L1,
but _PAGE_NX is set in L2.

>> Hand through both the current entry's flags as well as the accumulated
>> effective value (the latter as pgprotval_t instead of pgprot_t, as it's
>> not an actual entry's value).
>> 
>> Signed-off-by: Jan Beulich 
>> Reviewed-by: Juergen Gross 
>> ---
>>  arch/x86/mm/dump_pagetables.c |   92 
>> ++
>>  1 file changed, 57 insertions(+), 35 deletions(-)
> 
> Could you please rebase this on top of latest tip:master, which changed 
> arch/x86/mm/dump_pagetables.c non-trivially due the dynamic 5 level paging 
> changes?

I'll see what I can do; it's a pity that the change here, which had
been sent weeks ago and is a bug fix, hadn't gone in before that
other change (being more an improvement than a bug fix). At
least it doesn't look like the re-basing would be very difficult.

Jan

Re: [PATCH RESEND] x86: consider effective protection attributes in W+X check

[Jan Beulich]

>>> On 20.02.18 at 09:10,  wrote:
> * Jan Beulich  wrote:
>> Using just the leaf page table entry flags would cause a false warning
>> in case _PAGE_RW is clear or _PAGE_NX is set in a higher level entry.
> 
> Under what circumstances did you see false positive warnings?

As explained in the 2-patch series this was originally part of, there
continues to be that W+X warning when running under Xen, as
commit 2cc42bac1c ("x86-64/Xen: eliminate W+X mappings") has
to make the necessary adjustment in L2 rather than L1 (the
reason is explained there). I.e. _PAGE_RW is clear there in L1,
but _PAGE_NX is set in L2.

>> Hand through both the current entry's flags as well as the accumulated
>> effective value (the latter as pgprotval_t instead of pgprot_t, as it's
>> not an actual entry's value).
>> 
>> Signed-off-by: Jan Beulich 
>> Reviewed-by: Juergen Gross 
>> ---
>>  arch/x86/mm/dump_pagetables.c |   92 
>> ++
>>  1 file changed, 57 insertions(+), 35 deletions(-)
> 
> Could you please rebase this on top of latest tip:master, which changed 
> arch/x86/mm/dump_pagetables.c non-trivially due the dynamic 5 level paging 
> changes?

I'll see what I can do; it's a pity that the change here, which had
been sent weeks ago and is a bug fix, hadn't gone in before that
other change (being more an improvement than a bug fix). At
least it doesn't look like the re-basing would be very difficult.

Jan

Re: [PATCH RESEND] x86: consider effective protection attributes in W+X check

[Ingo Molnar]

* Jan Beulich  wrote:

> Using just the leaf page table entry flags would cause a false warning
> in case _PAGE_RW is clear or _PAGE_NX is set in a higher level entry.

Under what circumstances did you see false positive warnings?

> Hand through both the current entry's flags as well as the accumulated
> effective value (the latter as pgprotval_t instead of pgprot_t, as it's
> not an actual entry's value).
> 
> Signed-off-by: Jan Beulich 
> Reviewed-by: Juergen Gross 
> ---
>  arch/x86/mm/dump_pagetables.c |   92 
> ++
>  1 file changed, 57 insertions(+), 35 deletions(-)

Could you please rebase this on top of latest tip:master, which changed 
arch/x86/mm/dump_pagetables.c non-trivially due the dynamic 5 level paging 
changes?

Thanks,

Ingo

Re: [PATCH RESEND] x86: consider effective protection attributes in W+X check

[Ingo Molnar]

* Jan Beulich  wrote:

> Using just the leaf page table entry flags would cause a false warning
> in case _PAGE_RW is clear or _PAGE_NX is set in a higher level entry.

Under what circumstances did you see false positive warnings?

> Hand through both the current entry's flags as well as the accumulated
> effective value (the latter as pgprotval_t instead of pgprot_t, as it's
> not an actual entry's value).
> 
> Signed-off-by: Jan Beulich 
> Reviewed-by: Juergen Gross 
> ---
>  arch/x86/mm/dump_pagetables.c |   92 
> ++
>  1 file changed, 57 insertions(+), 35 deletions(-)

Could you please rebase this on top of latest tip:master, which changed 
arch/x86/mm/dump_pagetables.c non-trivially due the dynamic 5 level paging 
changes?

Thanks,

Ingo