linux-kernel - Re: [patch] x86, mm: pass in 'total' to __copy_from_user

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]

Message-ID: <20090225162954.GA22251@elte.hu>
Date:	Wed, 25 Feb 2009 17:29:54 +0100
From:	Ingo Molnar <mingo@...e.hu>
To:	Linus Torvalds <torvalds@...ux-foundation.org>
Cc:	Nick Piggin <nickpiggin@...oo.com.au>,
	Salman Qazi <sqazi@...gle.com>, davem@...emloft.net,
	linux-kernel@...r.kernel.org, Thomas Gleixner <tglx@...utronix.de>,
	"H. Peter Anvin" <hpa@...or.com>, Andi Kleen <andi@...stfloor.org>
Subject: Re: [patch] x86, mm: pass in 'total' to __copy_from_user_*nocache()


* Linus Torvalds <torvalds@...ux-foundation.org> wrote:

> So I do think we should just apply the simple patch. Not make 
> a big deal out of it. We have numbers. We use cached memory 
> copies for everything else. It's always "safe".

ok, 4K is definitely a simple, defensible, calculatable rule.

The only thing that pushes me a bit towards the 'total' tweak is 
that it still is a simple rule and too applies to something the 
user app controls and is intimately familiar with (the total 
size of write()), and it is very cheap to do.

The 64-bit x86 defconfig comparison is:

|
|    text	   data	    bss	    dec	    hex	filename
| 7669425	1138036	 842840	9650301	 93407d	vmlinux.before
| 7669425	1138036	 842840	9650301	 93407d	vmlinux.after
|

md5 shows it's different instructions:

|
|  ec6ba8ae17bdd3ae55062fc59f83fd25  vmlinux.before
|  1f7e9ea67524926cb92b96d5db97f9ff  vmlinux.after
|

i.e. it got almost all of it eliminated at the inlining level, 
and what we have are different but already existing parameters 
being passed in.

There was a single instruction added:

|
|  $ codiff vmlinux.before vmlinux.after
|
|  vmlinux.after:
|   2 functions changed, 6 bytes added, 1 bytes removed, diff: +5
|
|  mm/filemap.c:
|    __iovec_copy_from_user_inatomic |   +6
|    iov_iter_copy_from_user         |   -1
|   2 functions changed, 6 bytes added, 1 bytes removed, diff: +5
|

But hidden in the total size end result by lucky function size 
alignment.

If you are still not convinced i'll remove the two followup 
commits and will keep the first one only.

	Ingo
--
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/