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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 26 Feb 2018 13:00:03 +0300
From:   Andrey Ryabinin <aryabinin@...tuozzo.com>
To:     jpoimboe@...hat.com, dvlasenk@...hat.com,
        torvalds@...ux-foundation.org, tglx@...utronix.de, jgross@...e.com,
        peterz@...radead.org, bp@...en8.de, mingo@...nel.org,
        brgerst@...il.com, boris.ostrovsky@...cle.com, jbeulich@...e.com,
        dvyukov@...gle.com, hpa@...or.com, glider@...gle.com,
        luto@...nel.org, linux-kernel@...r.kernel.org,
        linux-tip-commits@...r.kernel.org
Subject: Re: [tip:x86/mm] x86/mm: Consider effective protection attributes in
 W+X check


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.