Skip to content

Commit 672c0ae

Browse files
jbeulichIngo Molnar
jbeulich
authored and
Ingo Molnar
committed
x86/mm: Consider effective protection attributes in W+X check
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. 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). This in particular eliminates the false W+X warning when running under Xen, as commit: 2cc42ba ("x86-64/Xen: eliminate W+X mappings") had 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. Signed-off-by: Jan Beulich <jbeulich@suse.com> Reviewed-by: Juergen Gross <jgross@suse.com> Cc: Alexander Potapenko <glider@google.com> Cc: Andrey Ryabinin <aryabinin@virtuozzo.com> Cc: Andy Lutomirski <luto@kernel.org> Cc: Boris Ostrovsky <boris.ostrovsky@oracle.com> Cc: Borislav Petkov <bp@alien8.de> Cc: Brian Gerst <brgerst@gmail.com> Cc: Denys Vlasenko <dvlasenk@redhat.com> Cc: Dmitry Vyukov <dvyukov@google.com> Cc: H. Peter Anvin <hpa@zytor.com> Cc: Josh Poimboeuf <jpoimboe@redhat.com> Cc: Linus Torvalds <torvalds@linux-foundation.org> Cc: Peter Zijlstra <peterz@infradead.org> Cc: Thomas Gleixner <tglx@linutronix.de> Link: http://lkml.kernel.org/r/5A8FDE8902000078001AABBB@prv-mh.provo.novell.com Signed-off-by: Ingo Molnar <mingo@kernel.org>
1 parent c46dacb commit 672c0ae

File tree

1 file changed

+58
-36
lines changed

1 file changed

+58
-36
lines changed

‎arch/x86/mm/dump_pagetables.c‎

