Re: [tip:x86/mm] x86/mm: Consider effective protection attributes in W+X check

[Andrey Ryabinin]

On 02/26/2018 11:48 AM, tip-bot for Jan Beulich wrote:

>  static void note_page(struct seq_file *m, struct pg_state *st,
> -		      pgprot_t new_prot, int level)
> +		      pgprot_t new_prot, pgprotval_t new_eff, int level)
>  {
> -	pgprotval_t prot, cur;
> +	pgprotval_t prot, cur, eff;
>  	static const char units[] = "BKMGTPE";
>  
>  	/*
> @@ -247,23 +248,24 @@ static void note_page(struct seq_file *m, struct pg_state *st,
>  	 */
>  	prot = pgprot_val(new_prot);
>  	cur = pgprot_val(st->current_prot);
> +	eff = st->effective_prot;
>  
>  	if (!st->level) {
>  		/* First entry */
>  		st->current_prot = new_prot;
> +		st->effective_prot = new_eff;
>  		st->level = level;
>  		st->marker = address_markers;
>  		st->lines = 0;
>  		pt_dump_seq_printf(m, st->to_dmesg, "---[ %s ]---\n",
>  				   st->marker->name);
> -	} else if (prot != cur || level != st->level ||
> +	} else if (prot != cur || new_eff != eff || level != st->level ||
>  		   st->current_address >= st->marker[1].start_address) {
>  		const char *unit = units;
>  		unsigned long delta;
>  		int width = sizeof(unsigned long) * 2;
> -		pgprotval_t pr = pgprot_val(st->current_prot);
>  
> -		if (st->check_wx && (pr & _PAGE_RW) && !(pr & _PAGE_NX)) {
> +		if (st->check_wx && (eff & _PAGE_RW) && !(eff & _PAGE_NX)) {
>  			WARN_ONCE(1,
>  				  "x86/mm: Found insecure W+X mapping at address %p/%pS\n",
>  				  (void *)st->start_address,
> @@ -317,21 +319,30 @@ static void note_page(struct seq_file *m, struct pg_state *st,
>  
>  		st->start_address = st->current_address;
>  		st->current_prot = new_prot;
> +		st->effective_prot = new_eff;
>  		st->level = level;
>  	}
>  }
>  
> -static void walk_pte_level(struct seq_file *m, struct pg_state *st, pmd_t addr, unsigned long P)
> +static inline pgprotval_t effective_prot(pgprotval_t prot1, pgprotval_t prot2)
> +{
> +	return (prot1 & prot2 & (_PAGE_USER | _PAGE_RW)) |
> +	       ((prot1 | prot2) & _PAGE_NX);
> +}
> +
> +static void walk_pte_level(struct seq_file *m, struct pg_state *st, pmd_t addr,
> +			   pgprotval_t eff_in, unsigned long P)
>  {
>  	int i;
>  	pte_t *start;
> -	pgprotval_t prot;
> +	pgprotval_t prot, eff;
>  
>  	start = (pte_t *)pmd_page_vaddr(addr);
>  	for (i = 0; i < PTRS_PER_PTE; i++) {
>  		prot = pte_flags(*start);
> +		eff = effective_prot(eff_in, prot);
>  		st->current_address = normalize_addr(P + i * PTE_LEVEL_MULT);
> -		note_page(m, st, __pgprot(prot), 5);
> +		note_page(m, st, __pgprot(prot), eff, 5);
>  		start++;
>  	}
>  }
> @@ -351,7 +362,7 @@ static inline bool kasan_page_table(struct seq_file *m, struct pg_state *st,
>  	    (pgtable_l5_enabled && __pa(pt) == __pa(kasan_zero_p4d)) ||
>  	    __pa(pt) == __pa(kasan_zero_pud)) {
>  		pgprotval_t prot = pte_flags(kasan_zero_pte[0]);
> -		note_page(m, st, __pgprot(prot), 5);
> +		note_page(m, st, __pgprot(prot), 0, 5);

Isn't this disables W+X check for kasan page table?
Methinks it should be 'prot' here.