Modify seccomp_do_user_notification(), __seccomp_filter(),
__secure_computing() to use current_pt_regs().
Signed-off-by: Denis Efremov <[email protected]>
---
kernel/seccomp.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/kernel/seccomp.c b/kernel/seccomp.c
index 3ee59ce0a323..dc4eaa1d6002 100644
--- a/kernel/seccomp.c
+++ b/kernel/seccomp.c
@@ -910,7 +910,7 @@ static int seccomp_do_user_notification(int this_syscall,
if (flags & SECCOMP_USER_NOTIF_FLAG_CONTINUE)
return 0;
- syscall_set_return_value(current, task_pt_regs(current),
+ syscall_set_return_value(current, current_pt_regs(),
err, ret);
return -1;
}
@@ -943,13 +943,13 @@ static int __seccomp_filter(int this_syscall, const struct seccomp_data *sd,
/* Set low-order bits as an errno, capped at MAX_ERRNO. */
if (data > MAX_ERRNO)
data = MAX_ERRNO;
- syscall_set_return_value(current, task_pt_regs(current),
+ syscall_set_return_value(current, current_pt_regs(),
-data, 0);
goto skip;
case SECCOMP_RET_TRAP:
/* Show the handler the original registers. */
- syscall_rollback(current, task_pt_regs(current));
+ syscall_rollback(current, current_pt_regs());
/* Let the filter pass back 16 bits of data. */
seccomp_send_sigsys(this_syscall, data);
goto skip;
@@ -962,7 +962,7 @@ static int __seccomp_filter(int this_syscall, const struct seccomp_data *sd,
/* ENOSYS these calls if there is no tracer attached. */
if (!ptrace_event_enabled(current, PTRACE_EVENT_SECCOMP)) {
syscall_set_return_value(current,
- task_pt_regs(current),
+ current_pt_regs(),
-ENOSYS, 0);
goto skip;
}
@@ -982,7 +982,7 @@ static int __seccomp_filter(int this_syscall, const struct seccomp_data *sd,
if (fatal_signal_pending(current))
goto skip;
/* Check if the tracer forced the syscall to be skipped. */
- this_syscall = syscall_get_nr(current, task_pt_regs(current));
+ this_syscall = syscall_get_nr(current, current_pt_regs());
if (this_syscall < 0)
goto skip;
@@ -1025,7 +1025,7 @@ static int __seccomp_filter(int this_syscall, const struct seccomp_data *sd,
kernel_siginfo_t info;
/* Show the original registers in the dump. */
- syscall_rollback(current, task_pt_regs(current));
+ syscall_rollback(current, current_pt_regs());
/* Trigger a manual coredump since do_exit skips it. */
seccomp_init_siginfo(&info, this_syscall, data);
do_coredump(&info);
@@ -1060,7 +1060,7 @@ int __secure_computing(const struct seccomp_data *sd)
return 0;
this_syscall = sd ? sd->nr :
- syscall_get_nr(current, task_pt_regs(current));
+ syscall_get_nr(current, current_pt_regs());
switch (mode) {
case SECCOMP_MODE_STRICT:
--
2.26.2
On Mon, Aug 24, 2020 at 03:59:21PM +0300, Denis Efremov wrote:
> Modify seccomp_do_user_notification(), __seccomp_filter(),
> __secure_computing() to use current_pt_regs().
This looks okay. It seems some architectures have a separate
define for current_pt_regs(), though it's overlapped directly with
task_pt_regs(). I'm curious what the benefit of the change is?
-Kees
>
> Signed-off-by: Denis Efremov <[email protected]>
> ---
> kernel/seccomp.c | 14 +++++++-------
> 1 file changed, 7 insertions(+), 7 deletions(-)
>
> diff --git a/kernel/seccomp.c b/kernel/seccomp.c
> index 3ee59ce0a323..dc4eaa1d6002 100644
> --- a/kernel/seccomp.c
> +++ b/kernel/seccomp.c
> @@ -910,7 +910,7 @@ static int seccomp_do_user_notification(int this_syscall,
> if (flags & SECCOMP_USER_NOTIF_FLAG_CONTINUE)
> return 0;
>
> - syscall_set_return_value(current, task_pt_regs(current),
> + syscall_set_return_value(current, current_pt_regs(),
> err, ret);
> return -1;
> }
> @@ -943,13 +943,13 @@ static int __seccomp_filter(int this_syscall, const struct seccomp_data *sd,
> /* Set low-order bits as an errno, capped at MAX_ERRNO. */
> if (data > MAX_ERRNO)
> data = MAX_ERRNO;
> - syscall_set_return_value(current, task_pt_regs(current),
> + syscall_set_return_value(current, current_pt_regs(),
> -data, 0);
> goto skip;
>
> case SECCOMP_RET_TRAP:
> /* Show the handler the original registers. */
> - syscall_rollback(current, task_pt_regs(current));
> + syscall_rollback(current, current_pt_regs());
> /* Let the filter pass back 16 bits of data. */
> seccomp_send_sigsys(this_syscall, data);
> goto skip;
> @@ -962,7 +962,7 @@ static int __seccomp_filter(int this_syscall, const struct seccomp_data *sd,
> /* ENOSYS these calls if there is no tracer attached. */
> if (!ptrace_event_enabled(current, PTRACE_EVENT_SECCOMP)) {
> syscall_set_return_value(current,
> - task_pt_regs(current),
> + current_pt_regs(),
> -ENOSYS, 0);
> goto skip;
> }
> @@ -982,7 +982,7 @@ static int __seccomp_filter(int this_syscall, const struct seccomp_data *sd,
> if (fatal_signal_pending(current))
> goto skip;
> /* Check if the tracer forced the syscall to be skipped. */
> - this_syscall = syscall_get_nr(current, task_pt_regs(current));
> + this_syscall = syscall_get_nr(current, current_pt_regs());
> if (this_syscall < 0)
> goto skip;
>
> @@ -1025,7 +1025,7 @@ static int __seccomp_filter(int this_syscall, const struct seccomp_data *sd,
> kernel_siginfo_t info;
>
> /* Show the original registers in the dump. */
> - syscall_rollback(current, task_pt_regs(current));
> + syscall_rollback(current, current_pt_regs());
> /* Trigger a manual coredump since do_exit skips it. */
> seccomp_init_siginfo(&info, this_syscall, data);
> do_coredump(&info);
> @@ -1060,7 +1060,7 @@ int __secure_computing(const struct seccomp_data *sd)
> return 0;
>
> this_syscall = sd ? sd->nr :
> - syscall_get_nr(current, task_pt_regs(current));
> + syscall_get_nr(current, current_pt_regs());
>
> switch (mode) {
> case SECCOMP_MODE_STRICT:
> --
> 2.26.2
>
--
Kees Cook
On 8/24/20 7:34 PM, Kees Cook wrote:
> On Mon, Aug 24, 2020 at 03:59:21PM +0300, Denis Efremov wrote:
>> Modify seccomp_do_user_notification(), __seccomp_filter(),
>> __secure_computing() to use current_pt_regs().
>
> This looks okay. It seems some architectures have a separate
> define for current_pt_regs(), though it's overlapped directly with
> task_pt_regs(). I'm curious what the benefit of the change is?
>
Generally, it's just a shorthand.
From: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a3460a59747cfddfa7be4758e5ef08bf5d751d59
- arch versions are "optimized versions"
- some architectures have task_pt_regs() working only
for traced tasks blocked on signal delivery. current_pt_regs()
needs to work for *all* processes
My motivation:
I'm going to add cocci rule for using current_uid(), current_xxx(), ...
instead of raw accesses to current->cred, current->cred->uid
https://elixir.bootlin.com/linux/latest/source/include/linux/cred.h#L379
These interfaces use rcu_dereference_protected() internally for access
check.
I though that adding current_pt_regs(), current_user_stack_pointer() to
this cocci rule will be a good idea.
Thanks,
Denis
On Mon, 24 Aug 2020 15:59:21 +0300, Denis Efremov wrote:
> Modify seccomp_do_user_notification(), __seccomp_filter(),
> __secure_computing() to use current_pt_regs().
Applied, thanks!
[1/1] seccomp: Use current_pt_regs() instead of task_pt_regs(current)
https://git.kernel.org/kees/c/4484dbacd7b6
I reworded your commit based on the thread and added one comment for
a weird case where task == current, hopefully that looks correct:
https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/commit/?h=for-next/seccomp&id=4484dbacd7b61eaa4e21332c0a044dedce732ebb
--
Kees Cook