linux-kernel - Re: uaccess: add probe_kernel

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>] [day] [month] [year] [list]

Message-Id: <20080418144230.7a0e2df8.akpm@linux-foundation.org>
Date:	Fri, 18 Apr 2008 14:42:30 -0700
From:	Andrew Morton <akpm@...ux-foundation.org>
To:	Ingo Molnar <mingo@...e.hu>, Thomas Gleixner <tglx@...utronix.de>
Cc:	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: uaccess: add probe_kernel_write()

On Fri, 18 Apr 2008 18:58:59 GMT
Linux Kernel Mailing List <linux-kernel@...r.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@...e.hu>
> AuthorDate: Thu Apr 17 20:05:36 2008 +0200
> Committer:  Ingo Molnar <mingo@...e.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@...e.hu>
>     Reviewed-by: Thomas Gleixner <tglx@...utronix.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?

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