Lines changed: 58 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@
2929
struct pg_state {
3030
int level;
3131
pgprot_t current_prot;
32+
pgprotval_t effective_prot;
3233
unsigned long start_address;
3334
unsigned long current_address;
3435
const struct addr_marker *marker;
@@ -235,9 +236,9 @@ static unsigned long normalize_addr(unsigned long u)
235236
* print what we collected so far.
236237
*/
237238
static void note_page(struct seq_file *m, struct pg_state *st,
238-
pgprot_t new_prot, int level)
239+
pgprot_t new_prot, pgprotval_t new_eff, int level)
239240
{
240-
pgprotval_t prot, cur;
241+
pgprotval_t prot, cur, eff;
241242
static const char units[] = "BKMGTPE";
242243

243244
/*
@@ -247,23 +248,24 @@ static void note_page(struct seq_file *m, struct pg_state *st,
247248
*/
248249
prot = pgprot_val(new_prot);
249250
cur = pgprot_val(st->current_prot);
251+
eff = st->effective_prot;
250252

251253
if (!st->level) {
252254
/* First entry */
253255
st->current_prot = new_prot;
256+
st->effective_prot = new_eff;
254257
st->level = level;
255258
st->marker = address_markers;
256259
st->lines = 0;
257260
pt_dump_seq_printf(m, st->to_dmesg, "---[ %s ]---\n",
258261
st->marker->name);
259-
} else if (prot != cur || level != st->level ||
262+
} else if (prot != cur || new_eff != eff || level != st->level ||
260263
st->current_address >= st->marker[1].start_address) {
261264
const char *unit = units;
262265
unsigned long delta;
263266
int width = sizeof(unsigned long) * 2;
264-
pgprotval_t pr = pgprot_val(st->current_prot);
265267

266-
if (st->check_wx && (pr & _PAGE_RW) && !(pr & _PAGE_NX)) {
268+
if (st->check_wx && (eff & _PAGE_RW) && !(eff & _PAGE_NX)) {
267269
WARN_ONCE(1,
268270
"x86/mm: Found insecure W+X mapping at address %p/%pS\n",
269271
(void *)st->start_address,
@@ -317,21 +319,30 @@ static void note_page(struct seq_file *m, struct pg_state *st,
317319

318320
st->start_address = st->current_address;
319321
st->current_prot = new_prot;
322+
st->effective_prot = new_eff;
320323
st->level = level;
321324
}
322325
}
323326

324-
static void walk_pte_level(struct seq_file *m, struct pg_state *st, pmd_t addr, unsigned long P)
327+
static inline pgprotval_t effective_prot(pgprotval_t prot1, pgprotval_t prot2)
328+
{
329+
return (prot1 & prot2 & (_PAGE_USER | _PAGE_RW)) |
330+
((prot1 | prot2) & _PAGE_NX);
331+
}
332+
333+
static void walk_pte_level(struct seq_file *m, struct pg_state *st, pmd_t addr,
334+
pgprotval_t eff_in, unsigned long P)
325335
{
326336
int i;
327337
pte_t *start;
328-
pgprotval_t prot;
338+
pgprotval_t prot, eff;
329339

330340
start = (pte_t *)pmd_page_vaddr(addr);
331341
for (i = 0; i < PTRS_PER_PTE; i++) {
332342
prot = pte_flags(*start);
343+
eff = effective_prot(eff_in, prot);
333344
st->current_address = normalize_addr(P + i * PTE_LEVEL_MULT);
334-
note_page(m, st, __pgprot(prot), 5);
345+
note_page(m, st, __pgprot(prot), eff, 5);
335346
start++;
336347
}
337348
}
@@ -351,7 +362,7 @@ static inline bool kasan_page_table(struct seq_file *m, struct pg_state *st,
351362
(pgtable_l5_enabled && __pa(pt) == __pa(kasan_zero_p4d)) ||
352363
__pa(pt) == __pa(kasan_zero_pud)) {
353364
pgprotval_t prot = pte_flags(kasan_zero_pte[0]);
354-
note_page(m, st, __pgprot(prot), 5);
365+
note_page(m, st, __pgprot(prot), 0, 5);
355366
return true;
356367
}
357368
return false;
@@ -366,93 +377,99 @@ static inline bool kasan_page_table(struct seq_file *m, struct pg_state *st,
366377

367378
#if PTRS_PER_PMD > 1
368379

369-
static void walk_pmd_level(struct seq_file *m, struct pg_state *st, pud_t addr, unsigned long P)
380+
static void walk_pmd_level(struct seq_file *m, struct pg_state *st, pud_t addr,
381+
pgprotval_t eff_in, unsigned long P)
370382
{
371383
int i;
372384
pmd_t *start, *pmd_start;
373-
pgprotval_t prot;
385+
pgprotval_t prot, eff;
374386

375387
pmd_start = start = (pmd_t *)pud_page_vaddr(addr);
376388
for (i = 0; i < PTRS_PER_PMD; i++) {
377389
st->current_address = normalize_addr(P + i * PMD_LEVEL_MULT);
378390
if (!pmd_none(*start)) {
391+
prot = pmd_flags(*start);
392+
eff = effective_prot(eff_in, prot);
379393
if (pmd_large(*start) || !pmd_present(*start)) {
380-
prot = pmd_flags(*start);
381-
note_page(m, st, __pgprot(prot), 4);
394+
note_page(m, st, __pgprot(prot), eff, 4);
382395
} else if (!kasan_page_table(m, st, pmd_start)) {
383-
walk_pte_level(m, st, *start,
396+
walk_pte_level(m, st, *start, eff,
384397
P + i * PMD_LEVEL_MULT);
385398
}
386399
} else
387-
note_page(m, st, __pgprot(0), 4);
400+
note_page(m, st, __pgprot(0), 0, 4);
388401
start++;
389402
}
390403
}
391404

392405
#else
393-
#define walk_pmd_level(m,s,a,p) walk_pte_level(m,s,__pmd(pud_val(a)),p)
406+
#define walk_pmd_level(m,s,a,e,p) walk_pte_level(m,s,__pmd(pud_val(a)),e,p)
394407
#define pud_large(a) pmd_large(__pmd(pud_val(a)))
395408
#define pud_none(a) pmd_none(__pmd(pud_val(a)))
396409
#endif
397410

398411
#if PTRS_PER_PUD > 1
399412

400-
static void walk_pud_level(struct seq_file *m, struct pg_state *st, p4d_t addr, unsigned long P)
413+
static void walk_pud_level(struct seq_file *m, struct pg_state *st, p4d_t addr,
414+
pgprotval_t eff_in, unsigned long P)
401415
{
402416
int i;
403417
pud_t *start, *pud_start;
404-
pgprotval_t prot;
418+
pgprotval_t prot, eff;
405419
pud_t *prev_pud = NULL;
406420

407421
pud_start = start = (pud_t *)p4d_page_vaddr(addr);
408422

409423
for (i = 0; i < PTRS_PER_PUD; i++) {
410424
st->current_address = normalize_addr(P + i * PUD_LEVEL_MULT);
411425
if (!pud_none(*start)) {
426+
prot = pud_flags(*start);
427+
eff = effective_prot(eff_in, prot);
412428
if (pud_large(*start) || !pud_present(*start)) {
413-
prot = pud_flags(*start);
414-
note_page(m, st, __pgprot(prot), 3);
429+
note_page(m, st, __pgprot(prot), eff, 3);
415430
} else if (!kasan_page_table(m, st, pud_start)) {
416-
walk_pmd_level(m, st, *start,
431+
walk_pmd_level(m, st, *start, eff,
417432
P + i * PUD_LEVEL_MULT);
418433
}
419434
} else
420-
note_page(m, st, __pgprot(0), 3);
435+
note_page(m, st, __pgprot(0), 0, 3);
421436

422437
prev_pud = start;
423438
start++;
424439
}
425440
}
426441

427442
#else
428-
#define walk_pud_level(m,s,a,p) walk_pmd_level(m,s,__pud(p4d_val(a)),p)
443+
#define walk_pud_level(m,s,a,e,p) walk_pmd_level(m,s,__pud(p4d_val(a)),e,p)
429444
#define p4d_large(a) pud_large(__pud(p4d_val(a)))
430445
#define p4d_none(a) pud_none(__pud(p4d_val(a)))
431446
#endif
432447

433-
static void walk_p4d_level(struct seq_file *m, struct pg_state *st, pgd_t addr, unsigned long P)
448+
static void walk_p4d_level(struct seq_file *m, struct pg_state *st, pgd_t addr,
449+
pgprotval_t eff_in, unsigned long P)
434450
{
435451
int i;
436452
p4d_t *start, *p4d_start;
437-
pgprotval_t prot;
453+
pgprotval_t prot, eff;
438454

439455
if (PTRS_PER_P4D == 1)
440-
return walk_pud_level(m, st, __p4d(pgd_val(addr)), P);
456+
return walk_pud_level(m, st, __p4d(pgd_val(addr)), eff_in, P);
441457

442458
p4d_start = start = (p4d_t *)pgd_page_vaddr(addr);
443459

444460
for (i = 0; i < PTRS_PER_P4D; i++) {
445461
st->current_address = normalize_addr(P + i * P4D_LEVEL_MULT);
446462
if (!p4d_none(*start)) {
463+
prot = p4d_flags(*start);
464+
eff = effective_prot(eff_in, prot);
447465
if (p4d_large(*start) || !p4d_present(*start)) {
448-
prot = p4d_flags(*start);
449-
note_page(m, st, __pgprot(prot), 2);
466+
note_page(m, st, __pgprot(prot), eff, 2);
450467
} else if (!kasan_page_table(m, st, p4d_start)) {
451-
walk_pud_level(m, st, *start,
468+
walk_pud_level(m, st, *start, eff,
452469
P + i * P4D_LEVEL_MULT);
453470
}
454471
} else
455-
note_page(m, st, __pgprot(0), 2);
472+
note_page(m, st, __pgprot(0), 0, 2);
456473

457474
start++;
458475
}
@@ -483,7 +500,7 @@ static void ptdump_walk_pgd_level_core(struct seq_file *m, pgd_t *pgd,
483500
#else
484501
pgd_t *start = swapper_pg_dir;
485502
#endif
486-
pgprotval_t prot;
503+
pgprotval_t prot, eff;
487504
int i;
488505
struct pg_state st = {};
489506

@@ -499,23 +516,28 @@ static void ptdump_walk_pgd_level_core(struct seq_file *m, pgd_t *pgd,
499516
for (i = 0; i < PTRS_PER_PGD; i++) {
500517
st.current_address = normalize_addr(i * PGD_LEVEL_MULT);
501518
if (!pgd_none(*start) && !is_hypervisor_range(i)) {
519+
prot = pgd_flags(*start);
520+
#ifdef CONFIG_X86_PAE
521+
eff = _PAGE_USER | _PAGE_RW;
522+
#else
523+
eff = prot;
524+
#endif
502525
if (pgd_large(*start) || !pgd_present(*start)) {
503-
prot = pgd_flags(*start);
504-
note_page(m, &st, __pgprot(prot), 1);
526+
note_page(m, &st, __pgprot(prot), eff, 1);
505527
} else {
506-
walk_p4d_level(m, &st, *start,
528+
walk_p4d_level(m, &st, *start, eff,
507529
i * PGD_LEVEL_MULT);
508530
}
509531
} else
510-
note_page(m, &st, __pgprot(0), 1);
532+
note_page(m, &st, __pgprot(0), 0, 1);
511533

512534
cond_resched();
513535
start++;
514536
}
515537

516538
/* Flush out the last page */
517539
st.current_address = normalize_addr(PTRS_PER_PGD*PGD_LEVEL_MULT);
518-
note_page(m, &st, __pgprot(0), 0);
540+
note_page(m, &st, __pgprot(0), 0, 0);
519541
if (!checkwx)
520542
return;
521543
if (st.wx_pages)

0 commit comments

Comments
 (0)