From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Sun, 6 Feb 2005 11:38:14 +0100 From: Andi Kleen Subject: Re: [patch 19/24] TASK_SIZE is variable. Message-ID: <20050206103814.GA4291@wotan.suse.de> References: <200502050150.j151osl11380@mail.osdl.org> <20050205065452.GA32565@wotan.suse.de> <20050204231816.76b989a6.akpm@osdl.org> <20050205074025.GA23814@wotan.suse.de> <20050205152752.671e7a72.davem@davemloft.net> Mime-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20050205152752.671e7a72.davem@davemloft.net> To: "David S. Miller" Cc: Andi Kleen , akpm@osdl.org, torvalds@osdl.org, dwmw2@infradead.org, linux-arch@vger.kernel.org, mingo@elte.hu List-ID: > int compat_sys_foo(compat_uptr_t u_buf, compat_uptr_t u_ret_val) > { > const char __user *buf = compat_ptr(u_buf); > unsigned long k_val; > mm_segment_t old_fs = get_fs(); > int err; > > set_fs(KERNEL_DS); > err = sys_foo(buf, (unsigned long __user *) &k_val); > ... > > This does not fault on x86_64, but it does on platforms like sparc64. Actually it faults on UML/x86-64 too :) > Even though it doesn't fault on x86_64, it's a security hole because it > allows the user to pass in kernel addresses, and such kernel addresses > will just work since we're in KERNEL_DS. the caller just has to verify_area() everything. Not doing that would be a security hole yes. One of the reason I think it's a good idea to discourage because driver writers often get this detail wrong. But even with all that compat code going away set_fs will stay: there are places like network IO in kernel etc. where there is just no way around it. > If set_fs() updated some mm->max_addr thing, access_ok() and friends > would trap things like this in software even on x86_64. Therefore, > I think if anything it's a very good bug check. Hmm, I don't see what change it would make. Currently in KERNEL_DS access_ok is always true. I don't see how you can change this without breaking everything? In theory you could make it check for user space addresses and then fail on i386/x86-64 ((addr) >= TASK_SIZE), but that would bloat the code generated by this common macro a lot and it's probably not worth it. But mm->max_addr wouldn't help you with this at all, you would need a new mm->min_addr which I didn't think anybody was proposing. -Andi