From: Steven Rostedt <rostedt@goodmis.org>

In order to know if a task is a user thread or a kernel thread it is
recommended to test the task flags for PF_KTHREAD. The old way was to
check if the task mm pointer is NULL.

It is an easy mistake to not test the flag correctly, as:

	if (!(task->flag & PF_KTHREAD))

Is not immediately obvious that it's testing for a user thread.

Add helper functions:

  is_user_thread()
  is_kernel_thread()

that can make seeing what is being tested for much more obvious:

	if (is_user_thread(task))

Link: https://lore.kernel.org/all/20250425133416.63d3e3b8@gandalf.local.home/

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

diff --git a/include/linux/sched.h b/include/linux/sched.h
index f96ac1982893..823f38b0fd3e 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1785,6 +1785,16 @@ static __always_inline bool is_percpu_thread(void)
 #endif
 }
 
+static __always_inline bool is_user_thread(struct task_struct *task)
+{
+	return !(task->flags & PF_KTHREAD);
+}
+
+static __always_inline bool is_kernel_thread(struct task_struct *task)
+{
+	return task->flags & PF_KTHREAD;
+}
+
 /* Per-process atomic flags. */
 #define PFA_NO_NEW_PRIVS		0	/* May not gain new privileges. */
 #define PFA_SPREAD_PAGE			1	/* Spread page cache over cpuset */
-- 
2.47.2

On April 25, 2025 11:41:21 PM GMT+03:00, Steven Rostedt <rostedt@goodmis.org> wrote:
>From: Steven Rostedt <rostedt@goodmis.org>
>
>In order to know if a task is a user thread or a kernel thread it is
>recommended to test the task flags for PF_KTHREAD. The old way was to
>check if the task mm pointer is NULL.
>
>It is an easy mistake to not test the flag correctly, as:
>
>	if (!(task->flag & PF_KTHREAD))
>
>Is not immediately obvious that it's testing for a user thread.
>
>Add helper functions:
>
>  is_user_thread()
>  is_kernel_thread()
>
>that can make seeing what is being tested for much more obvious:
>
>	if (is_user_thread(task))
>
>Link: https://lore.kernel.org/all/20250425133416.63d3e3b8@gandalf.local.home/
>
>Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
>---
> include/linux/sched.h | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
>diff --git a/include/linux/sched.h b/include/linux/sched.h
>index f96ac1982893..823f38b0fd3e 100644
>--- a/include/linux/sched.h
>+++ b/include/linux/sched.h
>@@ -1785,6 +1785,16 @@ static __always_inline bool is_percpu_thread(void)
> #endif
> }
> 
>+static __always_inline bool is_user_thread(struct task_struct *task)
>+{
>+	return !(task->flags & PF_KTHREAD);
>+}
>+
>+static __always_inline bool is_kernel_thread(struct task_struct *task)
>+{
>+	return task->flags & PF_KTHREAD;

return !is_user_thread(task);

or the other way around. 

🙂

-- 
Sent from a small device: formatting sucks and brevity is inevitable.

On Sat, 26 Apr 2025 14:08:46 +0300
Borislav Petkov <bp@alien8.de> wrote:

> >+static __always_inline bool is_kernel_thread(struct task_struct *task)
> >+{
> >+	return task->flags & PF_KTHREAD;  
> 
> return !is_user_thread(task);
> 
> or the other way around. 

Yeah, I thought about doing that but decided against it.

As Kees mentioned to use !!, I think using the !is_user_thread() is a
better approach.

-- Steve

On Fri, Apr 25, 2025 at 04:41:21PM -0400, Steven Rostedt wrote:
> From: Steven Rostedt <rostedt@goodmis.org>
> 
> In order to know if a task is a user thread or a kernel thread it is
> recommended to test the task flags for PF_KTHREAD. The old way was to
> check if the task mm pointer is NULL.
> 
> It is an easy mistake to not test the flag correctly, as:
> 
> 	if (!(task->flag & PF_KTHREAD))
> 
> Is not immediately obvious that it's testing for a user thread.
> 
> Add helper functions:
> 
>   is_user_thread()
>   is_kernel_thread()
> 
> that can make seeing what is being tested for much more obvious:
> 
> 	if (is_user_thread(task))
> 
> Link: https://lore.kernel.org/all/20250425133416.63d3e3b8@gandalf.local.home/
> 
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
>  include/linux/sched.h | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index f96ac1982893..823f38b0fd3e 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1785,6 +1785,16 @@ static __always_inline bool is_percpu_thread(void)
>  #endif
>  }
>  
> +static __always_inline bool is_user_thread(struct task_struct *task)
> +{
> +	return !(task->flags & PF_KTHREAD);
> +}
> +
> +static __always_inline bool is_kernel_thread(struct task_struct *task)
> +{
> +	return task->flags & PF_KTHREAD;

nit: maybe do explicit type conversion:

	return !!(task->flags & PF_KTHREAD);

but that's just a style issue, really.

Reviewed-by: Kees Cook <kees@kernel.org>

Thank you for not using current->mm -- KUnit, live patching, etc, all
use current->mm but are kthreads. :)

-- 
Kees Cook

On Fri, 25 Apr 2025 16:03:16 -0700
Kees Cook <kees@kernel.org> wrote:

> > +static __always_inline bool is_kernel_thread(struct task_struct *task)
> > +{
> > +	return task->flags & PF_KTHREAD;  
> 
> nit: maybe do explicit type conversion:
> 
> 	return !!(task->flags & PF_KTHREAD);
> 
> but that's just a style issue, really.

I may use Boris's suggestion (which I thought of doing originally too)
and have this return:

	return !is_user_thread(task);


> 
> Reviewed-by: Kees Cook <kees@kernel.org>

Thanks.

> 
> Thank you for not using current->mm -- KUnit, live patching, etc, all
> use current->mm but are kthreads. :)

Yeah, Peter was stressing this.

-- Steve