lkml.org 
[lkml]   [2008]   [Apr]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: uaccess: add probe_kernel_write()
On Fri, 18 Apr 2008 18:58:59 GMT
Linux Kernel Mailing List <linux-kernel@vger.kernel.org> wrote:

> Gitweb: http://git.kernel.org/git/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=c33fa9f5609e918824446ef9a75319d4a802f1f4
> Commit: c33fa9f5609e918824446ef9a75319d4a802f1f4
> Parent: 4b119e21d0c66c22e8ca03df05d9de623d0eb50f
> Author: Ingo Molnar <mingo@elte.hu>
> AuthorDate: Thu Apr 17 20:05:36 2008 +0200
> Committer: Ingo Molnar <mingo@elte.hu>
> CommitDate: Thu Apr 17 20:05:36 2008 +0200
>
> uaccess: add probe_kernel_write()
>
> add probe_kernel_read() and probe_kernel_write().
>
> Uninlined and restricted to kernel range memory only, as suggested
> by Linus.
>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> Reviewed-by: Thomas Gleixner <tglx@linutronix.de>
>
> diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
> index 975c963..fec6dec 100644
> --- a/include/linux/uaccess.h
> +++ b/include/linux/uaccess.h
> @@ -84,4 +84,26 @@ static inline unsigned long __copy_from_user_nocache(void *to,
> ret; \
> })
>
> +/*
> + * probe_kernel_read(): safely attempt to read from a location
> + * @dst: pointer to the buffer that shall take the data
> + * @src: address to read from
> + * @size: size of the data chunk
> + *
> + * Safely read from address @src to the buffer at @dst. If a kernel fault
> + * happens, handle that and return -EFAULT.
> + */
> +extern long probe_kernel_read(void *dst, void *src, size_t size);
> +
> +/*
> + * probe_kernel_write(): safely attempt to write to a location
> + * @dst: address to write to
> + * @src: pointer to the data that shall be written
> + * @size: size of the data chunk
> + *
> + * Safely write to address @dst from the buffer at @src. If a kernel fault
> + * happens, handle that and return -EFAULT.
> + */
> +extern long probe_kernel_write(void *dst, void *src, size_t size);

The above comments appear to be kerneldoc but actually aren't. I don't
think there's much point in duplicating the kerneldoc comments in the .h
file as well - people should know by now to go to the definition site to
find the documentation.

> --- /dev/null
> +++ b/mm/maccess.c
> @@ -0,0 +1,49 @@
> +/*
> + * Access kernel memory without faulting.
> + */
> +#include <linux/uaccess.h>
> +#include <linux/module.h>
> +#include <linux/mm.h>
> +
> +/**
> + * probe_kernel_read(): safely attempt to read from a location
> + * @dst: pointer to the buffer that shall take the data
> + * @src: address to read from
> + * @size: size of the data chunk
> + *
> + * Safely read from address @src to the buffer at @dst. If a kernel fault
> + * happens, handle that and return -EFAULT.
> + */
> +long probe_kernel_read(void *dst, void *src, size_t size)
> +{
> + long ret;
> +
> + pagefault_disable();
> + ret = __copy_from_user_inatomic(dst,
> + (__force const void __user *)src, size);
> + pagefault_enable();
> +
> + return ret ? -EFAULT : 0;
> +}
> +EXPORT_SYMBOL_GPL(probe_kernel_read);

I think the documentation should point out that this function can probe both
a user address and a kernel address (if that's right, which I think it is).

And just looking at it, I think the set_fs() in probe_kernel_address() was
always unneeded.

As I pointed out the other day (was apparently ignored) I think we
can/should reimplement probe_kernel_address() to use this. For
compatibility reasons, this will require EXPORT_SYMBOL(), not
EXPORT_SYMBOL_GPL().



All the above observations should of course have been made prior to the
patch begin merged into mainline but afacit it was never sent out for
review?



\
 
 \ /
  Last update: 2008-04-18 23:45    [W:0.027 / U:0.060 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site