From: Carsten Haitzler carsten.haitzler@foss.arm.com
Fix the kcmp syscall (when CONFIG_CHECKPOINT_RESTORE is enabled) for the case that the idx arguemnts might be pointers (capabilites) passed in. One of the kcmp types uses idx2 to carry a capability pointer to extended structure information copied in from userspace. This allows for idx2 and idx1 to potentially do this now.
Signed-off-by: Carsten Haitzler carsten.haitzler@foss.arm.com --- kernel/kcmp.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/kernel/kcmp.c b/kernel/kcmp.c index 5353edfad8e1..6f29a94b07d4 100644 --- a/kernel/kcmp.c +++ b/kernel/kcmp.c @@ -133,13 +133,12 @@ static int kcmp_epoll_target(struct task_struct *task1, #endif
SYSCALL_DEFINE5(kcmp, pid_t, pid1, pid_t, pid2, int, type, - unsigned long, idx1, unsigned long, idx2) + user_uintptr_t, idx1, user_uintptr_t, idx2) { struct task_struct *task1, *task2; int ret;
rcu_read_lock(); - /* * Tasks are looked up in caller's PID namespace only. */ @@ -204,7 +203,7 @@ SYSCALL_DEFINE5(kcmp, pid_t, pid1, pid_t, pid2, int, type, #endif break; case KCMP_EPOLL_TFD: - ret = kcmp_epoll_target(task1, task2, idx1, (void *)idx2); + ret = kcmp_epoll_target(task1, task2, idx1, (struct kcmp_epoll_slot __user *)idx2); break; default: ret = -EINVAL;
In title: this is generic enough that we shouldn't mention "morello", just "purecap" or "PCuABI" is fine.
On 19/10/2022 19:03, carsten.haitzler@foss.arm.com wrote:
From: Carsten Haitzler carsten.haitzler@foss.arm.com
Fix the kcmp syscall (when CONFIG_CHECKPOINT_RESTORE is enabled) for the case that the idx arguemnts might be pointers (capabilites) passed in. One
"arguments"
of the kcmp types uses idx2 to carry a capability pointer to extended structure information copied in from userspace. This allows for idx2 and idx1 to potentially do this now.
The only case where a pointer might be specified is indeed idx2, so I'd prefer not to change idx1. Manipulating capabilities (in PCuABI) should be avoided if possible, it's hard for multiplexed syscalls but we can avoid it for idx1 here. Worth noting that in compat, a valid capability will be created for any pointer type in the syscall arguments, which is another reason to use user_uintptr_t only if there is no other option.
Signed-off-by: Carsten Haitzler carsten.haitzler@foss.arm.com
kernel/kcmp.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/kernel/kcmp.c b/kernel/kcmp.c index 5353edfad8e1..6f29a94b07d4 100644 --- a/kernel/kcmp.c +++ b/kernel/kcmp.c @@ -133,13 +133,12 @@ static int kcmp_epoll_target(struct task_struct *task1, #endif SYSCALL_DEFINE5(kcmp, pid_t, pid1, pid_t, pid2, int, type,
unsigned long, idx1, unsigned long, idx2)
user_uintptr_t, idx1, user_uintptr_t, idx2)rcu_read_lock();
Spurious change.
/* * Tasks are looked up in caller's PID namespace only. */ @@ -204,7 +203,7 @@ SYSCALL_DEFINE5(kcmp, pid_t, pid1, pid_t, pid2, int, type, #endif break; case KCMP_EPOLL_TFD:
ret = kcmp_epoll_target(task1, task2, idx1, (void *)idx2);
ret = kcmp_epoll_target(task1, task2, idx1, (struct kcmp_epoll_slot __user *)idx2);
That's well over 80 char now, so better break that line. Or if you don't like that, use void __user * :)
Kevin
break;default: ret = -EINVAL;
On 10/25/22 10:44, Kevin Brodsky wrote:
In title: this is generic enough that we shouldn't mention "morello", just "purecap" or "PCuABI" is fine.
On 19/10/2022 19:03, carsten.haitzler@foss.arm.com wrote:
From: Carsten Haitzler carsten.haitzler@foss.arm.com
Fix the kcmp syscall (when CONFIG_CHECKPOINT_RESTORE is enabled) for the case that the idx arguemnts might be pointers (capabilites) passed in. One
"arguments"
fixed in update
of the kcmp types uses idx2 to carry a capability pointer to extended structure information copied in from userspace. This allows for idx2 and idx1 to potentially do this now.
The only case where a pointer might be specified is indeed idx2, so I'd prefer not to change idx1. Manipulating capabilities (in PCuABI) should be avoided if possible, it's hard for multiplexed syscalls but we can avoid it for idx1 here. Worth noting that in compat, a valid capability will be created for any pointer type in the syscall arguments, which is another reason to use user_uintptr_t only if there is no other option.
this was a debate i had with myself. i chose to "design ahead" in case it's ever needed with idx1 as well, but updated patch removes that.
Signed-off-by: Carsten Haitzler carsten.haitzler@foss.arm.com
kernel/kcmp.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/kernel/kcmp.c b/kernel/kcmp.c index 5353edfad8e1..6f29a94b07d4 100644 --- a/kernel/kcmp.c +++ b/kernel/kcmp.c @@ -133,13 +133,12 @@ static int kcmp_epoll_target(struct task_struct *task1, #endif SYSCALL_DEFINE5(kcmp, pid_t, pid1, pid_t, pid2, int, type, - unsigned long, idx1, unsigned long, idx2) + user_uintptr_t, idx1, user_uintptr_t, idx2) { struct task_struct *task1, *task2; int ret; rcu_read_lock();
Spurious change.
missed that -0 my colorized diff sometimes has me miss single red - lines :)
/* * Tasks are looked up in caller's PID namespace only. */ @@ -204,7 +203,7 @@ SYSCALL_DEFINE5(kcmp, pid_t, pid1, pid_t, pid2, int, type, #endif break; case KCMP_EPOLL_TFD: - ret = kcmp_epoll_target(task1, task2, idx1, (void *)idx2); + ret = kcmp_epoll_target(task1, task2, idx1, (struct kcmp_epoll_slot __user *)idx2);
That's well over 80 char now, so better break that line. Or if you don't like that, use void __user * :)
it was close - but put it on another line now.
Kevin
break; default: ret = -EINVAL;
linux-morello@op-lists.linaro.org
-
Carsten Haitzler -
carsten.haitzler＠foss.arm.com
-
Kevin Brodsky