* [PATCH 3/6] exec: cleanup the count() function
From: Christoph Hellwig @ 2020-06-15 13:00 UTC (permalink / raw)
To: Al Viro
Cc: linux-arch, linux-s390, Arnd Bergmann, linux-parisc, x86,
linux-mips, linux-kernel, linux-fsdevel, Luis Chamberlain,
sparclinux, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20200615130032.931285-1-hch@lst.de>
Remove the max argument as it is hard wired to MAX_ARG_STRINGS, and
give the function a slightly less generic name.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/exec.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index 94107eceda8a67..6e4d9d1ffa35fa 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -411,9 +411,9 @@ get_user_arg_ptr(const char __user *const __user *argv, int nr)
}
/*
- * count() counts the number of strings in array ARGV.
+ * count_strings() counts the number of strings in array ARGV.
*/
-static int count(const char __user *const __user *argv, int max)
+static int count_strings(const char __user *const __user *argv)
{
int i = 0;
@@ -427,7 +427,7 @@ static int count(const char __user *const __user *argv, int max)
if (IS_ERR(p))
return -EFAULT;
- if (i >= max)
+ if (i >= MAX_ARG_STRINGS)
return -E2BIG;
++i;
@@ -445,11 +445,11 @@ static int prepare_arg_pages(struct linux_binprm *bprm,
{
unsigned long limit, ptr_size;
- bprm->argc = count(argv, MAX_ARG_STRINGS);
+ bprm->argc = count_strings(argv);
if (bprm->argc < 0)
return bprm->argc;
- bprm->envc = count(envp, MAX_ARG_STRINGS);
+ bprm->envc = count_strings(envp);
if (bprm->envc < 0)
return bprm->envc;
--
2.26.2
^ permalink raw reply related
* [PATCH 4/6] exec: split prepare_arg_pages
From: Christoph Hellwig @ 2020-06-15 13:00 UTC (permalink / raw)
To: Al Viro
Cc: linux-arch, linux-s390, Arnd Bergmann, linux-parisc, x86,
linux-mips, linux-kernel, linux-fsdevel, Luis Chamberlain,
sparclinux, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20200615130032.931285-1-hch@lst.de>
Move counting the arguments and enviroment variables out of
prepare_arg_pages and rename the rest of the function to check_arg_limit.
This prepares for a version of do_execvat that takes kernel pointers.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/exec.c | 26 ++++++++++++++------------
1 file changed, 14 insertions(+), 12 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index 6e4d9d1ffa35fa..696b1e8180d7d8 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -439,20 +439,10 @@ static int count_strings(const char __user *const __user *argv)
return i;
}
-static int prepare_arg_pages(struct linux_binprm *bprm,
- const char __user *const __user *argv,
- const char __user *const __user *envp)
+static int check_arg_limit(struct linux_binprm *bprm)
{
unsigned long limit, ptr_size;
- bprm->argc = count_strings(argv);
- if (bprm->argc < 0)
- return bprm->argc;
-
- bprm->envc = count_strings(envp);
- if (bprm->envc < 0)
- return bprm->envc;
-
/*
* Limit to 1/4 of the max stack size or 3/4 of _STK_LIM
* (whichever is smaller) for the argv+env strings.
@@ -1890,7 +1880,19 @@ int do_execveat(int fd, struct filename *filename,
if (retval)
goto out_unmark;
- retval = prepare_arg_pages(bprm, argv, envp);
+ bprm->argc = count_strings(argv);
+ if (bprm->argc < 0) {
+ retval = bprm->argc;
+ goto out;
+ }
+
+ bprm->envc = count_strings(envp);
+ if (bprm->envc < 0) {
+ retval = bprm->envc;
+ goto out;
+ }
+
+ retval = check_arg_limit(bprm);
if (retval < 0)
goto out;
--
2.26.2
^ permalink raw reply related
* [PATCH 5/6] exec: add a kernel_execveat helper
From: Christoph Hellwig @ 2020-06-15 13:00 UTC (permalink / raw)
To: Al Viro
Cc: linux-arch, linux-s390, Arnd Bergmann, linux-parisc, x86,
linux-mips, linux-kernel, linux-fsdevel, Luis Chamberlain,
sparclinux, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20200615130032.931285-1-hch@lst.de>
Add a kernel_execveat helper to execute a binary with kernel space argv
and envp pointers. Switch executing init and user mode helpers to this
new helper instead of relying on the implicit set_fs(KERNEL_DS) for early
init code and kernel threads, and move the getname call into the
do_execve helper.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
fs/exec.c | 116 +++++++++++++++++++++++++++++++---------
include/linux/binfmts.h | 6 +--
init/main.c | 6 +--
kernel/umh.c | 8 ++-
4 files changed, 97 insertions(+), 39 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index 696b1e8180d7d8..9e16d56aa53e86 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -439,6 +439,21 @@ static int count_strings(const char __user *const __user *argv)
return i;
}
+static int count_kernel_strings(const char *const *argv)
+{
+ int i;
+
+ if (!argv)
+ return 0;
+
+ for (i = 0; argv[i]; i++) {
+ if (i >= MAX_ARG_STRINGS)
+ return -E2BIG;
+ }
+
+ return i;
+}
+
static int check_arg_limit(struct linux_binprm *bprm)
{
unsigned long limit, ptr_size;
@@ -615,6 +630,19 @@ int copy_string_kernel(const char *arg, struct linux_binprm *bprm)
}
EXPORT_SYMBOL(copy_string_kernel);
+static int copy_strings_kernel(int argc, const char *const *argv,
+ struct linux_binprm *bprm)
+{
+ int ret;
+
+ while (argc-- > 0) {
+ ret = copy_string_kernel(argv[argc], bprm);
+ if (ret)
+ break;
+ }
+ return ret;
+}
+
#ifdef CONFIG_MMU
/*
@@ -1797,9 +1825,11 @@ static int exec_binprm(struct linux_binprm *bprm)
return 0;
}
-int do_execveat(int fd, struct filename *filename,
+static int __do_execveat(int fd, struct filename *filename,
const char __user *const __user *argv,
const char __user *const __user *envp,
+ const char *const *kernel_argv,
+ const char *const *kernel_envp,
int flags, struct file *file)
{
char *pathbuf = NULL;
@@ -1880,16 +1910,30 @@ int do_execveat(int fd, struct filename *filename,
if (retval)
goto out_unmark;
- bprm->argc = count_strings(argv);
- if (bprm->argc < 0) {
- retval = bprm->argc;
- goto out;
- }
+ if (unlikely(kernel_argv)) {
+ bprm->argc = count_kernel_strings(kernel_argv);
+ if (bprm->argc < 0) {
+ retval = bprm->argc;
+ goto out;
+ }
- bprm->envc = count_strings(envp);
- if (bprm->envc < 0) {
- retval = bprm->envc;
- goto out;
+ bprm->envc = count_kernel_strings(kernel_envp);
+ if (bprm->envc < 0) {
+ retval = bprm->envc;
+ goto out;
+ }
+ } else {
+ bprm->argc = count_strings(argv);
+ if (bprm->argc < 0) {
+ retval = bprm->argc;
+ goto out;
+ }
+
+ bprm->envc = count_strings(envp);
+ if (bprm->envc < 0) {
+ retval = bprm->envc;
+ goto out;
+ }
}
retval = check_arg_limit(bprm);
@@ -1906,13 +1950,22 @@ int do_execveat(int fd, struct filename *filename,
goto out;
bprm->exec = bprm->p;
- retval = copy_strings(bprm->envc, envp, bprm);
- if (retval < 0)
- goto out;
- retval = copy_strings(bprm->argc, argv, bprm);
- if (retval < 0)
- goto out;
+ if (unlikely(kernel_argv)) {
+ retval = copy_strings_kernel(bprm->envc, kernel_envp, bprm);
+ if (retval < 0)
+ goto out;
+ retval = copy_strings_kernel(bprm->argc, kernel_argv, bprm);
+ if (retval < 0)
+ goto out;
+ } else {
+ retval = copy_strings(bprm->envc, envp, bprm);
+ if (retval < 0)
+ goto out;
+ retval = copy_strings(bprm->argc, argv, bprm);
+ if (retval < 0)
+ goto out;
+ }
retval = exec_binprm(bprm);
if (retval < 0)
@@ -1963,6 +2016,23 @@ int do_execveat(int fd, struct filename *filename,
return retval;
}
+static int do_execveat(int fd, const char *filename,
+ const char __user *const __user *argv,
+ const char __user *const __user *envp, int flags)
+{
+ int lookup_flags = (flags & AT_EMPTY_PATH) ? LOOKUP_EMPTY : 0;
+ struct filename *name = getname_flags(filename, lookup_flags, NULL);
+
+ return __do_execveat(fd, name, argv, envp, NULL, NULL, flags, NULL);
+}
+
+int kernel_execveat(int fd, const char *filename, const char *const *argv,
+ const char *const *envp, int flags, struct file *file)
+{
+ return __do_execveat(fd, getname_kernel(filename), NULL, NULL, argv,
+ envp, flags, file);
+}
+
void set_binfmt(struct linux_binfmt *new)
{
struct mm_struct *mm = current->mm;
@@ -1992,7 +2062,7 @@ SYSCALL_DEFINE3(execve,
const char __user *const __user *, argv,
const char __user *const __user *, envp)
{
- return do_execveat(AT_FDCWD, getname(filename), argv, envp, 0, NULL);
+ return do_execveat(AT_FDCWD, filename, argv, envp, 0);
}
SYSCALL_DEFINE5(execveat,
@@ -2001,10 +2071,7 @@ SYSCALL_DEFINE5(execveat,
const char __user *const __user *, envp,
int, flags)
{
- int lookup_flags = (flags & AT_EMPTY_PATH) ? LOOKUP_EMPTY : 0;
- struct filename *name = getname_flags(filename, lookup_flags, NULL);
-
- return do_execveat(fd, name, argv, envp, flags, NULL);
+ return do_execveat(fd, filename, argv, envp, flags);
}
/*
@@ -2017,7 +2084,7 @@ COMPAT_SYSCALL_DEFINE3(execve, const char __user *, filename,
const char __user *const __user *, argv,
const char __user *const __user *, envp)
{
- return do_execveat(AT_FDCWD, getname(filename), argv, envp, 0, NULL);
+ return do_execveat(AT_FDCWD, filename, argv, envp, 0);
}
COMPAT_SYSCALL_DEFINE5(execveat, int, fd,
@@ -2026,9 +2093,6 @@ COMPAT_SYSCALL_DEFINE5(execveat, int, fd,
const char __user *const __user *, envp,
int, flags)
{
- int lookup_flags = (flags & AT_EMPTY_PATH) ? LOOKUP_EMPTY : 0;
- struct filename *name = getname_flags(filename, lookup_flags, NULL);
-
- return do_execveat(fd, name, argv, envp, flags, NULL);
+ return do_execveat(fd, filename, argv, envp, flags);
}
#endif /* CONFIG_X86_X32 */
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index bed702e4b1fbd9..1e61c980c16354 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -134,9 +134,7 @@ int copy_string_kernel(const char *arg, struct linux_binprm *bprm);
extern void set_binfmt(struct linux_binfmt *new);
extern ssize_t read_code(struct file *, unsigned long, loff_t, size_t);
-int do_execveat(int fd, struct filename *filename,
- const char __user *const __user *__argv,
- const char __user *const __user *__envp,
- int flags, struct file *file);
+int kernel_execveat(int fd, const char *filename, const char *const *argv,
+ const char *const *envp, int flags, struct file *file);
#endif /* _LINUX_BINFMTS_H */
diff --git a/init/main.c b/init/main.c
index 838950ea7bca22..33de235dc2aa00 100644
--- a/init/main.c
+++ b/init/main.c
@@ -1329,10 +1329,8 @@ static int run_init_process(const char *init_filename)
pr_debug(" with environment:\n");
for (p = envp_init; *p; p++)
pr_debug(" %s\n", *p);
- return do_execveat(AT_FDCWD, getname_kernel(init_filename),
- (const char __user *const __user *)argv_init,
- (const char __user *const __user *)envp_init,
- 0, NULL);
+ return kernel_execveat(AT_FDCWD, init_filename, argv_init, envp_init, 0,
+ NULL);
}
static int try_to_run_init_process(const char *init_filename)
diff --git a/kernel/umh.c b/kernel/umh.c
index 7aa9a5817582ca..1284823dbad338 100644
--- a/kernel/umh.c
+++ b/kernel/umh.c
@@ -103,11 +103,9 @@ static int call_usermodehelper_exec_async(void *data)
commit_creds(new);
sub_info->pid = task_pid_nr(current);
- retval = do_execveat(AT_FDCWD,
- sub_info->path ? getname_kernel(sub_info->path) : NULL,
- (const char __user *const __user *)sub_info->argv,
- (const char __user *const __user *)sub_info->envp,
- 0, sub_info->file);
+ retval = kernel_execveat(AT_FDCWD, sub_info->path,
+ (const char *const *)sub_info->argv,
+ (const char *const *)sub_info->envp, 0, sub_info->file);
if (sub_info->file && !retval)
current->flags |= PF_UMH;
out:
--
2.26.2
^ permalink raw reply related
* [PATCH 6/6] kernel: add a kernel_wait helper
From: Christoph Hellwig @ 2020-06-15 13:00 UTC (permalink / raw)
To: Al Viro
Cc: linux-arch, linux-s390, Arnd Bergmann, linux-parisc, x86,
linux-mips, linux-kernel, linux-fsdevel, Luis Chamberlain,
sparclinux, linuxppc-dev, linux-arm-kernel
In-Reply-To: <20200615130032.931285-1-hch@lst.de>
Add a helper that waits for a pid and stores the status in the passed
in kernel pointer. Use it to fix the usage of kernel_wait4 in
call_usermodehelper_exec_sync that only happens to work due to the
implicit set_fs(KERNEL_DS) for kernel threads.
Signed-off-by: Christoph Hellwig <hch@lst.de>
---
include/linux/sched/task.h | 1 +
kernel/exit.c | 16 ++++++++++++++++
kernel/umh.c | 29 ++++-------------------------
3 files changed, 21 insertions(+), 25 deletions(-)
diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h
index 38359071236ad7..a80007df396e95 100644
--- a/include/linux/sched/task.h
+++ b/include/linux/sched/task.h
@@ -102,6 +102,7 @@ struct task_struct *fork_idle(int);
struct mm_struct *copy_init_mm(void);
extern pid_t kernel_thread(int (*fn)(void *), void *arg, unsigned long flags);
extern long kernel_wait4(pid_t, int __user *, int, struct rusage *);
+int kernel_wait(pid_t pid, int *stat);
extern void free_task(struct task_struct *tsk);
diff --git a/kernel/exit.c b/kernel/exit.c
index 727150f2810338..fd598846df0b17 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -1626,6 +1626,22 @@ long kernel_wait4(pid_t upid, int __user *stat_addr, int options,
return ret;
}
+int kernel_wait(pid_t pid, int *stat)
+{
+ struct wait_opts wo = {
+ .wo_type = PIDTYPE_PID,
+ .wo_pid = find_get_pid(pid),
+ .wo_flags = WEXITED,
+ };
+ int ret;
+
+ ret = do_wait(&wo);
+ if (ret > 0 && wo.wo_stat)
+ *stat = wo.wo_stat;
+ put_pid(wo.wo_pid);
+ return ret;
+}
+
SYSCALL_DEFINE4(wait4, pid_t, upid, int __user *, stat_addr,
int, options, struct rusage __user *, ru)
{
diff --git a/kernel/umh.c b/kernel/umh.c
index 1284823dbad338..6fd948e478bec4 100644
--- a/kernel/umh.c
+++ b/kernel/umh.c
@@ -126,37 +126,16 @@ static void call_usermodehelper_exec_sync(struct subprocess_info *sub_info)
{
pid_t pid;
- /* If SIGCLD is ignored kernel_wait4 won't populate the status. */
+ /* If SIGCLD is ignored do_wait won't populate the status. */
kernel_sigaction(SIGCHLD, SIG_DFL);
pid = kernel_thread(call_usermodehelper_exec_async, sub_info, SIGCHLD);
- if (pid < 0) {
+ if (pid < 0)
sub_info->retval = pid;
- } else {
- int ret = -ECHILD;
- /*
- * Normally it is bogus to call wait4() from in-kernel because
- * wait4() wants to write the exit code to a userspace address.
- * But call_usermodehelper_exec_sync() always runs as kernel
- * thread (workqueue) and put_user() to a kernel address works
- * OK for kernel threads, due to their having an mm_segment_t
- * which spans the entire address space.
- *
- * Thus the __user pointer cast is valid here.
- */
- kernel_wait4(pid, (int __user *)&ret, 0, NULL);
-
- /*
- * If ret is 0, either call_usermodehelper_exec_async failed and
- * the real error code is already in sub_info->retval or
- * sub_info->retval is 0 anyway, so don't mess with it then.
- */
- if (ret)
- sub_info->retval = ret;
- }
+ else
+ kernel_wait(pid, &sub_info->retval);
/* Restore default kernel sig handler */
kernel_sigaction(SIGCHLD, SIG_IGN);
-
umh_complete(sub_info);
}
--
2.26.2
^ permalink raw reply related
* [PATCH] powerpc/ptdump: Fix build failure in hashpagetable.c
From: Christophe Leroy @ 2020-06-15 13:18 UTC (permalink / raw)
To: Benjamin Herrenschmidt, Paul Mackerras, Michael Ellerman
Cc: linuxppc-dev, linux-kernel
H_SUCCESS is only defined when CONFIG_PPC_PSERIES is defined.
!= H_SUCCESS means != 0. Modify the test accordingly.
Reported-by: kernel test robot <lkp@intel.com>
Fixes: 65e701b2d2a8 ("powerpc/ptdump: drop non vital #ifdefs")
Cc: stable@vger.kernel.org
Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
---
arch/powerpc/mm/ptdump/hashpagetable.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/powerpc/mm/ptdump/hashpagetable.c b/arch/powerpc/mm/ptdump/hashpagetable.c
index a2c33efc7ce8..5b8bd34cd3a1 100644
--- a/arch/powerpc/mm/ptdump/hashpagetable.c
+++ b/arch/powerpc/mm/ptdump/hashpagetable.c
@@ -258,7 +258,7 @@ static int pseries_find(unsigned long ea, int psize, bool primary, u64 *v, u64 *
for (i = 0; i < HPTES_PER_GROUP; i += 4, hpte_group += 4) {
lpar_rc = plpar_pte_read_4(0, hpte_group, (void *)ptes);
- if (lpar_rc != H_SUCCESS)
+ if (lpar_rc)
continue;
for (j = 0; j < 4; j++) {
if (HPTE_V_COMPARE(ptes[j].v, want_v) &&
--
2.25.0
^ permalink raw reply related
* Re: [PATCH 3/3] powerpc/8xx: Provide ptep_get() with 16k pages
From: Peter Zijlstra @ 2020-06-15 13:22 UTC (permalink / raw)
To: Christophe Leroy
Cc: linuxppc-dev, linux-kernel, linux-mm, Paul Mackerras,
Andrew Morton, Will Deacon
In-Reply-To: <341688399c1b102756046d19ea6ce39db1ae4742.1592225558.git.christophe.leroy@csgroup.eu>
On Mon, Jun 15, 2020 at 12:57:59PM +0000, Christophe Leroy wrote:
> READ_ONCE() now enforces atomic read, which leads to:
> Fixes: 2ab3a0a02905 ("READ_ONCE: Enforce atomicity for {READ,WRITE}_ONCE() memory accesses")
> Cc: Will Deacon <will@kernel.org>
> Signed-off-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> arch/powerpc/include/asm/nohash/32/pgtable.h | 10 ++++++++++
> 1 file changed, 10 insertions(+)
>
> diff --git a/arch/powerpc/include/asm/nohash/32/pgtable.h b/arch/powerpc/include/asm/nohash/32/pgtable.h
> index b56f14160ae5..77addb599ce7 100644
> --- a/arch/powerpc/include/asm/nohash/32/pgtable.h
> +++ b/arch/powerpc/include/asm/nohash/32/pgtable.h
> @@ -286,6 +286,16 @@ static inline pte_t ptep_get_and_clear(struct mm_struct *mm, unsigned long addr,
> return __pte(pte_update(mm, addr, ptep, ~0, 0, 0));
> }
>
> +#if defined(CONFIG_PPC_8xx) && defined(CONFIG_PPC_16K_PAGES)
> +#define __HAVE_ARCH_PTEP_GET
> +static inline pte_t ptep_get(pte_t *ptep)
> +{
> + pte_t pte = {READ_ONCE(ptep->pte), 0, 0, 0};
> +
> + return pte;
> +}
> +#endif
Would it make sense to have a comment with this magic? The casual reader
might wonder WTH just happened when he stumbles on this :-)
Other than that, the series looks good to me, Thanks!
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
^ permalink raw reply
* Re: properly support exec and wait with kernel pointers
From: Arnd Bergmann @ 2020-06-15 13:42 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch, linux-s390, Parisc List, the arch/x86 maintainers,
open list:BROADCOM NVRAM DRIVER, linux-kernel@vger.kernel.org,
Linux FS-devel Mailing List, Luis Chamberlain, Al Viro,
sparclinux, linuxppc-dev, Linux ARM
In-Reply-To: <20200615130032.931285-1-hch@lst.de>
On Mon, Jun 15, 2020 at 3:00 PM Christoph Hellwig <hch@lst.de> wrote:
>
> Hi all,
>
> this series first cleans up the exec code and then adds proper
> kernel_execveat and kernel_wait callers instead of relying on the fact
> that the early init code and kernel threads implicitly run with
> the address limit set to KERNEL_DS.
>
> Note that the cleanup removes the compat execve(at) handlers (almost)
> entirely, as we can handle the compat difference very nicely in a
> unified codebase. The only exception is x86 where this would list the
> handlers twice in the same syscall table due to the messed up x32
> design. I had to add an extra compat handler just for that case, but
> maybe someone has a better idea.
I looked at all the patches and I like it a lot. I replied with some suggestions
for x32, but maybe I misunderstood what its problem is, as I don't see
anything preventing us from having two entries in the x32 table pointing
to the same function.
Arnd
^ permalink raw reply
* Re: [PATCH 2/6] exec: simplify the compat syscall handling
From: Arnd Bergmann @ 2020-06-15 13:31 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch, linux-s390, Parisc List, the arch/x86 maintainers,
open list:BROADCOM NVRAM DRIVER, linux-kernel@vger.kernel.org,
Linux FS-devel Mailing List, Luis Chamberlain, Al Viro,
sparclinux, linuxppc-dev, Linux ARM
In-Reply-To: <20200615130032.931285-3-hch@lst.de>
On Mon, Jun 15, 2020 at 3:00 PM Christoph Hellwig <hch@lst.de> wrote:
>
> The only differenence betweeen the compat exec* syscalls and their
> native versions is that compat_ptr sign extension, and the fact that
> the pointer arithmetics for the two dimensional arrays needs to use
> the compat pointer size. Instead of the compat wrappers and the
> struct user_arg_ptr machinery just use in_compat_syscall() to do the
> right thing for the compat case deep inside get_user_arg_ptr().
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
Nice!
I have an older patch doing the same for sys_mount() somewhere,
but never got around to send that. One day we should see if we
can just do this for all of them.
> -
> -static const char __user *get_user_arg_ptr(struct user_arg_ptr argv, int nr)
> +static const char __user *
> +get_user_arg_ptr(const char __user *const __user *argv, int nr)
> {
> const char __user *native;
>
> #ifdef CONFIG_COMPAT
> - if (unlikely(argv.is_compat)) {
> + if (in_compat_syscall()) {
> + const compat_uptr_t __user *compat_argv =
> + compat_ptr((unsigned long)argv);
> compat_uptr_t compat;
>
> - if (get_user(compat, argv.ptr.compat + nr))
> + if (get_user(compat, compat_argv + nr))
> return ERR_PTR(-EFAULT);
>
> return compat_ptr(compat);
> }
> #endif
I would expect that the "#ifdef CONFIG_COMPAT" can be removed
now, since compat_ptr() and in_compat_syscall() are now defined
unconditionally. I have not tried that though.
> +/*
> + * x32 syscalls are listed in the same table as x86_64 ones, so we need to
> + * define compat syscalls that are exactly the same as the native version for
> + * the syscall table machinery to work. Sigh..
> + */
> +#ifdef CONFIG_X86_X32
> COMPAT_SYSCALL_DEFINE3(execve, const char __user *, filename,
> - const compat_uptr_t __user *, argv,
> - const compat_uptr_t __user *, envp)
> + const char __user *const __user *, argv,
> + const char __user *const __user *, envp)
> {
> - return do_compat_execve(AT_FDCWD, getname(filename), argv, envp, 0);
> + return do_execveat(AT_FDCWD, getname(filename), argv, envp, 0, NULL);
> }
Maybe move it to arch/x86/kernel/process_64.c or arch/x86/entry/syscall_x32.c
to keep it out of the common code if this is needed. I don't really understand
the comment, why can't this just use this?
--- a/arch/x86/entry/syscalls/syscall_64.tbl
+++ b/arch/x86/entry/syscalls/syscall_64.tbl
@@ -379,7 +379,7 @@
517 x32 recvfrom compat_sys_recvfrom
518 x32 sendmsg compat_sys_sendmsg
519 x32 recvmsg compat_sys_recvmsg
-520 x32 execve compat_sys_execve
+520 x32 execve sys_execve
521 x32 ptrace compat_sys_ptrace
522 x32 rt_sigpending compat_sys_rt_sigpending
523 x32 rt_sigtimedwait compat_sys_rt_sigtimedwait_time64
@@ -404,7 +404,7 @@
542 x32 getsockopt compat_sys_getsockopt
543 x32 io_setup compat_sys_io_setup
544 x32 io_submit compat_sys_io_submit
-545 x32 execveat compat_sys_execveat
+545 x32 execveat sys_execveat
546 x32 preadv2 compat_sys_preadv64v2
547 x32 pwritev2 compat_sys_pwritev64v2
548 x32 process_madvise compat_sys_process_madvise
Arnd
^ permalink raw reply
* Re: [PATCH 0/8 v2] PCI: Align return values of PCIe capability and PCI accessors
From: Jason Gunthorpe @ 2020-06-15 13:46 UTC (permalink / raw)
To: refactormyself
Cc: Don Brace, Sam Bobroff, Mike Marciniszyn, linux-scsi,
Martin K. Petersen, linux-rdma, linux-pci, Dennis Dalessandro,
esc.storagedev, Doug Ledford, linux-kernel, dmaengine, Vinod Koul,
helgaas, skhan, bjorn, Oliver O'Halloran, linuxppc-dev,
James E.J. Bottomley, linux-kernel-mentees
In-Reply-To: <20200615073225.24061-1-refactormyself@gmail.com>
On Mon, Jun 15, 2020 at 09:32:17AM +0200, refactormyself@gmail.com wrote:
> From: Bolarinwa Olayemi Saheed <refactormyself@gmail.com>
>
>
> PATCH 1/8 to 7/8:
> PCIBIOS_ error codes have positive values and they are passed down the
> call heirarchy from accessors. For functions which are meant to return
> only a negative value on failure, passing on this value is a bug.
> To mitigate this, call pcibios_err_to_errno() before passing on return
> value from PCIe capability accessors call heirarchy. This function
> converts any positive PCIBIOS_ error codes to negative generic error
> values.
>
> PATCH 8/8:
> The PCIe capability accessors can return 0, -EINVAL, or any PCIBIOS_ error
> code. The pci accessor on the other hand can only return 0 or any PCIBIOS_
> error code.This inconsistency among these accessor makes it harder for
> callers to check for errors.
> Return PCIBIOS_BAD_REGISTER_NUMBER instead of -EINVAL in all PCIe
> capability accessors.
>
> MERGING:
> These may all be merged via the PCI tree, since it is a collection of
> similar fixes. This way they all get merged at once.
I prefer this not happen for active trees, it just risks needless
merge conflicts.
I will take the hfi1 patches at least, let me know when they are
reviewed
Thanks,
Jason
^ permalink raw reply
* Re: [PATCH 2/6] exec: simplify the compat syscall handling
From: Christoph Hellwig @ 2020-06-15 14:12 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arch, linux-s390, Parisc List, the arch/x86 maintainers,
open list:BROADCOM NVRAM DRIVER, linux-kernel@vger.kernel.org,
Linux FS-devel Mailing List, Luis Chamberlain, Al Viro,
sparclinux, linuxppc-dev, Christoph Hellwig, Linux ARM
In-Reply-To: <CAK8P3a0bRD3RzE_X6Tjzu9Tj+OhHhP+S=k6+VYODBGko8oQhew@mail.gmail.com>
On Mon, Jun 15, 2020 at 03:31:35PM +0200, Arnd Bergmann wrote:
> > #ifdef CONFIG_COMPAT
> > - if (unlikely(argv.is_compat)) {
> > + if (in_compat_syscall()) {
> > + const compat_uptr_t __user *compat_argv =
> > + compat_ptr((unsigned long)argv);
> > compat_uptr_t compat;
> >
> > - if (get_user(compat, argv.ptr.compat + nr))
> > + if (get_user(compat, compat_argv + nr))
> > return ERR_PTR(-EFAULT);
> >
> > return compat_ptr(compat);
> > }
> > #endif
>
> I would expect that the "#ifdef CONFIG_COMPAT" can be removed
> now, since compat_ptr() and in_compat_syscall() are now defined
> unconditionally. I have not tried that though.
True, I'll give it a spin.
> > +/*
> > + * x32 syscalls are listed in the same table as x86_64 ones, so we need to
> > + * define compat syscalls that are exactly the same as the native version for
> > + * the syscall table machinery to work. Sigh..
> > + */
> > +#ifdef CONFIG_X86_X32
> > COMPAT_SYSCALL_DEFINE3(execve, const char __user *, filename,
> > - const compat_uptr_t __user *, argv,
> > - const compat_uptr_t __user *, envp)
> > + const char __user *const __user *, argv,
> > + const char __user *const __user *, envp)
> > {
> > - return do_compat_execve(AT_FDCWD, getname(filename), argv, envp, 0);
> > + return do_execveat(AT_FDCWD, getname(filename), argv, envp, 0, NULL);
> > }
>
> Maybe move it to arch/x86/kernel/process_64.c or arch/x86/entry/syscall_x32.c
> to keep it out of the common code if this is needed.
I'd rather keep it in common code as that allows all the low-level
exec stuff to be marked static, and avoid us growing new pointless
compat variants through copy and paste.
smart compiler to d
> I don't really understand
> the comment, why can't this just use this?
That errors out with:
ld: arch/x86/entry/syscall_x32.o:(.rodata+0x1040): undefined reference to
`__x32_sys_execve'
ld: arch/x86/entry/syscall_x32.o:(.rodata+0x1108): undefined reference to
`__x32_sys_execveat'
make: *** [Makefile:1139: vmlinux] Error 1
^ permalink raw reply
* Re: [PATCH 2/6] exec: simplify the compat syscall handling
From: Christoph Hellwig @ 2020-06-15 14:43 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arch, linux-s390, Parisc List, the arch/x86 maintainers,
open list:BROADCOM NVRAM DRIVER, linux-kernel@vger.kernel.org,
Linux FS-devel Mailing List, Luis Chamberlain, Al Viro,
sparclinux, linuxppc-dev, Christoph Hellwig, Linux ARM
In-Reply-To: <CAK8P3a2MeZhayZWkPbd4Ckq3n410p_n808NJTwN=JjzqHRiAXg@mail.gmail.com>
On Mon, Jun 15, 2020 at 04:40:28PM +0200, Arnd Bergmann wrote:
> > ld: arch/x86/entry/syscall_x32.o:(.rodata+0x1040): undefined reference to
> > `__x32_sys_execve'
> > ld: arch/x86/entry/syscall_x32.o:(.rodata+0x1108): undefined reference to
> > `__x32_sys_execveat'
> > make: *** [Makefile:1139: vmlinux] Error 1
>
> Ah, I see: it's marked x32-only, so arch/x86/entry/syscall_x32.c
> uses the __x32 prefix instead of the __x64 one. Marking it 'common'
> instead would make it work, but also create an extra entry point
> for native processes, something that commit
> 6365b842aae4 ("x86/syscalls: Split the x32 syscalls into their own table")
> was trying to avoid.
Marking it common also doesn't compile at all because __NR_execve
and __NR_execveat get redefined in unistd_64.h. I then tried to rename
the x32 versions, which failed in yet another way. At that point I gave
up instead of digging myself into a deeper hole..
^ permalink raw reply
* Re: [PATCH 2/6] exec: simplify the compat syscall handling
From: Arnd Bergmann @ 2020-06-15 14:40 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch, linux-s390, Parisc List, the arch/x86 maintainers,
open list:BROADCOM NVRAM DRIVER, linux-kernel@vger.kernel.org,
Linux FS-devel Mailing List, Luis Chamberlain, Al Viro,
sparclinux, linuxppc-dev, Linux ARM
In-Reply-To: <20200615141239.GA12951@lst.de>
On Mon, Jun 15, 2020 at 4:12 PM Christoph Hellwig <hch@lst.de> wrote:
> On Mon, Jun 15, 2020 at 03:31:35PM +0200, Arnd Bergmann wrote:
>
> > I don't really understand
> > the comment, why can't this just use this?
>
> That errors out with:
>
> ld: arch/x86/entry/syscall_x32.o:(.rodata+0x1040): undefined reference to
> `__x32_sys_execve'
> ld: arch/x86/entry/syscall_x32.o:(.rodata+0x1108): undefined reference to
> `__x32_sys_execveat'
> make: *** [Makefile:1139: vmlinux] Error 1
Ah, I see: it's marked x32-only, so arch/x86/entry/syscall_x32.c
uses the __x32 prefix instead of the __x64 one. Marking it 'common'
instead would make it work, but also create an extra entry point
for native processes, something that commit
6365b842aae4 ("x86/syscalls: Split the x32 syscalls into their own table")
was trying to avoid.
Arnd
^ permalink raw reply
* Re: [PATCH 2/6] exec: simplify the compat syscall handling
From: Arnd Bergmann @ 2020-06-15 14:46 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch, linux-s390, Parisc List, the arch/x86 maintainers,
open list:BROADCOM NVRAM DRIVER, linux-kernel@vger.kernel.org,
Linux FS-devel Mailing List, Luis Chamberlain, Al Viro,
sparclinux, linuxppc-dev, Linux ARM
In-Reply-To: <20200615144310.GA15101@lst.de>
On Mon, Jun 15, 2020 at 4:43 PM Christoph Hellwig <hch@lst.de> wrote:
>
> On Mon, Jun 15, 2020 at 04:40:28PM +0200, Arnd Bergmann wrote:
> > > ld: arch/x86/entry/syscall_x32.o:(.rodata+0x1040): undefined reference to
> > > `__x32_sys_execve'
> > > ld: arch/x86/entry/syscall_x32.o:(.rodata+0x1108): undefined reference to
> > > `__x32_sys_execveat'
> > > make: *** [Makefile:1139: vmlinux] Error 1
> >
> > Ah, I see: it's marked x32-only, so arch/x86/entry/syscall_x32.c
> > uses the __x32 prefix instead of the __x64 one. Marking it 'common'
> > instead would make it work, but also create an extra entry point
> > for native processes, something that commit
> > 6365b842aae4 ("x86/syscalls: Split the x32 syscalls into their own table")
> > was trying to avoid.
>
> Marking it common also doesn't compile at all because __NR_execve
> and __NR_execveat get redefined in unistd_64.h. I then tried to rename
> the x32 versions, which failed in yet another way. At that point I gave
> up instead of digging myself into a deeper hole..
How about this one:
diff --git a/arch/x86/entry/syscall_x32.c b/arch/x86/entry/syscall_x32.c
index 3d8d70d3896c..0ce15807cf54 100644
--- a/arch/x86/entry/syscall_x32.c
+++ b/arch/x86/entry/syscall_x32.c
@@ -16,6 +16,9 @@
#undef __SYSCALL_X32
#undef __SYSCALL_COMMON
+#define __x32_sys_execve __x64_sys_execve
+#define __x32_sys_execveat __x64_sys_execveat
+
#define __SYSCALL_X32(nr, sym) [nr] = __x32_##sym,
#define __SYSCALL_COMMON(nr, sym) [nr] = __x64_##sym,
Still ugly, but much simpler and more localized (if it works).
Arnd
^ permalink raw reply related
* Re: [PATCH 2/6] exec: simplify the compat syscall handling
From: Brian Gerst @ 2020-06-15 14:48 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch, linux-s390, Parisc List, Arnd Bergmann,
the arch/x86 maintainers, open list:BROADCOM NVRAM DRIVER,
linux-kernel@vger.kernel.org, Linux FS-devel Mailing List,
Luis Chamberlain, Al Viro, sparclinux, linuxppc-dev, Linux ARM
In-Reply-To: <20200615141239.GA12951@lst.de>
On Mon, Jun 15, 2020 at 10:13 AM Christoph Hellwig <hch@lst.de> wrote:
>
> On Mon, Jun 15, 2020 at 03:31:35PM +0200, Arnd Bergmann wrote:
> > > #ifdef CONFIG_COMPAT
> > > - if (unlikely(argv.is_compat)) {
> > > + if (in_compat_syscall()) {
> > > + const compat_uptr_t __user *compat_argv =
> > > + compat_ptr((unsigned long)argv);
> > > compat_uptr_t compat;
> > >
> > > - if (get_user(compat, argv.ptr.compat + nr))
> > > + if (get_user(compat, compat_argv + nr))
> > > return ERR_PTR(-EFAULT);
> > >
> > > return compat_ptr(compat);
> > > }
> > > #endif
> >
> > I would expect that the "#ifdef CONFIG_COMPAT" can be removed
> > now, since compat_ptr() and in_compat_syscall() are now defined
> > unconditionally. I have not tried that though.
>
> True, I'll give it a spin.
>
> > > +/*
> > > + * x32 syscalls are listed in the same table as x86_64 ones, so we need to
> > > + * define compat syscalls that are exactly the same as the native version for
> > > + * the syscall table machinery to work. Sigh..
> > > + */
> > > +#ifdef CONFIG_X86_X32
> > > COMPAT_SYSCALL_DEFINE3(execve, const char __user *, filename,
> > > - const compat_uptr_t __user *, argv,
> > > - const compat_uptr_t __user *, envp)
> > > + const char __user *const __user *, argv,
> > > + const char __user *const __user *, envp)
> > > {
> > > - return do_compat_execve(AT_FDCWD, getname(filename), argv, envp, 0);
> > > + return do_execveat(AT_FDCWD, getname(filename), argv, envp, 0, NULL);
> > > }
> >
> > Maybe move it to arch/x86/kernel/process_64.c or arch/x86/entry/syscall_x32.c
> > to keep it out of the common code if this is needed.
>
> I'd rather keep it in common code as that allows all the low-level
> exec stuff to be marked static, and avoid us growing new pointless
> compat variants through copy and paste.
> smart compiler to d
>
> > I don't really understand
> > the comment, why can't this just use this?
>
> That errors out with:
>
> ld: arch/x86/entry/syscall_x32.o:(.rodata+0x1040): undefined reference to
> `__x32_sys_execve'
> ld: arch/x86/entry/syscall_x32.o:(.rodata+0x1108): undefined reference to
> `__x32_sys_execveat'
> make: *** [Makefile:1139: vmlinux] Error 1
I think I have a fix for this, by modifying the syscall wrappers to
add an alias for the __x32 variant to the native __x64_sys_foo().
I'll get back to you with a patch.
--
Brian Gerst
^ permalink raw reply
* Re: [PATCH net] ibmvnic: Harden device login requests
From: Thomas Falcon @ 2020-06-15 14:52 UTC (permalink / raw)
To: David Miller; +Cc: netdev, linuxppc-dev, danymadden
In-Reply-To: <20200612.141040.977929535227856014.davem@davemloft.net>
On 6/12/20 4:10 PM, David Miller wrote:
> From: Thomas Falcon <tlfalcon@linux.ibm.com>
> Date: Fri, 12 Jun 2020 13:31:39 -0500
>
>> @@ -841,13 +841,14 @@ static int ibmvnic_login(struct net_device *netdev)
>> {
>> struct ibmvnic_adapter *adapter = netdev_priv(netdev);
>> unsigned long timeout = msecs_to_jiffies(30000);
>> + int retries = 10;
>> int retry_count = 0;
>> bool retry;
>> int rc;
> Reverse christmas tree, please.
Oops, sending a v2 soon.
Thanks,
Tom
^ permalink raw reply
* Re: [PATCH v2 00/12] x86: tag application address space for devices
From: Fenghua Yu @ 2020-06-15 14:53 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Dave Hansen, H Peter Anvin, Dave Jiang, Ashok Raj, Joerg Roedel,
x86, amd-gfx, Ingo Molnar, Ravi V Shankar, Yu-cheng Yu,
Andrew Donnellan, Borislav Petkov, Sohil Mehta, Thomas Gleixner,
Tony Luck, linuxppc-dev, Felix Kuehling, linux-kernel, iommu,
Jacob Jun Pan, Frederic Barrat, David Woodhouse, Lu Baolu
In-Reply-To: <20200615075202.GI2497@hirez.programming.kicks-ass.net>
Hi, Peter,
On Mon, Jun 15, 2020 at 09:52:02AM +0200, Peter Zijlstra wrote:
> On Fri, Jun 12, 2020 at 05:41:21PM -0700, Fenghua Yu wrote:
>
> > This series only provides simple and basic support for ENQCMD and the MSR:
> > 1. Clean up type definitions (patch 1-3). These patches can be in a
> > separate series.
> > - Define "pasid" as "unsigned int" consistently (patch 1 and 2).
> > - Define "flags" as "unsigned int"
> > 2. Explain different various technical terms used in the series (patch 4).
> > 3. Enumerate support for ENQCMD in the processor (patch 5).
> > 4. Handle FPU PASID state and the MSR during context switch (patches 6-7).
> > 5. Define "pasid" in mm_struct (patch 8).
> > 5. Clear PASID state for new mm and forked and cloned thread (patch 9-10).
> > 6. Allocate and free PASID for a process (patch 11).
> > 7. Fix up the PASID MSR in #GP handler when one thread in a process
> > executes ENQCMD for the first time (patches 12).
>
> If this is per mm, should not switch_mm() update the MSR ? I'm not
> seeing that, nor do I see it explained why not.
PASID value is per mm and all threads in a process have the same PASID
value in the MSR. However, the MSR is per thread and is context switched
by XSAVES/XRSTROS in patches 6-7.
Thanks.
-Fenghua
^ permalink raw reply
* Re: [PATCH] powerpc/fsl_booke/32: fix build with CONFIG_RANDOMIZE_BASE
From: Scott Wood @ 2020-06-15 14:53 UTC (permalink / raw)
To: Arseny Solokha, Michael Ellerman, Jason Yan, linuxppc-dev
Cc: Christophe Leroy, linux-kernel, stable
In-Reply-To: <20200613162801.1946619-1-asolokha@kb.kras.ru>
On Sat, 2020-06-13 at 23:28 +0700, Arseny Solokha wrote:
> Building the current 5.8 kernel for a e500 machine with
> CONFIG_RANDOMIZE_BASE set yields the following failure:
>
> arch/powerpc/mm/nohash/kaslr_booke.c: In function 'kaslr_early_init':
> arch/powerpc/mm/nohash/kaslr_booke.c:387:2: error: implicit declaration
> of function 'flush_icache_range'; did you mean 'flush_tlb_range'?
> [-Werror=implicit-function-declaration]
>
> Indeed, including asm/cacheflush.h into kaslr_booke.c fixes the build.
>
> The issue dates back to the introduction of that file and probably went
> unnoticed because there's no in-tree defconfig with CONFIG_RANDOMIZE_BASE
> set.
>
> Fixes: 2b0e86cc5de6 ("powerpc/fsl_booke/32: implement KASLR infrastructure")
> Cc: stable@vger.kernel.org
> Signed-off-by: Arseny Solokha <asolokha@kb.kras.ru>
> ---
> arch/powerpc/mm/nohash/kaslr_booke.c | 1 +
> 1 file changed, 1 insertion(+)
Acked-by: Scott Wood <oss@buserror.net>
-Scott
^ permalink raw reply
* Re: [PATCH 2/6] exec: simplify the compat syscall handling
From: Christoph Hellwig @ 2020-06-15 15:09 UTC (permalink / raw)
To: Arnd Bergmann
Cc: linux-arch, linux-s390, Parisc List, the arch/x86 maintainers,
open list:BROADCOM NVRAM DRIVER, linux-kernel@vger.kernel.org,
Linux FS-devel Mailing List, Luis Chamberlain, Al Viro,
sparclinux, linuxppc-dev, Christoph Hellwig, Linux ARM
In-Reply-To: <CAK8P3a17h782gO65qJ9Mmz0EuiTSKQPEyr_=nvqOtnmQZuh9Kw@mail.gmail.com>
On Mon, Jun 15, 2020 at 04:46:15PM +0200, Arnd Bergmann wrote:
> How about this one:
>
> diff --git a/arch/x86/entry/syscall_x32.c b/arch/x86/entry/syscall_x32.c
> index 3d8d70d3896c..0ce15807cf54 100644
> --- a/arch/x86/entry/syscall_x32.c
> +++ b/arch/x86/entry/syscall_x32.c
> @@ -16,6 +16,9 @@
> #undef __SYSCALL_X32
> #undef __SYSCALL_COMMON
>
> +#define __x32_sys_execve __x64_sys_execve
> +#define __x32_sys_execveat __x64_sys_execveat
> +
arch/x86/entry/syscall_x32.c:19:26: error: ‘__x64_sys_execve’ undeclared here (not in a function); did you mean ‘__x32_sys_execve’?
19 | #define __x32_sys_execve __x64_sys_execve
| ^~~~~~~~~~~~~~~~
arch/x86/entry/syscall_x32.c:22:39: note: in expansion of macro ‘__x32_sys_execve’
22 | #define __SYSCALL_X32(nr, sym) [nr] = __x32_##sym,
| ^~~~~~
./arch/x86/include/generated/asm/syscalls_64.h:344:1: note: in expansion of macro ‘__SYSCALL_X32’
344 | __SYSCALL_X32(520, sys_execve)
| ^~~~~~~~~~~~~
arch/x86/entry/syscall_x32.c:20:28: error: ‘__x64_sys_execveat’ undeclared here (not in a function); did you mean ‘__x32_sys_execveat’?
20 | #define __x32_sys_execveat __x64_sys_execveat
| ^~~~~~~~~~~~~~~~~~
arch/x86/entry/syscall_x32.c:22:39: note: in expansion of macro ‘__x32_sys_execveat’
22 | #define __SYSCALL_X32(nr, sym) [nr] = __x32_##sym,
| ^~~~~~
./arch/x86/include/generated/asm/syscalls_64.h:369:1: note: in expansion of macro ‘__SYSCALL_X32’
369 | __SYSCALL_X32(545, sys_execveat)
| ^~~~~~~~~~~~~
make[2]: *** [scripts/Makefile.build:281: arch/x86/entry/syscall_x32.o] Error 1
make[1]: *** [scripts/Makefile.build:497: arch/x86/entry] Error 2
make[1]: *** Waiting for unfinished jobs....
kernel/exit.o: warning: objtool: __x64_sys_exit_group()+0x14: unreachable instruction
make: *** [Makefile:1764: arch/x86] Error 2
make: *** Waiting for unfinished jobs....
^ permalink raw reply
* Re: [PATCH] scsi: target/sbp: remove firewire SBP target driver
From: Chris Boot @ 2020-06-15 15:00 UTC (permalink / raw)
To: Finn Thain
Cc: Bart Van Assche, linux-scsi, Chuhong Yuan, linux-kernel,
Nicholas Bellinger, target-devel, Martin K . Petersen,
linux1394-devel, linuxppc-dev, Stefan Richter
In-Reply-To: <alpine.LNX.2.22.394.2006150919110.8@nippy.intranet>
On 15/06/2020 00:28, Finn Thain wrote:
> On Sun, 14 Jun 2020, Chris Boot wrote:
>
>> I expect that if someone finds this useful it can stick around (but
>> that's not my call).
>
> Who's call is that? If the patch had said "From: Martin K. Petersen" and
> "This driver is being removed because it has the following defects..."
> that would be some indication of a good-faith willingness to accept users
> as developers in the spirit of the GPL, which is what you seem to be
> alluding to (?).
If you're asking me, I'd say it was martin's call:
> SCSI TARGET SUBSYSTEM
> M: "Martin K. Petersen" <martin.petersen@oracle.com>
[...]
> F: drivers/target/
> F: include/target/
>> I just don't have the time or inclination or hardware to be able to
>> maintain it anymore, so someone else would have to pick it up.
>>
>
> Which is why most drivers get orphaned, right?
Sure, but that's not what Martin asked me to do, hence this patch.
--
Chris Boot
bootc@boo.tc
^ permalink raw reply
* [PATCH net v2] ibmvnic: Harden device login requests
From: Thomas Falcon @ 2020-06-15 15:29 UTC (permalink / raw)
To: davem; +Cc: netdev, Thomas Falcon, linuxppc-dev, danymadden
The VNIC driver's "login" command sequence is the final step
in the driver's initialization process with device firmware,
confirming the available device queue resources to be utilized
by the driver. Under high system load, firmware may not respond
to the request in a timely manner or may abort the request. In
such cases, the driver should reattempt the login command
sequence. In case of a device error, the number of retries
is bounded.
Signed-off-by: Thomas Falcon <tlfalcon@linux.ibm.com>
---
v2: declare variables in Reverse Christmas tree format
---
drivers/net/ethernet/ibm/ibmvnic.c | 21 +++++++++++++++++----
1 file changed, 17 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/ibm/ibmvnic.c b/drivers/net/ethernet/ibm/ibmvnic.c
index 1b4d04e..2baf7b3 100644
--- a/drivers/net/ethernet/ibm/ibmvnic.c
+++ b/drivers/net/ethernet/ibm/ibmvnic.c
@@ -842,12 +842,13 @@ static int ibmvnic_login(struct net_device *netdev)
struct ibmvnic_adapter *adapter = netdev_priv(netdev);
unsigned long timeout = msecs_to_jiffies(30000);
int retry_count = 0;
+ int retries = 10;
bool retry;
int rc;
do {
retry = false;
- if (retry_count > IBMVNIC_MAX_QUEUES) {
+ if (retry_count > retries) {
netdev_warn(netdev, "Login attempts exceeded\n");
return -1;
}
@@ -862,11 +863,23 @@ static int ibmvnic_login(struct net_device *netdev)
if (!wait_for_completion_timeout(&adapter->init_done,
timeout)) {
- netdev_warn(netdev, "Login timed out\n");
- return -1;
+ netdev_warn(netdev, "Login timed out, retrying...\n");
+ retry = true;
+ adapter->init_done_rc = 0;
+ retry_count++;
+ continue;
}
- if (adapter->init_done_rc == PARTIALSUCCESS) {
+ if (adapter->init_done_rc == ABORTED) {
+ netdev_warn(netdev, "Login aborted, retrying...\n");
+ retry = true;
+ adapter->init_done_rc = 0;
+ retry_count++;
+ /* FW or device may be busy, so
+ * wait a bit before retrying login
+ */
+ msleep(500);
+ } else if (adapter->init_done_rc == PARTIALSUCCESS) {
retry_count++;
release_sub_crqs(adapter, 1);
--
1.8.3.1
^ permalink raw reply related
* Re: [PATCH 2/6] exec: simplify the compat syscall handling
From: Brian Gerst @ 2020-06-15 15:33 UTC (permalink / raw)
To: Christoph Hellwig
Cc: linux-arch, linux-s390, Parisc List, Arnd Bergmann,
the arch/x86 maintainers, open list:BROADCOM NVRAM DRIVER,
linux-kernel@vger.kernel.org, Linux FS-devel Mailing List,
Luis Chamberlain, Al Viro, sparclinux, linuxppc-dev, Linux ARM
In-Reply-To: <20200615150926.GA17108@lst.de>
On Mon, Jun 15, 2020 at 11:10 AM Christoph Hellwig <hch@lst.de> wrote:
>
> On Mon, Jun 15, 2020 at 04:46:15PM +0200, Arnd Bergmann wrote:
> > How about this one:
> >
> > diff --git a/arch/x86/entry/syscall_x32.c b/arch/x86/entry/syscall_x32.c
> > index 3d8d70d3896c..0ce15807cf54 100644
> > --- a/arch/x86/entry/syscall_x32.c
> > +++ b/arch/x86/entry/syscall_x32.c
> > @@ -16,6 +16,9 @@
> > #undef __SYSCALL_X32
> > #undef __SYSCALL_COMMON
> >
> > +#define __x32_sys_execve __x64_sys_execve
> > +#define __x32_sys_execveat __x64_sys_execveat
> > +
>
>
> arch/x86/entry/syscall_x32.c:19:26: error: ‘__x64_sys_execve’ undeclared here (not in a function); did you mean ‘__x32_sys_execve’?
> 19 | #define __x32_sys_execve __x64_sys_execve
> | ^~~~~~~~~~~~~~~~
> arch/x86/entry/syscall_x32.c:22:39: note: in expansion of macro ‘__x32_sys_execve’
> 22 | #define __SYSCALL_X32(nr, sym) [nr] = __x32_##sym,
> | ^~~~~~
> ./arch/x86/include/generated/asm/syscalls_64.h:344:1: note: in expansion of macro ‘__SYSCALL_X32’
> 344 | __SYSCALL_X32(520, sys_execve)
> | ^~~~~~~~~~~~~
> arch/x86/entry/syscall_x32.c:20:28: error: ‘__x64_sys_execveat’ undeclared here (not in a function); did you mean ‘__x32_sys_execveat’?
> 20 | #define __x32_sys_execveat __x64_sys_execveat
> | ^~~~~~~~~~~~~~~~~~
> arch/x86/entry/syscall_x32.c:22:39: note: in expansion of macro ‘__x32_sys_execveat’
> 22 | #define __SYSCALL_X32(nr, sym) [nr] = __x32_##sym,
> | ^~~~~~
> ./arch/x86/include/generated/asm/syscalls_64.h:369:1: note: in expansion of macro ‘__SYSCALL_X32’
> 369 | __SYSCALL_X32(545, sys_execveat)
> | ^~~~~~~~~~~~~
> make[2]: *** [scripts/Makefile.build:281: arch/x86/entry/syscall_x32.o] Error 1
> make[1]: *** [scripts/Makefile.build:497: arch/x86/entry] Error 2
> make[1]: *** Waiting for unfinished jobs....
> kernel/exit.o: warning: objtool: __x64_sys_exit_group()+0x14: unreachable instruction
> make: *** [Makefile:1764: arch/x86] Error 2
> make: *** Waiting for unfinished jobs....
If you move those aliases above all the __SYSCALL_* defines it will
work, since that will get the forward declaration too. This would be
the simplest workaround.
--
Brian Gerst
^ permalink raw reply
* Re: [PATCH v2 12/12] x86/traps: Fix up invalid PASID
From: Fenghua Yu @ 2020-06-15 15:48 UTC (permalink / raw)
To: Peter Zijlstra
Cc: Dave Hansen, H Peter Anvin, Dave Jiang, Ashok Raj, Joerg Roedel,
x86, amd-gfx, Ingo Molnar, Ravi V Shankar, Yu-cheng Yu,
Andrew Donnellan, Borislav Petkov, Sohil Mehta, Thomas Gleixner,
Tony Luck, linuxppc-dev, Felix Kuehling, linux-kernel, iommu,
Jacob Jun Pan, Frederic Barrat, David Woodhouse, Lu Baolu
In-Reply-To: <20200615075649.GK2497@hirez.programming.kicks-ass.net>
Hi, Peter,
On Mon, Jun 15, 2020 at 09:56:49AM +0200, Peter Zijlstra wrote:
> On Fri, Jun 12, 2020 at 05:41:33PM -0700, Fenghua Yu wrote:
> > +/*
> > + * Apply some heuristics to see if the #GP fault was caused by a thread
> > + * that hasn't had the IA32_PASID MSR initialized. If it looks like that
> > + * is the problem, try initializing the IA32_PASID MSR. If the heuristic
> > + * guesses incorrectly, take one more #GP fault.
>
> How is that going to help? Aren't we then going to run this same
> heuristic again and again and again?
The heuristic always initializes the MSR with the per mm PASID IIF the
mm has a valid PASID but the MSR doesn't have one. This heuristic usually
happens only once on the first #GP in a thread.
If the next #GP still comes in, the heuristic finds out the MSR already
has a valid PASID and thus will not fixup the MSR any more. The fixup()
returns "false" and lets others to handle the #GP.
So the heuristic will be executed once (at most) and won't be executed
again and again.
>
> > + */
> > +bool __fixup_pasid_exception(void)
> > +{
> > + u64 pasid_msr;
> > + unsigned int pasid;
> > +
> > + /*
> > + * This function is called only when this #GP was triggered from user
> > + * space. So the mm cannot be NULL.
> > + */
> > + pasid = current->mm->pasid;
> > + /* If the mm doesn't have a valid PASID, then can't help. */
> > + if (invalid_pasid(pasid))
> > + return false;
> > +
> > + /*
> > + * Since IRQ is disabled now, the current task still owns the FPU on
>
> That's just weird and confusing. What you want to say is that you rely
> on the exception disabling the interrupt.
I checked SDM again. You are right. #GP can be interrupted by machine check
or other interrupts. So I cannot assume the current task still owns the FPU.
Instead of directly rdmsr() and wrmsr(), I will add helpers that can access
either the MSR on the processor or the PASID state in the memory.
>
> > + * this CPU and the PASID MSR can be directly accessed.
> > + *
> > + * If the MSR has a valid PASID, the #GP must be for some other reason.
> > + *
> > + * If rdmsr() is really a performance issue, a TIF_ flag may be
> > + * added to check if the thread has a valid PASID instead of rdmsr().
>
> I don't understand any of this. Nobody except us writes to this MSR, we
> should bloody well know what's in it. What gives?
Patch 4 describes how to manage the MSR and patch 7 describes the format
of the MSR (20-bit PASID value and bit 31 is valid bit).
Are they sufficient to help? Or do you mean something else?
> > + */
> > + rdmsrl(MSR_IA32_PASID, pasid_msr);
> > + if (pasid_msr & MSR_IA32_PASID_VALID)
> > + return false;
> > +
> > + /* Fix up the MSR if the MSR doesn't have a valid PASID. */
> > + wrmsrl(MSR_IA32_PASID, pasid | MSR_IA32_PASID_VALID);
> > +
> > + return true;
> > +}
> > --
> > 2.19.1
> >
Thanks.
-Fenghua
^ permalink raw reply
* Re: [PATCH v2 12/12] x86/traps: Fix up invalid PASID
From: Peter Zijlstra @ 2020-06-15 16:03 UTC (permalink / raw)
To: Fenghua Yu
Cc: Dave Hansen, H Peter Anvin, Dave Jiang, Ashok Raj, Joerg Roedel,
x86, amd-gfx, Ingo Molnar, Ravi V Shankar, Yu-cheng Yu,
Andrew Donnellan, Borislav Petkov, Sohil Mehta, Thomas Gleixner,
Tony Luck, linuxppc-dev, Felix Kuehling, linux-kernel, iommu,
Jacob Jun Pan, Frederic Barrat, David Woodhouse, Lu Baolu
In-Reply-To: <20200615154854.GB13792@romley-ivt3.sc.intel.com>
On Mon, Jun 15, 2020 at 08:48:54AM -0700, Fenghua Yu wrote:
> Hi, Peter,
> On Mon, Jun 15, 2020 at 09:56:49AM +0200, Peter Zijlstra wrote:
> > On Fri, Jun 12, 2020 at 05:41:33PM -0700, Fenghua Yu wrote:
> > > +/*
> > > + * Apply some heuristics to see if the #GP fault was caused by a thread
> > > + * that hasn't had the IA32_PASID MSR initialized. If it looks like that
> > > + * is the problem, try initializing the IA32_PASID MSR. If the heuristic
> > > + * guesses incorrectly, take one more #GP fault.
> >
> > How is that going to help? Aren't we then going to run this same
> > heuristic again and again and again?
>
> The heuristic always initializes the MSR with the per mm PASID IIF the
> mm has a valid PASID but the MSR doesn't have one. This heuristic usually
> happens only once on the first #GP in a thread.
But it doesn't guarantee the PASID is the right one. Suppose both the mm
has a PASID and the MSR has a VALID one, but the MSR isn't the mm one.
Then we NO-OP. So if the exception was due to us having the wrong PASID,
we stuck.
> If the next #GP still comes in, the heuristic finds out the MSR already
> has a valid PASID and thus will not fixup the MSR any more. The fixup()
> returns "false" and lets others to handle the #GP.
>
> So the heuristic will be executed once (at most) and won't be executed
> again and again.
So I get that you want to set the PASID on-demand, but I find this all
really weird code to make that happen.
> > > +bool __fixup_pasid_exception(void)
> > > +{
> > > + u64 pasid_msr;
> > > + unsigned int pasid;
> > > +
> > > + /*
> > > + * This function is called only when this #GP was triggered from user
> > > + * space. So the mm cannot be NULL.
> > > + */
> > > + pasid = current->mm->pasid;
> > > + /* If the mm doesn't have a valid PASID, then can't help. */
> > > + if (invalid_pasid(pasid))
> > > + return false;
> > > +
> > > + /*
> > > + * Since IRQ is disabled now, the current task still owns the FPU on
> >
> > That's just weird and confusing. What you want to say is that you rely
> > on the exception disabling the interrupt.
>
> I checked SDM again. You are right. #GP can be interrupted by machine check
> or other interrupts. So I cannot assume the current task still owns the FPU.
> Instead of directly rdmsr() and wrmsr(), I will add helpers that can access
> either the MSR on the processor or the PASID state in the memory.
That's not in fact what I meant, but yes, you can take exceptions while
!IF just fine.
> > > + * this CPU and the PASID MSR can be directly accessed.
> > > + *
> > > + * If the MSR has a valid PASID, the #GP must be for some other reason.
> > > + *
> > > + * If rdmsr() is really a performance issue, a TIF_ flag may be
> > > + * added to check if the thread has a valid PASID instead of rdmsr().
> >
> > I don't understand any of this. Nobody except us writes to this MSR, we
> > should bloody well know what's in it. What gives?
>
> Patch 4 describes how to manage the MSR and patch 7 describes the format
> of the MSR (20-bit PASID value and bit 31 is valid bit).
>
> Are they sufficient to help? Or do you mean something else?
I don't get why you need a rdmsr here, or why not having one would
require a TIF flag. Is that because this MSR is XSAVE/XRSTOR managed?
> > > + */
> > > + rdmsrl(MSR_IA32_PASID, pasid_msr);
> > > + if (pasid_msr & MSR_IA32_PASID_VALID)
> > > + return false;
> > > +
> > > + /* Fix up the MSR if the MSR doesn't have a valid PASID. */
> > > + wrmsrl(MSR_IA32_PASID, pasid | MSR_IA32_PASID_VALID);
How much more expensive is the wrmsr over the rdmsr? Can't we just
unconditionally write the current PASID and call it a day?
^ permalink raw reply
* Re: [PATCH 2/6] exec: simplify the compat syscall handling
From: Christoph Hellwig @ 2020-06-15 16:41 UTC (permalink / raw)
To: Brian Gerst
Cc: linux-arch, linux-s390, Parisc List, Arnd Bergmann,
the arch/x86 maintainers, open list:BROADCOM NVRAM DRIVER,
linux-kernel@vger.kernel.org, Linux FS-devel Mailing List,
Luis Chamberlain, Al Viro, sparclinux, linuxppc-dev,
Christoph Hellwig, Linux ARM
In-Reply-To: <CAMzpN2htYX7s6pmRg-c8qwZL1f1_+sB=ztDG_L=617hWsm-=8g@mail.gmail.com>
On Mon, Jun 15, 2020 at 11:33:49AM -0400, Brian Gerst wrote:
> If you move those aliases above all the __SYSCALL_* defines it will
> work, since that will get the forward declaration too. This would be
> the simplest workaround.
That compiles and also passes my exaustive x32 tests (chroot + ls -l).
This is the updated version:
http://git.infradead.org/users/hch/misc.git/commitdiff/c8d319711ad2f53be003ae8e9be08519068bdcee
^ permalink raw reply
* Re: [PATCH v1 4/4] KVM: PPC: Book3S HV: migrate hot plugged memory
From: Laurent Dufour @ 2020-06-15 17:00 UTC (permalink / raw)
To: Ram Pai, kvm-ppc, linuxppc-dev
Cc: cclaudio, bharata, aneesh.kumar, sukadev, bauerman, david
In-Reply-To: <1590892071-25549-5-git-send-email-linuxram@us.ibm.com>
Le 31/05/2020 à 04:27, Ram Pai a écrit :
> From: Laurent Dufour <ldufour@linux.ibm.com>
>
> When a memory slot is hot plugged to a SVM, GFNs associated with that
> memory slot automatically default to secure GFN. Hence migrate the
> PFNs associated with these GFNs to device-PFNs.
>
> uv_migrate_mem_slot() is called to achieve that. It will not call
> UV_PAGE_IN since this request is ignored by the Ultravisor.
> NOTE: Ultravisor does not trust any page content provided by
> the Hypervisor, ones the VM turns secure.
>
> Cc: Paul Mackerras <paulus@ozlabs.org>
> Cc: Benjamin Herrenschmidt <benh@kernel.crashing.org>
> Cc: Michael Ellerman <mpe@ellerman.id.au>
> Cc: Bharata B Rao <bharata@linux.ibm.com>
> Cc: Aneesh Kumar K.V <aneesh.kumar@linux.ibm.com>
> Cc: Sukadev Bhattiprolu <sukadev@linux.vnet.ibm.com>
> Cc: Laurent Dufour <ldufour@linux.ibm.com>
> Cc: Thiago Jung Bauermann <bauerman@linux.ibm.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Claudio Carvalho <cclaudio@linux.ibm.com>
> Cc: kvm-ppc@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Signed-off-by: Ram Pai <linuxram@us.ibm.com>
> (fixed merge conflicts. Modified the commit message)
> Signed-off-by: Laurent Dufour <ldufour@linux.ibm.com>
> ---
> arch/powerpc/include/asm/kvm_book3s_uvmem.h | 4 ++++
> arch/powerpc/kvm/book3s_hv.c | 11 +++++++----
> arch/powerpc/kvm/book3s_hv_uvmem.c | 3 +--
> 3 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/include/asm/kvm_book3s_uvmem.h b/arch/powerpc/include/asm/kvm_book3s_uvmem.h
> index f0c5708..2ec2e5afb 100644
> --- a/arch/powerpc/include/asm/kvm_book3s_uvmem.h
> +++ b/arch/powerpc/include/asm/kvm_book3s_uvmem.h
> @@ -23,6 +23,7 @@ unsigned long kvmppc_h_svm_page_out(struct kvm *kvm,
> void kvmppc_uvmem_drop_pages(const struct kvm_memory_slot *free,
> struct kvm *kvm, bool skip_page_out,
> bool purge_gfn);
> +int uv_migrate_mem_slot(struct kvm *kvm, const struct kvm_memory_slot *memslot);
> #else
> static inline int kvmppc_uvmem_init(void)
> {
> @@ -78,5 +79,8 @@ static inline int kvmppc_send_page_to_uv(struct kvm *kvm, unsigned long gfn)
> kvmppc_uvmem_drop_pages(const struct kvm_memory_slot *free,
> struct kvm *kvm, bool skip_page_out,
> bool purge_gfn) { }
> +
> +static int uv_migrate_mem_slot(struct kvm *kvm,
> + const struct kvm_memory_slot *memslot);
> #endif /* CONFIG_PPC_UV */
> #endif /* __ASM_KVM_BOOK3S_UVMEM_H__ */
> diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
> index 4c62bfe..604d062 100644
> --- a/arch/powerpc/kvm/book3s_hv.c
> +++ b/arch/powerpc/kvm/book3s_hv.c
> @@ -4516,13 +4516,16 @@ static void kvmppc_core_commit_memory_region_hv(struct kvm *kvm,
> case KVM_MR_CREATE:
> if (kvmppc_uvmem_slot_init(kvm, new))
> return;
> - uv_register_mem_slot(kvm->arch.lpid,
> - new->base_gfn << PAGE_SHIFT,
> - new->npages * PAGE_SIZE,
> - 0, new->id);
> + if (uv_register_mem_slot(kvm->arch.lpid,
> + new->base_gfn << PAGE_SHIFT,
> + new->npages * PAGE_SIZE,
> + 0, new->id))
> + return;
> + uv_migrate_mem_slot(kvm, new);
> break;
> case KVM_MR_DELETE:
> uv_unregister_mem_slot(kvm->arch.lpid, old->id);
> + kvmppc_uvmem_drop_pages(old, kvm, true, true);
My mistake, kvmppc_radix_flush_memslot() called just before is already
triggering the call to kvmppc_uvmem_drop_pages(), so that call is useless.
You should remove it in your v2.
> kvmppc_uvmem_slot_free(kvm, old);
> break;
> default:
> diff --git a/arch/powerpc/kvm/book3s_hv_uvmem.c b/arch/powerpc/kvm/book3s_hv_uvmem.c
> index 36dda1d..1fa5f2a 100644
> --- a/arch/powerpc/kvm/book3s_hv_uvmem.c
> +++ b/arch/powerpc/kvm/book3s_hv_uvmem.c
> @@ -377,8 +377,7 @@ static int kvmppc_svm_migrate_page(struct vm_area_struct *vma,
> return ret;
> }
>
> -static int uv_migrate_mem_slot(struct kvm *kvm,
> - const struct kvm_memory_slot *memslot)
> +int uv_migrate_mem_slot(struct kvm *kvm, const struct kvm_memory_slot *memslot)
> {
> unsigned long gfn = memslot->base_gfn;
> unsigned long end;
>
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox