linux-kernel - Re: [PATCH v4] RO/NX protection for loadable kernel modules

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: Windows password security audit tool. GUI, reports in PDF.

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

Date:	Tue, 7 Jul 2009 22:06:40 -0700
From:	Arjan van de Ven <arjan@...radead.org>
To:	Siarhei Liakh <sliakh.lkml@...il.com>
Cc:	linux-kernel@...r.kernel.org,
	linux-security-module@...r.kernel.org,
	James Morris <jmorris@...ei.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Andi Kleen <ak@....de>, Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>, Ingo Molnar <mingo@...e.hu>,
	Rusty Russell <rusty@...tcorp.com.au>
Subject: Re: [PATCH v4] RO/NX protection for loadable kernel modules

On Tue, 7 Jul 2009 20:47:42 -0400
Siarhei Liakh <sliakh.lkml@...il.com> wrote:

> 
> The patch have been developed for Linux 2.6.30 by Siarhei Liakh
> <sliakh.lkml@...il.com> and Xuxian Jiang <jiang@...ncsu.edu>.
> 
> ---
> 
> Signed-off-by: Siarhei Liakh <sliakh.lkml@...il.com>
> Signed-off-by: Xuxian Jiang <jiang@...ncsu.edu>

I like it, you can already put

Acked-by: Arjan van de Ven <arjan@...ux.intel.com> 

there if you want. If you're going to make a v5 then I do have another
suggestion for improvement... (only possible now that the code is very
clean)

> +	/* Set RO for module text and RO-data*/
> +	if (ro_size > 0) {
> +		begin_addr = (unsigned long) base;
> +		end_addr = begin_addr + ro_size;
> +
> +		/*skip last page if end address is not page-aligned*/
> +		if (!IS_ALIGNED(end_addr, PAGE_SIZE))
> +			end_addr = ALIGN(end_addr - PAGE_SIZE,PAGE_SIZE); 
> +
> +		/*Set text RO if there are still pages between begin
> and end*/
> +		if (end_addr > begin_addr) {
> +			pg_count = PFN_DOWN(end_addr - 1) -
> +					PFN_DOWN(begin_addr) + 1;
> +			DEBUGP("  RO: 0x%lx %lu\n", begin_addr,
> pg_count);
> +			set_memory_ro(begin_addr, pg_count);
> +		} else {
> +			DEBUGP("  RO: less than a page, not
> enforcing.\n");
> +		}
> +	} else {
> +		DEBUGP("  RO: section not present.\n");
> +	}

I *think* this can be done as

begin_pfn = PFN_UP( (base);
end_pfn = PFN_DOWN(base + ro_size);

if (end_pfn > begin_pfn)
	set_memory_ro(begin_pfn >> PAGE_SHIFT, end_pfn - begin_pfn);

(note that I think the +1 you have might be buggy)

if you use PFN_UP/PFN_DOWN like this (rounding the start up, rounding the end down),
then your entire "fix alignment" is not needed, the PFN rounding will automatically
take case of this.

similar construct also applies to the NX codepath that follows right after this RO codepath.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/