[PATCH] uaccess: Comment that copy to/from inatomic requires page fault disabled

[PATCH] uaccess: Comment that copy to/from inatomic requires page fault disabled

[Steven Rostedt]

From: Steven Rostedt <rostedt@goodmis.org>

The functions __copy_from_user_inatomic() and __copy_to_user_inatomic()
both require that either the user space memory is pinned, or that page
faults are disabled when they are called. If page faults are not disabled,
and the memory is not present, the fault handling of reading or writing to
that memory may cause the kernel to schedule. That would be bad in an
atomic context.

Link: https://lore.kernel.org/all/20250819105152.2766363-1-luogengkun@huaweicloud.com/

Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
 include/linux/uaccess.h | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index 1beb5b395d81..add99fa9b656 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -86,6 +86,12 @@
  * as usual) and both source and destination can trigger faults.
  */
 
+/*
+ * __copy_from_user_inatomic() is safe to use in an atomic context but
+ * the user space memory must either be pinned in memory, or page faults
+ * must be disabled, otherwise the page fault handling may cause the function
+ * to schedule.
+ */
 static __always_inline __must_check unsigned long
 __copy_from_user_inatomic(void *to, const void __user *from, unsigned long n)
 {
@@ -124,7 +130,8 @@ __copy_from_user(void *to, const void __user *from, unsigned long n)
  * Copy data from kernel space to user space.  Caller must check
  * the specified block with access_ok() before calling this function.
  * The caller should also make sure he pins the user space address
- * so that we don't result in page fault and sleep.
+ * or call page_fault_disable() so that we don't result in a page fault
+ * and sleep.
  */
 static __always_inline __must_check unsigned long
 __copy_to_user_inatomic(void __user *to, const void *from, unsigned long n)
-- 
2.50.1

Re: [PATCH] uaccess: Comment that copy to/from inatomic requires page fault disabled

[Masami Hiramatsu]

On Wed, 10 Sep 2025 16:18:20 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> From: Steven Rostedt <rostedt@goodmis.org>
> 
> The functions __copy_from_user_inatomic() and __copy_to_user_inatomic()
> both require that either the user space memory is pinned, or that page
> faults are disabled when they are called. If page faults are not disabled,
> and the memory is not present, the fault handling of reading or writing to
> that memory may cause the kernel to schedule. That would be bad in an
> atomic context.
> 
> Link: https://lore.kernel.org/all/20250819105152.2766363-1-luogengkun@huaweicloud.com/
> 

Looks good to me.

Reviewed-by: Masami Hiramatsu (Google) <mhiramat@kernel.org>

Thanks,


> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
>  include/linux/uaccess.h | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
> index 1beb5b395d81..add99fa9b656 100644
> --- a/include/linux/uaccess.h
> +++ b/include/linux/uaccess.h
> @@ -86,6 +86,12 @@
>   * as usual) and both source and destination can trigger faults.
>   */
>  
> +/*
> + * __copy_from_user_inatomic() is safe to use in an atomic context but
> + * the user space memory must either be pinned in memory, or page faults
> + * must be disabled, otherwise the page fault handling may cause the function
> + * to schedule.
> + */
>  static __always_inline __must_check unsigned long
>  __copy_from_user_inatomic(void *to, const void __user *from, unsigned long n)
>  {
> @@ -124,7 +130,8 @@ __copy_from_user(void *to, const void __user *from, unsigned long n)
>   * Copy data from kernel space to user space.  Caller must check
>   * the specified block with access_ok() before calling this function.
>   * The caller should also make sure he pins the user space address
> - * so that we don't result in page fault and sleep.
> + * or call page_fault_disable() so that we don't result in a page fault
> + * and sleep.
>   */
>  static __always_inline __must_check unsigned long
>  __copy_to_user_inatomic(void __user *to, const void *from, unsigned long n)
> -- 
> 2.50.1
> 
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>

Re: [PATCH] uaccess: Comment that copy to/from inatomic requires page fault disabled

[David Laight]

On Wed, 10 Sep 2025 16:18:20 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> From: Steven Rostedt <rostedt@goodmis.org>
> 
> The functions __copy_from_user_inatomic() and __copy_to_user_inatomic()
> both require that either the user space memory is pinned, or that page
> faults are disabled when they are called. If page faults are not disabled,
> and the memory is not present, the fault handling of reading or writing to
> that memory may cause the kernel to schedule. That would be bad in an
> atomic context.
> 
> Link: https://lore.kernel.org/all/20250819105152.2766363-1-luogengkun@huaweicloud.com/
> 
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
>  include/linux/uaccess.h | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
> index 1beb5b395d81..add99fa9b656 100644
> --- a/include/linux/uaccess.h
> +++ b/include/linux/uaccess.h
> @@ -86,6 +86,12 @@
>   * as usual) and both source and destination can trigger faults.
>   */
>  
> +/*
> + * __copy_from_user_inatomic() is safe to use in an atomic context but
> + * the user space memory must either be pinned in memory, or page faults
> + * must be disabled, otherwise the page fault handling may cause the function
> + * to schedule.
> + */
>  static __always_inline __must_check unsigned long
>  __copy_from_user_inatomic(void *to, const void __user *from, unsigned long n)
>  {
> @@ -124,7 +130,8 @@ __copy_from_user(void *to, const void __user *from, unsigned long n)
>   * Copy data from kernel space to user space.  Caller must check
>   * the specified block with access_ok() before calling this function.
>   * The caller should also make sure he pins the user space address
> - * so that we don't result in page fault and sleep.
> + * or call page_fault_disable() so that we don't result in a page fault
> + * and sleep.

It is worse than that - it must avoid a COW fault as well.
I suspect the comment should really be that these are not the functions you
are looking for, you probably want the 'nofault' variants.

Even if the code thinks it has pinned the user buffer it has to be better
to use the 'nofault' variant.

The only exception might be in code that already has page faults disabled.
But even then it would have to be pretty performance critical for normal code.

	David  


>   */
>  static __always_inline __must_check unsigned long
>  __copy_to_user_inatomic(void __user *to, const void *from, unsigned long n)

Re: [PATCH] uaccess: Comment that copy to/from inatomic requires page fault disabled

[Steven Rostedt]

On Mon, 15 Sep 2025 16:43:13 +0100
David Laight <david.laight.linux@gmail.com> wrote:

> > @@ -124,7 +130,8 @@ __copy_from_user(void *to, const void __user *from, unsigned long n)
> >   * Copy data from kernel space to user space.  Caller must check
> >   * the specified block with access_ok() before calling this function.
> >   * The caller should also make sure he pins the user space address
> > - * so that we don't result in page fault and sleep.
> > + * or call page_fault_disable() so that we don't result in a page fault
> > + * and sleep.  
> 
> It is worse than that - it must avoid a COW fault as well.
> I suspect the comment should really be that these are not the functions you
> are looking for, you probably want the 'nofault' variants.
> 
> Even if the code thinks it has pinned the user buffer it has to be better
> to use the 'nofault' variant.
> 
> The only exception might be in code that already has page faults disabled.
> But even then it would have to be pretty performance critical for normal code.

OK, perhaps just change the comments to state that this is an internal
version and should be avoided unless you know what you are doing. Otherwise
use the _nofault() versions.

-- Steve