Messages in this thread | | | Date | Mon, 21 Apr 2008 16:00:59 +0200 | From | Ingo Molnar <> | Subject | Re: kgdb: fix optional arch functions and probe_kernel_* |
| |
* Andrew Morton <akpm@linux-foundation.org> wrote:
> On Fri, 18 Apr 2008 17:42:54 GMT > Linux Kernel Mailing List <linux-kernel@vger.kernel.org> wrote: > > > --- a/mm/maccess.c > > +++ b/mm/maccess.c > > @@ -17,11 +17,14 @@ > > long probe_kernel_read(void *dst, void *src, size_t size) > > { > > long ret; > > + mm_segment_t old_fs = get_fs(); > > > > + set_fs(KERNEL_DS); > > pagefault_disable(); > > ret = __copy_from_user_inatomic(dst, > > (__force const void __user *)src, size); > > pagefault_enable(); > > + set_fs(old_fs); > > > > return ret ? -EFAULT : 0; > > } > > Oh. Well that rather invalidates my earlier comments. It looks like > this change could have been folded, but I understand that this > sometimes gets wearisome and isn't terribly important if > > a) the fix doesn't repair build breakage and > > b) the fix doesn't fix runtime breakage and > > c) the fix fixes code which the git-bisect user won't have enabled in > config anyway.
yeah. I mentioned it in the pull request that i kept the fixes apart to demonstrate the overall fix dynamics of the KGDB tree over a full kernel cycle. I normally backmerge and create a clean queue - but that creates a false perception that the tree is 'too fresh' and trust is harder to be expressed.
> Still. Do we need the set_fs() in there? __copy_from_user_inatomic() > is a "__" uaccess function and hence shouldn't be running access_ok()?
yeah, i guess that's true. Jason?
Ingo
| |