Base of uaccess pile - series from Christophe Leroy adding
user_{read,write}_access_begin(). Sat in ppc tree all along,
in vfs.git it's #uaccess.base. No changes since the last time
it got posted, repeated for reference, based at v5.7-rc1
Christophe Leroy (3):
uaccess: Add user_read_access_begin/end and user_write_access_begin/end
uaccess: Selectively open read or write user access
drm/i915/gem: Replace user_access_begin by user_write_access_begin
drivers/gpu/drm/i915/gem/i915_gem_execbuffer.c | 5 +++--
fs/readdir.c | 12 ++++++------
include/linux/uaccess.h | 8 ++++++++
kernel/compat.c | 12 ++++++------
kernel/exit.c | 12 ++++++------
lib/strncpy_from_user.c | 4 ++--
lib/strnlen_user.c | 4 ++--
lib/usercopy.c | 6 +++---
8 files changed, 36 insertions(+), 27 deletions(-)
readdir.c uaccess stuff. Lives in #uaccess.readdir, based at
#uaccess.base, gets the rest of fs/readdir.c in sync with
getdents()/getdents64().
Al Viro (3):
switch readdir(2) to unsafe_copy_dirent_name()
readdir.c: get compat_filldir() more or less in sync with filldir()
readdir.c: get rid of the last __put_user(), drop now-useless access_ok()
fs/readdir.c | 92 +++++++++++++++++++++++++++++-------------------------------
1 file changed, 44 insertions(+), 48 deletions(-)
From: Christophe Leroy <[email protected]>
Some architectures like powerpc64 have the capability to separate
read access and write access protection.
For get_user() and copy_from_user(), powerpc64 only open read access.
For put_user() and copy_to_user(), powerpc64 only open write access.
But when using unsafe_get_user() or unsafe_put_user(),
user_access_begin open both read and write.
Other architectures like powerpc book3s 32 bits only allow write
access protection. And on this architecture protection is an heavy
operation as it requires locking/unlocking per segment of 256Mbytes.
On those architecture it is therefore desirable to do the unlocking
only for write access. (Note that book3s/32 ranges from very old
powermac from the 90's with powerpc 601 processor, till modern
ADSL boxes with PowerQuicc II processors for instance so it
is still worth considering.)
In order to avoid any risk based of hacking some variable parameters
passed to user_access_begin/end that would allow hacking and
leaving user access open or opening too much, it is preferable to
use dedicated static functions that can't be overridden.
Add a user_read_access_begin and user_read_access_end to only open
read access.
Add a user_write_access_begin and user_write_access_end to only open
write access.
By default, when undefined, those new access helpers default on the
existing user_access_begin and user_access_end.
Signed-off-by: Christophe Leroy <[email protected]>
Reviewed-by: Kees Cook <[email protected]>
Signed-off-by: Michael Ellerman <[email protected]>
Link: https://lore.kernel.org/r/36e43241c7f043a24b5069e78c6a7edd11043be5.1585898438.git.christophe.leroy@c-s.fr
---
include/linux/uaccess.h | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/include/linux/uaccess.h b/include/linux/uaccess.h
index 67f016010aad..9861c89f93be 100644
--- a/include/linux/uaccess.h
+++ b/include/linux/uaccess.h
@@ -378,6 +378,14 @@ extern long strnlen_unsafe_user(const void __user *unsafe_addr, long count);
static inline unsigned long user_access_save(void) { return 0UL; }
static inline void user_access_restore(unsigned long flags) { }
#endif
+#ifndef user_write_access_begin
+#define user_write_access_begin user_access_begin
+#define user_write_access_end user_access_end
+#endif
+#ifndef user_read_access_begin
+#define user_read_access_begin user_access_begin
+#define user_read_access_end user_access_end
+#endif
#ifdef CONFIG_HARDENED_USERCOPY
void usercopy_warn(const char *name, const char *detail, bool to_user,
--
2.11.0
A couple of __copy_from_user()-related patches. That's
the stuff that didn't fit anywhere else. The end goal is
to kill uses of that thing outside of arch/* and we are not
far from getting there.
Branch in uaccess.__copy_from_user, based at v5.7-rc1.
Al Viro (2):
firewire: switch ioctl_queue_iso to use of copy_from_user()
pstore: switch to copy_from_user()
drivers/firewire/core-cdev.c | 4 +---
fs/pstore/ram_core.c | 4 +---
2 files changed, 2 insertions(+), 6 deletions(-)
From: Al Viro <[email protected]>
no point trying to do access_ok() for all those __copy_from_user()
at once.
Signed-off-by: Al Viro <[email protected]>
---
drivers/firewire/core-cdev.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/drivers/firewire/core-cdev.c b/drivers/firewire/core-cdev.c
index 6e291d8f3a27..c7ea4f2d5ca6 100644
--- a/drivers/firewire/core-cdev.c
+++ b/drivers/firewire/core-cdev.c
@@ -1081,8 +1081,6 @@ static int ioctl_queue_iso(struct client *client, union ioctl_arg *arg)
return -EINVAL;
p = (struct fw_cdev_iso_packet __user *)u64_to_uptr(a->packets);
- if (!access_ok(p, a->size))
- return -EFAULT;
end = (void __user *)p + a->size;
count = 0;
@@ -1120,7 +1118,7 @@ static int ioctl_queue_iso(struct client *client, union ioctl_arg *arg)
&p->header[transmit_header_bytes / 4];
if (next > end)
return -EINVAL;
- if (__copy_from_user
+ if (copy_from_user
(u.packet.header, p->header, transmit_header_bytes))
return -EFAULT;
if (u.packet.skip && ctx->type == FW_ISO_CONTEXT_TRANSMIT &&
--
2.11.0
From: Al Viro <[email protected]>
don't bother trying to do bulk access_ok()
Signed-off-by: Al Viro <[email protected]>
---
fs/pstore/ram_core.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/fs/pstore/ram_core.c b/fs/pstore/ram_core.c
index c917c191e78c..aa8e0b65ff1a 100644
--- a/fs/pstore/ram_core.c
+++ b/fs/pstore/ram_core.c
@@ -283,7 +283,7 @@ static int notrace persistent_ram_update_user(struct persistent_ram_zone *prz,
const void __user *s, unsigned int start, unsigned int count)
{
struct persistent_ram_buffer *buffer = prz->buffer;
- int ret = unlikely(__copy_from_user(buffer->data + start, s, count)) ?
+ int ret = unlikely(copy_from_user(buffer->data + start, s, count)) ?
-EFAULT : 0;
persistent_ram_update_ecc(prz, start, count);
return ret;
@@ -348,8 +348,6 @@ int notrace persistent_ram_write_user(struct persistent_ram_zone *prz,
int rem, ret = 0, c = count;
size_t start;
- if (unlikely(!access_ok(s, count)))
- return -EFAULT;
if (unlikely(c > prz->buffer_size)) {
s += c - prz->buffer_size;
c = prz->buffer_size;
--
2.11.0
Similar for __copy_to_user(); patches that didn't
fit anywhere else (e.g. into readdir series). The goal,
again, is to get rid of __copy_to_user() outside of arch/*.
We are not quite there (e.g. there's regset crap, some
KVM/vhost-related callers that want different predicate
instead of access_ok()), but it's getting fairly close.
Branch is #uaccess.__copy_to_user, based at
v5.7-rc1.
Al Viro (2):
esas2r: don't bother with __copy_to_user()
dlmfs: convert dlmfs_file_read() to copy_to_user()
drivers/scsi/esas2r/esas2r_ioctl.c | 2 +-
fs/ocfs2/dlmfs/dlmfs.c | 33 ++++++++++++++-------------------
2 files changed, 15 insertions(+), 20 deletions(-)
Similar misc patches dealing with __put_user()
eliminations not fitting into other series (e.g. quite
a bit went into net-next already, then there's readdir
series and comedi one - the last one is the single
biggest pile of __put_user() outside of arch/*, etc.).
Branch in #uaccess.__put_user, based at v5.7-rc1
Al Viro (3):
compat sysinfo(2): don't bother with field-by-field copyout
scsi_ioctl.c: switch SCSI_IOCTL_GET_IDLUN to copy_to_user()
pcm_native: result of put_user() needs to be checked
drivers/scsi/scsi_ioctl.c | 20 ++++++++++----------
kernel/sys.c | 33 +++++++++++++++++----------------
sound/core/pcm_native.c | 12 ++++++++----
3 files changed, 35 insertions(+), 30 deletions(-)
From: Al Viro <[email protected]>
Signed-off-by: Al Viro <[email protected]>
---
kernel/sys.c | 33 +++++++++++++++++----------------
1 file changed, 17 insertions(+), 16 deletions(-)
diff --git a/kernel/sys.c b/kernel/sys.c
index d325f3ab624a..b4a0324a0699 100644
--- a/kernel/sys.c
+++ b/kernel/sys.c
@@ -2634,6 +2634,7 @@ struct compat_sysinfo {
COMPAT_SYSCALL_DEFINE1(sysinfo, struct compat_sysinfo __user *, info)
{
struct sysinfo s;
+ struct compat_sysinfo s_32;
do_sysinfo(&s);
@@ -2658,23 +2659,23 @@ COMPAT_SYSCALL_DEFINE1(sysinfo, struct compat_sysinfo __user *, info)
s.freehigh >>= bitcount;
}
- if (!access_ok(info, sizeof(struct compat_sysinfo)) ||
- __put_user(s.uptime, &info->uptime) ||
- __put_user(s.loads[0], &info->loads[0]) ||
- __put_user(s.loads[1], &info->loads[1]) ||
- __put_user(s.loads[2], &info->loads[2]) ||
- __put_user(s.totalram, &info->totalram) ||
- __put_user(s.freeram, &info->freeram) ||
- __put_user(s.sharedram, &info->sharedram) ||
- __put_user(s.bufferram, &info->bufferram) ||
- __put_user(s.totalswap, &info->totalswap) ||
- __put_user(s.freeswap, &info->freeswap) ||
- __put_user(s.procs, &info->procs) ||
- __put_user(s.totalhigh, &info->totalhigh) ||
- __put_user(s.freehigh, &info->freehigh) ||
- __put_user(s.mem_unit, &info->mem_unit))
+ memset(&s_32, 0, sizeof(s_32));
+ s_32.uptime = s.uptime;
+ s_32.loads[0] = s.loads[0];
+ s_32.loads[1] = s.loads[1];
+ s_32.loads[2] = s.loads[2];
+ s_32.totalram = s.totalram;
+ s_32.freeram = s.freeram;
+ s_32.sharedram = s.sharedram;
+ s_32.bufferram = s.bufferram;
+ s_32.totalswap = s.totalswap;
+ s_32.freeswap = s.freeswap;
+ s_32.procs = s.procs;
+ s_32.totalhigh = s.totalhigh;
+ s_32.freehigh = s.freehigh;
+ s_32.mem_unit = s.mem_unit;
+ if (copy_to_user(info, &s_32, sizeof(s_32)))
return -EFAULT;
-
return 0;
}
#endif /* CONFIG_COMPAT */
--
2.11.0
From: Al Viro <[email protected]>
... and no, __put_user() doesn't help here - skipping
access_ok() on the second call does not remove the
possibility of page having become unmapped or r/o
in the meanwhile
Signed-off-by: Al Viro <[email protected]>
---
sound/core/pcm_native.c | 12 ++++++++----
1 file changed, 8 insertions(+), 4 deletions(-)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index aef860256278..47838f57a647 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -3093,7 +3093,8 @@ static int snd_pcm_xferi_frames_ioctl(struct snd_pcm_substream *substream,
result = snd_pcm_lib_write(substream, xferi.buf, xferi.frames);
else
result = snd_pcm_lib_read(substream, xferi.buf, xferi.frames);
- __put_user(result, &_xferi->result);
+ if (put_user(result, &_xferi->result))
+ return -EFAULT;
return result < 0 ? result : 0;
}
@@ -3122,7 +3123,8 @@ static int snd_pcm_xfern_frames_ioctl(struct snd_pcm_substream *substream,
else
result = snd_pcm_lib_readv(substream, bufs, xfern.frames);
kfree(bufs);
- __put_user(result, &_xfern->result);
+ if (put_user(result, &_xfern->result))
+ return -EFAULT;
return result < 0 ? result : 0;
}
@@ -3137,7 +3139,8 @@ static int snd_pcm_rewind_ioctl(struct snd_pcm_substream *substream,
if (put_user(0, _frames))
return -EFAULT;
result = snd_pcm_rewind(substream, frames);
- __put_user(result, _frames);
+ if (put_user(result, _frames))
+ return -EFAULT;
return result < 0 ? result : 0;
}
@@ -3152,7 +3155,8 @@ static int snd_pcm_forward_ioctl(struct snd_pcm_substream *substream,
if (put_user(0, _frames))
return -EFAULT;
result = snd_pcm_forward(substream, frames);
- __put_user(result, _frames);
+ if (put_user(result, _frames))
+ return -EFAULT;
return result < 0 ? result : 0;
}
--
2.11.0
The way comedi compat ioctls are done is wrong.
Instead of having ->compat_ioctl() copying the 32bit
stuff in, then passing the kernel copies to helpers shared
with native ->ioctl() and doing copyout with conversion if
needed, it's playing silly buggers with creating a 64bit
copy on user stack, then calling native ioctl (which copies
that copy into the kernel), then fetching it from user stack,
converting to 32bit variant and copying that to user.
Extra headache for no good reason. And the single
largest remaining pile of __put_user()/__get_user() this side
of arch/*. IMO compat_alloc_user_space() should die...
NOTE: this is only compile-tested - I simply don't
have the hardware in question.
Anyway, the branch lives in #uaccess.comedi, based
at v5.7-rc1
Al Viro (10):
comedi: move compat ioctl handling to native fops
comedi: get rid of indirection via translated_ioctl()
comedi: get rid of compat_alloc_user_space() mess in COMEDI_CHANINFO compat
comedi: get rid of compat_alloc_user_space() mess in COMEDI_RANGEINFO compat
comedi: get rid of compat_alloc_user_space() mess in COMEDI_INSN compat
comedi: get rid of compat_alloc_user_space() mess in COMEDI_INSNLIST compat
comedi: lift copy_from_user() into callers of __comedi_get_user_cmd()
comedi: do_cmdtest_ioctl(): lift copyin/copyout into the caller
comedi: do_cmd_ioctl(): lift copyin/copyout into the caller
comedi: get rid of compat_alloc_user_space() mess in COMEDI_CMD{,TEST} compat
drivers/staging/comedi/Makefile | 1 -
drivers/staging/comedi/comedi_compat32.c | 455 -------------------------
drivers/staging/comedi/comedi_compat32.h | 28 --
drivers/staging/comedi/comedi_fops.c | 563 +++++++++++++++++++++++++------
drivers/staging/comedi/comedi_internal.h | 2 +-
drivers/staging/comedi/range.c | 17 +-
6 files changed, 466 insertions(+), 600 deletions(-)
delete mode 100644 drivers/staging/comedi/comedi_compat32.c
delete mode 100644 drivers/staging/comedi/comedi_compat32.h
From: Al Viro <[email protected]>
mechanical move
Signed-off-by: Al Viro <[email protected]>
---
drivers/staging/comedi/Makefile | 1 -
drivers/staging/comedi/comedi_compat32.c | 455 -------------------------------
drivers/staging/comedi/comedi_compat32.h | 28 --
drivers/staging/comedi/comedi_fops.c | 451 +++++++++++++++++++++++++++++-
4 files changed, 448 insertions(+), 487 deletions(-)
delete mode 100644 drivers/staging/comedi/comedi_compat32.c
delete mode 100644 drivers/staging/comedi/comedi_compat32.h
diff --git a/drivers/staging/comedi/Makefile b/drivers/staging/comedi/Makefile
index 6af5da3b4315..072ed83a5a6a 100644
--- a/drivers/staging/comedi/Makefile
+++ b/drivers/staging/comedi/Makefile
@@ -4,7 +4,6 @@ ccflags-$(CONFIG_COMEDI_DEBUG) := -DDEBUG
comedi-y := comedi_fops.o range.o drivers.o \
comedi_buf.o
comedi-$(CONFIG_PROC_FS) += proc.o
-comedi-$(CONFIG_COMPAT) += comedi_compat32.o
obj-$(CONFIG_COMEDI_PCI_DRIVERS) += comedi_pci.o
obj-$(CONFIG_COMEDI_PCMCIA_DRIVERS) += comedi_pcmcia.o
diff --git a/drivers/staging/comedi/comedi_compat32.c b/drivers/staging/comedi/comedi_compat32.c
deleted file mode 100644
index 36a3564ba1fb..000000000000
--- a/drivers/staging/comedi/comedi_compat32.c
+++ /dev/null
@@ -1,455 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0+
-/*
- * comedi/comedi_compat32.c
- * 32-bit ioctl compatibility for 64-bit comedi kernel module.
- *
- * Author: Ian Abbott, MEV Ltd. <[email protected]>
- * Copyright (C) 2007 MEV Ltd. <http://www.mev.co.uk/>
- *
- * COMEDI - Linux Control and Measurement Device Interface
- * Copyright (C) 1997-2007 David A. Schleef <[email protected]>
- */
-
-#include <linux/uaccess.h>
-#include <linux/compat.h>
-#include <linux/fs.h>
-#include "comedi.h"
-#include "comedi_compat32.h"
-
-#define COMEDI32_CHANINFO _IOR(CIO, 3, struct comedi32_chaninfo_struct)
-#define COMEDI32_RANGEINFO _IOR(CIO, 8, struct comedi32_rangeinfo_struct)
-/*
- * N.B. COMEDI32_CMD and COMEDI_CMD ought to use _IOWR, not _IOR.
- * It's too late to change it now, but it only affects the command number.
- */
-#define COMEDI32_CMD _IOR(CIO, 9, struct comedi32_cmd_struct)
-/*
- * N.B. COMEDI32_CMDTEST and COMEDI_CMDTEST ought to use _IOWR, not _IOR.
- * It's too late to change it now, but it only affects the command number.
- */
-#define COMEDI32_CMDTEST _IOR(CIO, 10, struct comedi32_cmd_struct)
-#define COMEDI32_INSNLIST _IOR(CIO, 11, struct comedi32_insnlist_struct)
-#define COMEDI32_INSN _IOR(CIO, 12, struct comedi32_insn_struct)
-
-struct comedi32_chaninfo_struct {
- unsigned int subdev;
- compat_uptr_t maxdata_list; /* 32-bit 'unsigned int *' */
- compat_uptr_t flaglist; /* 32-bit 'unsigned int *' */
- compat_uptr_t rangelist; /* 32-bit 'unsigned int *' */
- unsigned int unused[4];
-};
-
-struct comedi32_rangeinfo_struct {
- unsigned int range_type;
- compat_uptr_t range_ptr; /* 32-bit 'void *' */
-};
-
-struct comedi32_cmd_struct {
- unsigned int subdev;
- unsigned int flags;
- unsigned int start_src;
- unsigned int start_arg;
- unsigned int scan_begin_src;
- unsigned int scan_begin_arg;
- unsigned int convert_src;
- unsigned int convert_arg;
- unsigned int scan_end_src;
- unsigned int scan_end_arg;
- unsigned int stop_src;
- unsigned int stop_arg;
- compat_uptr_t chanlist; /* 32-bit 'unsigned int *' */
- unsigned int chanlist_len;
- compat_uptr_t data; /* 32-bit 'short *' */
- unsigned int data_len;
-};
-
-struct comedi32_insn_struct {
- unsigned int insn;
- unsigned int n;
- compat_uptr_t data; /* 32-bit 'unsigned int *' */
- unsigned int subdev;
- unsigned int chanspec;
- unsigned int unused[3];
-};
-
-struct comedi32_insnlist_struct {
- unsigned int n_insns;
- compat_uptr_t insns; /* 32-bit 'struct comedi_insn *' */
-};
-
-/* Handle translated ioctl. */
-static int translated_ioctl(struct file *file, unsigned int cmd,
- unsigned long arg)
-{
- if (file->f_op->unlocked_ioctl)
- return file->f_op->unlocked_ioctl(file, cmd, arg);
-
- return -ENOTTY;
-}
-
-/* Handle 32-bit COMEDI_CHANINFO ioctl. */
-static int compat_chaninfo(struct file *file, unsigned long arg)
-{
- struct comedi_chaninfo __user *chaninfo;
- struct comedi32_chaninfo_struct __user *chaninfo32;
- int err;
- union {
- unsigned int uint;
- compat_uptr_t uptr;
- } temp;
-
- chaninfo32 = compat_ptr(arg);
- chaninfo = compat_alloc_user_space(sizeof(*chaninfo));
-
- /* Copy chaninfo structure. Ignore unused members. */
- if (!access_ok(chaninfo32, sizeof(*chaninfo32)) ||
- !access_ok(chaninfo, sizeof(*chaninfo)))
- return -EFAULT;
-
- err = 0;
- err |= __get_user(temp.uint, &chaninfo32->subdev);
- err |= __put_user(temp.uint, &chaninfo->subdev);
- err |= __get_user(temp.uptr, &chaninfo32->maxdata_list);
- err |= __put_user(compat_ptr(temp.uptr), &chaninfo->maxdata_list);
- err |= __get_user(temp.uptr, &chaninfo32->flaglist);
- err |= __put_user(compat_ptr(temp.uptr), &chaninfo->flaglist);
- err |= __get_user(temp.uptr, &chaninfo32->rangelist);
- err |= __put_user(compat_ptr(temp.uptr), &chaninfo->rangelist);
- if (err)
- return -EFAULT;
-
- return translated_ioctl(file, COMEDI_CHANINFO, (unsigned long)chaninfo);
-}
-
-/* Handle 32-bit COMEDI_RANGEINFO ioctl. */
-static int compat_rangeinfo(struct file *file, unsigned long arg)
-{
- struct comedi_rangeinfo __user *rangeinfo;
- struct comedi32_rangeinfo_struct __user *rangeinfo32;
- int err;
- union {
- unsigned int uint;
- compat_uptr_t uptr;
- } temp;
-
- rangeinfo32 = compat_ptr(arg);
- rangeinfo = compat_alloc_user_space(sizeof(*rangeinfo));
-
- /* Copy rangeinfo structure. */
- if (!access_ok(rangeinfo32, sizeof(*rangeinfo32)) ||
- !access_ok(rangeinfo, sizeof(*rangeinfo)))
- return -EFAULT;
-
- err = 0;
- err |= __get_user(temp.uint, &rangeinfo32->range_type);
- err |= __put_user(temp.uint, &rangeinfo->range_type);
- err |= __get_user(temp.uptr, &rangeinfo32->range_ptr);
- err |= __put_user(compat_ptr(temp.uptr), &rangeinfo->range_ptr);
- if (err)
- return -EFAULT;
-
- return translated_ioctl(file, COMEDI_RANGEINFO,
- (unsigned long)rangeinfo);
-}
-
-/* Copy 32-bit cmd structure to native cmd structure. */
-static int get_compat_cmd(struct comedi_cmd __user *cmd,
- struct comedi32_cmd_struct __user *cmd32)
-{
- int err;
- union {
- unsigned int uint;
- compat_uptr_t uptr;
- } temp;
-
- /* Copy cmd structure. */
- if (!access_ok(cmd32, sizeof(*cmd32)) ||
- !access_ok(cmd, sizeof(*cmd)))
- return -EFAULT;
-
- err = 0;
- err |= __get_user(temp.uint, &cmd32->subdev);
- err |= __put_user(temp.uint, &cmd->subdev);
- err |= __get_user(temp.uint, &cmd32->flags);
- err |= __put_user(temp.uint, &cmd->flags);
- err |= __get_user(temp.uint, &cmd32->start_src);
- err |= __put_user(temp.uint, &cmd->start_src);
- err |= __get_user(temp.uint, &cmd32->start_arg);
- err |= __put_user(temp.uint, &cmd->start_arg);
- err |= __get_user(temp.uint, &cmd32->scan_begin_src);
- err |= __put_user(temp.uint, &cmd->scan_begin_src);
- err |= __get_user(temp.uint, &cmd32->scan_begin_arg);
- err |= __put_user(temp.uint, &cmd->scan_begin_arg);
- err |= __get_user(temp.uint, &cmd32->convert_src);
- err |= __put_user(temp.uint, &cmd->convert_src);
- err |= __get_user(temp.uint, &cmd32->convert_arg);
- err |= __put_user(temp.uint, &cmd->convert_arg);
- err |= __get_user(temp.uint, &cmd32->scan_end_src);
- err |= __put_user(temp.uint, &cmd->scan_end_src);
- err |= __get_user(temp.uint, &cmd32->scan_end_arg);
- err |= __put_user(temp.uint, &cmd->scan_end_arg);
- err |= __get_user(temp.uint, &cmd32->stop_src);
- err |= __put_user(temp.uint, &cmd->stop_src);
- err |= __get_user(temp.uint, &cmd32->stop_arg);
- err |= __put_user(temp.uint, &cmd->stop_arg);
- err |= __get_user(temp.uptr, &cmd32->chanlist);
- err |= __put_user((unsigned int __force *)compat_ptr(temp.uptr),
- &cmd->chanlist);
- err |= __get_user(temp.uint, &cmd32->chanlist_len);
- err |= __put_user(temp.uint, &cmd->chanlist_len);
- err |= __get_user(temp.uptr, &cmd32->data);
- err |= __put_user(compat_ptr(temp.uptr), &cmd->data);
- err |= __get_user(temp.uint, &cmd32->data_len);
- err |= __put_user(temp.uint, &cmd->data_len);
- return err ? -EFAULT : 0;
-}
-
-/* Copy native cmd structure to 32-bit cmd structure. */
-static int put_compat_cmd(struct comedi32_cmd_struct __user *cmd32,
- struct comedi_cmd __user *cmd)
-{
- int err;
- unsigned int temp;
-
- /*
- * Copy back most of cmd structure.
- *
- * Assume the pointer values are already valid.
- * (Could use ptr_to_compat() to set them.)
- */
- if (!access_ok(cmd, sizeof(*cmd)) ||
- !access_ok(cmd32, sizeof(*cmd32)))
- return -EFAULT;
-
- err = 0;
- err |= __get_user(temp, &cmd->subdev);
- err |= __put_user(temp, &cmd32->subdev);
- err |= __get_user(temp, &cmd->flags);
- err |= __put_user(temp, &cmd32->flags);
- err |= __get_user(temp, &cmd->start_src);
- err |= __put_user(temp, &cmd32->start_src);
- err |= __get_user(temp, &cmd->start_arg);
- err |= __put_user(temp, &cmd32->start_arg);
- err |= __get_user(temp, &cmd->scan_begin_src);
- err |= __put_user(temp, &cmd32->scan_begin_src);
- err |= __get_user(temp, &cmd->scan_begin_arg);
- err |= __put_user(temp, &cmd32->scan_begin_arg);
- err |= __get_user(temp, &cmd->convert_src);
- err |= __put_user(temp, &cmd32->convert_src);
- err |= __get_user(temp, &cmd->convert_arg);
- err |= __put_user(temp, &cmd32->convert_arg);
- err |= __get_user(temp, &cmd->scan_end_src);
- err |= __put_user(temp, &cmd32->scan_end_src);
- err |= __get_user(temp, &cmd->scan_end_arg);
- err |= __put_user(temp, &cmd32->scan_end_arg);
- err |= __get_user(temp, &cmd->stop_src);
- err |= __put_user(temp, &cmd32->stop_src);
- err |= __get_user(temp, &cmd->stop_arg);
- err |= __put_user(temp, &cmd32->stop_arg);
- /* Assume chanlist pointer is unchanged. */
- err |= __get_user(temp, &cmd->chanlist_len);
- err |= __put_user(temp, &cmd32->chanlist_len);
- /* Assume data pointer is unchanged. */
- err |= __get_user(temp, &cmd->data_len);
- err |= __put_user(temp, &cmd32->data_len);
- return err ? -EFAULT : 0;
-}
-
-/* Handle 32-bit COMEDI_CMD ioctl. */
-static int compat_cmd(struct file *file, unsigned long arg)
-{
- struct comedi_cmd __user *cmd;
- struct comedi32_cmd_struct __user *cmd32;
- int rc, err;
-
- cmd32 = compat_ptr(arg);
- cmd = compat_alloc_user_space(sizeof(*cmd));
-
- rc = get_compat_cmd(cmd, cmd32);
- if (rc)
- return rc;
-
- rc = translated_ioctl(file, COMEDI_CMD, (unsigned long)cmd);
- if (rc == -EAGAIN) {
- /* Special case: copy cmd back to user. */
- err = put_compat_cmd(cmd32, cmd);
- if (err)
- rc = err;
- }
-
- return rc;
-}
-
-/* Handle 32-bit COMEDI_CMDTEST ioctl. */
-static int compat_cmdtest(struct file *file, unsigned long arg)
-{
- struct comedi_cmd __user *cmd;
- struct comedi32_cmd_struct __user *cmd32;
- int rc, err;
-
- cmd32 = compat_ptr(arg);
- cmd = compat_alloc_user_space(sizeof(*cmd));
-
- rc = get_compat_cmd(cmd, cmd32);
- if (rc)
- return rc;
-
- rc = translated_ioctl(file, COMEDI_CMDTEST, (unsigned long)cmd);
- if (rc < 0)
- return rc;
-
- err = put_compat_cmd(cmd32, cmd);
- if (err)
- rc = err;
-
- return rc;
-}
-
-/* Copy 32-bit insn structure to native insn structure. */
-static int get_compat_insn(struct comedi_insn __user *insn,
- struct comedi32_insn_struct __user *insn32)
-{
- int err;
- union {
- unsigned int uint;
- compat_uptr_t uptr;
- } temp;
-
- /* Copy insn structure. Ignore the unused members. */
- err = 0;
- if (!access_ok(insn32, sizeof(*insn32)) ||
- !access_ok(insn, sizeof(*insn)))
- return -EFAULT;
-
- err |= __get_user(temp.uint, &insn32->insn);
- err |= __put_user(temp.uint, &insn->insn);
- err |= __get_user(temp.uint, &insn32->n);
- err |= __put_user(temp.uint, &insn->n);
- err |= __get_user(temp.uptr, &insn32->data);
- err |= __put_user(compat_ptr(temp.uptr), &insn->data);
- err |= __get_user(temp.uint, &insn32->subdev);
- err |= __put_user(temp.uint, &insn->subdev);
- err |= __get_user(temp.uint, &insn32->chanspec);
- err |= __put_user(temp.uint, &insn->chanspec);
- return err ? -EFAULT : 0;
-}
-
-/* Handle 32-bit COMEDI_INSNLIST ioctl. */
-static int compat_insnlist(struct file *file, unsigned long arg)
-{
- struct combined_insnlist {
- struct comedi_insnlist insnlist;
- struct comedi_insn insn[1];
- } __user *s;
- struct comedi32_insnlist_struct __user *insnlist32;
- struct comedi32_insn_struct __user *insn32;
- compat_uptr_t uptr;
- unsigned int n_insns, n;
- int err, rc;
-
- insnlist32 = compat_ptr(arg);
-
- /* Get 32-bit insnlist structure. */
- if (!access_ok(insnlist32, sizeof(*insnlist32)))
- return -EFAULT;
-
- err = 0;
- err |= __get_user(n_insns, &insnlist32->n_insns);
- err |= __get_user(uptr, &insnlist32->insns);
- insn32 = compat_ptr(uptr);
- if (err)
- return -EFAULT;
-
- /* Allocate user memory to copy insnlist and insns into. */
- s = compat_alloc_user_space(offsetof(struct combined_insnlist,
- insn[n_insns]));
-
- /* Set native insnlist structure. */
- if (!access_ok(&s->insnlist, sizeof(s->insnlist)))
- return -EFAULT;
-
- err |= __put_user(n_insns, &s->insnlist.n_insns);
- err |= __put_user(&s->insn[0], &s->insnlist.insns);
- if (err)
- return -EFAULT;
-
- /* Copy insn structures. */
- for (n = 0; n < n_insns; n++) {
- rc = get_compat_insn(&s->insn[n], &insn32[n]);
- if (rc)
- return rc;
- }
-
- return translated_ioctl(file, COMEDI_INSNLIST,
- (unsigned long)&s->insnlist);
-}
-
-/* Handle 32-bit COMEDI_INSN ioctl. */
-static int compat_insn(struct file *file, unsigned long arg)
-{
- struct comedi_insn __user *insn;
- struct comedi32_insn_struct __user *insn32;
- int rc;
-
- insn32 = compat_ptr(arg);
- insn = compat_alloc_user_space(sizeof(*insn));
-
- rc = get_compat_insn(insn, insn32);
- if (rc)
- return rc;
-
- return translated_ioctl(file, COMEDI_INSN, (unsigned long)insn);
-}
-
-/*
- * compat_ioctl file operation.
- *
- * Returns -ENOIOCTLCMD for unrecognised ioctl codes.
- */
-long comedi_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
-{
- int rc;
-
- switch (cmd) {
- case COMEDI_DEVCONFIG:
- case COMEDI_DEVINFO:
- case COMEDI_SUBDINFO:
- case COMEDI_BUFCONFIG:
- case COMEDI_BUFINFO:
- /* Just need to translate the pointer argument. */
- arg = (unsigned long)compat_ptr(arg);
- rc = translated_ioctl(file, cmd, arg);
- break;
- case COMEDI_LOCK:
- case COMEDI_UNLOCK:
- case COMEDI_CANCEL:
- case COMEDI_POLL:
- case COMEDI_SETRSUBD:
- case COMEDI_SETWSUBD:
- /* No translation needed. */
- rc = translated_ioctl(file, cmd, arg);
- break;
- case COMEDI32_CHANINFO:
- rc = compat_chaninfo(file, arg);
- break;
- case COMEDI32_RANGEINFO:
- rc = compat_rangeinfo(file, arg);
- break;
- case COMEDI32_CMD:
- rc = compat_cmd(file, arg);
- break;
- case COMEDI32_CMDTEST:
- rc = compat_cmdtest(file, arg);
- break;
- case COMEDI32_INSNLIST:
- rc = compat_insnlist(file, arg);
- break;
- case COMEDI32_INSN:
- rc = compat_insn(file, arg);
- break;
- default:
- rc = -ENOIOCTLCMD;
- break;
- }
- return rc;
-}
diff --git a/drivers/staging/comedi/comedi_compat32.h b/drivers/staging/comedi/comedi_compat32.h
deleted file mode 100644
index dc3e2a9442c7..000000000000
--- a/drivers/staging/comedi/comedi_compat32.h
+++ /dev/null
@@ -1,28 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0+ */
-/*
- * comedi/comedi_compat32.h
- * 32-bit ioctl compatibility for 64-bit comedi kernel module.
- *
- * Author: Ian Abbott, MEV Ltd. <[email protected]>
- * Copyright (C) 2007 MEV Ltd. <http://www.mev.co.uk/>
- *
- * COMEDI - Linux Control and Measurement Device Interface
- * Copyright (C) 1997-2007 David A. Schleef <[email protected]>
- */
-
-#ifndef _COMEDI_COMPAT32_H
-#define _COMEDI_COMPAT32_H
-
-#ifdef CONFIG_COMPAT
-
-struct file;
-long comedi_compat_ioctl(struct file *file, unsigned int cmd,
- unsigned long arg);
-
-#else /* CONFIG_COMPAT */
-
-#define comedi_compat_ioctl NULL
-
-#endif /* CONFIG_COMPAT */
-
-#endif /* _COMEDI_COMPAT32_H */
diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c
index 08d1bbbebf2d..9dfb81dfe43c 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -4,13 +4,14 @@
* comedi kernel module
*
* COMEDI - Linux Control and Measurement Device Interface
- * Copyright (C) 1997-2000 David A. Schleef <[email protected]>
+ * Copyright (C) 1997-2007 David A. Schleef <[email protected]>
+ * compat ioctls:
+ * Author: Ian Abbott, MEV Ltd. <[email protected]>
+ * Copyright (C) 2007 MEV Ltd. <http://www.mev.co.uk/>
*/
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
-#include "comedi_compat32.h"
-
#include <linux/module.h>
#include <linux/errno.h>
#include <linux/kernel.h>
@@ -27,6 +28,7 @@
#include <linux/io.h>
#include <linux/uaccess.h>
+#include <linux/compat.h>
#include "comedi_internal.h"
@@ -2806,6 +2808,449 @@ static int comedi_close(struct inode *inode, struct file *file)
return 0;
}
+#ifdef CONFIG_COMPAT
+
+#define COMEDI32_CHANINFO _IOR(CIO, 3, struct comedi32_chaninfo_struct)
+#define COMEDI32_RANGEINFO _IOR(CIO, 8, struct comedi32_rangeinfo_struct)
+/*
+ * N.B. COMEDI32_CMD and COMEDI_CMD ought to use _IOWR, not _IOR.
+ * It's too late to change it now, but it only affects the command number.
+ */
+#define COMEDI32_CMD _IOR(CIO, 9, struct comedi32_cmd_struct)
+/*
+ * N.B. COMEDI32_CMDTEST and COMEDI_CMDTEST ought to use _IOWR, not _IOR.
+ * It's too late to change it now, but it only affects the command number.
+ */
+#define COMEDI32_CMDTEST _IOR(CIO, 10, struct comedi32_cmd_struct)
+#define COMEDI32_INSNLIST _IOR(CIO, 11, struct comedi32_insnlist_struct)
+#define COMEDI32_INSN _IOR(CIO, 12, struct comedi32_insn_struct)
+
+struct comedi32_chaninfo_struct {
+ unsigned int subdev;
+ compat_uptr_t maxdata_list; /* 32-bit 'unsigned int *' */
+ compat_uptr_t flaglist; /* 32-bit 'unsigned int *' */
+ compat_uptr_t rangelist; /* 32-bit 'unsigned int *' */
+ unsigned int unused[4];
+};
+
+struct comedi32_rangeinfo_struct {
+ unsigned int range_type;
+ compat_uptr_t range_ptr; /* 32-bit 'void *' */
+};
+
+struct comedi32_cmd_struct {
+ unsigned int subdev;
+ unsigned int flags;
+ unsigned int start_src;
+ unsigned int start_arg;
+ unsigned int scan_begin_src;
+ unsigned int scan_begin_arg;
+ unsigned int convert_src;
+ unsigned int convert_arg;
+ unsigned int scan_end_src;
+ unsigned int scan_end_arg;
+ unsigned int stop_src;
+ unsigned int stop_arg;
+ compat_uptr_t chanlist; /* 32-bit 'unsigned int *' */
+ unsigned int chanlist_len;
+ compat_uptr_t data; /* 32-bit 'short *' */
+ unsigned int data_len;
+};
+
+struct comedi32_insn_struct {
+ unsigned int insn;
+ unsigned int n;
+ compat_uptr_t data; /* 32-bit 'unsigned int *' */
+ unsigned int subdev;
+ unsigned int chanspec;
+ unsigned int unused[3];
+};
+
+struct comedi32_insnlist_struct {
+ unsigned int n_insns;
+ compat_uptr_t insns; /* 32-bit 'struct comedi_insn *' */
+};
+
+/* Handle translated ioctl. */
+static int translated_ioctl(struct file *file, unsigned int cmd,
+ unsigned long arg)
+{
+ if (file->f_op->unlocked_ioctl)
+ return file->f_op->unlocked_ioctl(file, cmd, arg);
+
+ return -ENOTTY;
+}
+
+/* Handle 32-bit COMEDI_CHANINFO ioctl. */
+static int compat_chaninfo(struct file *file, unsigned long arg)
+{
+ struct comedi_chaninfo __user *chaninfo;
+ struct comedi32_chaninfo_struct __user *chaninfo32;
+ int err;
+ union {
+ unsigned int uint;
+ compat_uptr_t uptr;
+ } temp;
+
+ chaninfo32 = compat_ptr(arg);
+ chaninfo = compat_alloc_user_space(sizeof(*chaninfo));
+
+ /* Copy chaninfo structure. Ignore unused members. */
+ if (!access_ok(chaninfo32, sizeof(*chaninfo32)) ||
+ !access_ok(chaninfo, sizeof(*chaninfo)))
+ return -EFAULT;
+
+ err = 0;
+ err |= __get_user(temp.uint, &chaninfo32->subdev);
+ err |= __put_user(temp.uint, &chaninfo->subdev);
+ err |= __get_user(temp.uptr, &chaninfo32->maxdata_list);
+ err |= __put_user(compat_ptr(temp.uptr), &chaninfo->maxdata_list);
+ err |= __get_user(temp.uptr, &chaninfo32->flaglist);
+ err |= __put_user(compat_ptr(temp.uptr), &chaninfo->flaglist);
+ err |= __get_user(temp.uptr, &chaninfo32->rangelist);
+ err |= __put_user(compat_ptr(temp.uptr), &chaninfo->rangelist);
+ if (err)
+ return -EFAULT;
+
+ return translated_ioctl(file, COMEDI_CHANINFO, (unsigned long)chaninfo);
+}
+
+/* Handle 32-bit COMEDI_RANGEINFO ioctl. */
+static int compat_rangeinfo(struct file *file, unsigned long arg)
+{
+ struct comedi_rangeinfo __user *rangeinfo;
+ struct comedi32_rangeinfo_struct __user *rangeinfo32;
+ int err;
+ union {
+ unsigned int uint;
+ compat_uptr_t uptr;
+ } temp;
+
+ rangeinfo32 = compat_ptr(arg);
+ rangeinfo = compat_alloc_user_space(sizeof(*rangeinfo));
+
+ /* Copy rangeinfo structure. */
+ if (!access_ok(rangeinfo32, sizeof(*rangeinfo32)) ||
+ !access_ok(rangeinfo, sizeof(*rangeinfo)))
+ return -EFAULT;
+
+ err = 0;
+ err |= __get_user(temp.uint, &rangeinfo32->range_type);
+ err |= __put_user(temp.uint, &rangeinfo->range_type);
+ err |= __get_user(temp.uptr, &rangeinfo32->range_ptr);
+ err |= __put_user(compat_ptr(temp.uptr), &rangeinfo->range_ptr);
+ if (err)
+ return -EFAULT;
+
+ return translated_ioctl(file, COMEDI_RANGEINFO,
+ (unsigned long)rangeinfo);
+}
+
+/* Copy 32-bit cmd structure to native cmd structure. */
+static int get_compat_cmd(struct comedi_cmd __user *cmd,
+ struct comedi32_cmd_struct __user *cmd32)
+{
+ int err;
+ union {
+ unsigned int uint;
+ compat_uptr_t uptr;
+ } temp;
+
+ /* Copy cmd structure. */
+ if (!access_ok(cmd32, sizeof(*cmd32)) ||
+ !access_ok(cmd, sizeof(*cmd)))
+ return -EFAULT;
+
+ err = 0;
+ err |= __get_user(temp.uint, &cmd32->subdev);
+ err |= __put_user(temp.uint, &cmd->subdev);
+ err |= __get_user(temp.uint, &cmd32->flags);
+ err |= __put_user(temp.uint, &cmd->flags);
+ err |= __get_user(temp.uint, &cmd32->start_src);
+ err |= __put_user(temp.uint, &cmd->start_src);
+ err |= __get_user(temp.uint, &cmd32->start_arg);
+ err |= __put_user(temp.uint, &cmd->start_arg);
+ err |= __get_user(temp.uint, &cmd32->scan_begin_src);
+ err |= __put_user(temp.uint, &cmd->scan_begin_src);
+ err |= __get_user(temp.uint, &cmd32->scan_begin_arg);
+ err |= __put_user(temp.uint, &cmd->scan_begin_arg);
+ err |= __get_user(temp.uint, &cmd32->convert_src);
+ err |= __put_user(temp.uint, &cmd->convert_src);
+ err |= __get_user(temp.uint, &cmd32->convert_arg);
+ err |= __put_user(temp.uint, &cmd->convert_arg);
+ err |= __get_user(temp.uint, &cmd32->scan_end_src);
+ err |= __put_user(temp.uint, &cmd->scan_end_src);
+ err |= __get_user(temp.uint, &cmd32->scan_end_arg);
+ err |= __put_user(temp.uint, &cmd->scan_end_arg);
+ err |= __get_user(temp.uint, &cmd32->stop_src);
+ err |= __put_user(temp.uint, &cmd->stop_src);
+ err |= __get_user(temp.uint, &cmd32->stop_arg);
+ err |= __put_user(temp.uint, &cmd->stop_arg);
+ err |= __get_user(temp.uptr, &cmd32->chanlist);
+ err |= __put_user((unsigned int __force *)compat_ptr(temp.uptr),
+ &cmd->chanlist);
+ err |= __get_user(temp.uint, &cmd32->chanlist_len);
+ err |= __put_user(temp.uint, &cmd->chanlist_len);
+ err |= __get_user(temp.uptr, &cmd32->data);
+ err |= __put_user(compat_ptr(temp.uptr), &cmd->data);
+ err |= __get_user(temp.uint, &cmd32->data_len);
+ err |= __put_user(temp.uint, &cmd->data_len);
+ return err ? -EFAULT : 0;
+}
+
+/* Copy native cmd structure to 32-bit cmd structure. */
+static int put_compat_cmd(struct comedi32_cmd_struct __user *cmd32,
+ struct comedi_cmd __user *cmd)
+{
+ int err;
+ unsigned int temp;
+
+ /*
+ * Copy back most of cmd structure.
+ *
+ * Assume the pointer values are already valid.
+ * (Could use ptr_to_compat() to set them.)
+ */
+ if (!access_ok(cmd, sizeof(*cmd)) ||
+ !access_ok(cmd32, sizeof(*cmd32)))
+ return -EFAULT;
+
+ err = 0;
+ err |= __get_user(temp, &cmd->subdev);
+ err |= __put_user(temp, &cmd32->subdev);
+ err |= __get_user(temp, &cmd->flags);
+ err |= __put_user(temp, &cmd32->flags);
+ err |= __get_user(temp, &cmd->start_src);
+ err |= __put_user(temp, &cmd32->start_src);
+ err |= __get_user(temp, &cmd->start_arg);
+ err |= __put_user(temp, &cmd32->start_arg);
+ err |= __get_user(temp, &cmd->scan_begin_src);
+ err |= __put_user(temp, &cmd32->scan_begin_src);
+ err |= __get_user(temp, &cmd->scan_begin_arg);
+ err |= __put_user(temp, &cmd32->scan_begin_arg);
+ err |= __get_user(temp, &cmd->convert_src);
+ err |= __put_user(temp, &cmd32->convert_src);
+ err |= __get_user(temp, &cmd->convert_arg);
+ err |= __put_user(temp, &cmd32->convert_arg);
+ err |= __get_user(temp, &cmd->scan_end_src);
+ err |= __put_user(temp, &cmd32->scan_end_src);
+ err |= __get_user(temp, &cmd->scan_end_arg);
+ err |= __put_user(temp, &cmd32->scan_end_arg);
+ err |= __get_user(temp, &cmd->stop_src);
+ err |= __put_user(temp, &cmd32->stop_src);
+ err |= __get_user(temp, &cmd->stop_arg);
+ err |= __put_user(temp, &cmd32->stop_arg);
+ /* Assume chanlist pointer is unchanged. */
+ err |= __get_user(temp, &cmd->chanlist_len);
+ err |= __put_user(temp, &cmd32->chanlist_len);
+ /* Assume data pointer is unchanged. */
+ err |= __get_user(temp, &cmd->data_len);
+ err |= __put_user(temp, &cmd32->data_len);
+ return err ? -EFAULT : 0;
+}
+
+/* Handle 32-bit COMEDI_CMD ioctl. */
+static int compat_cmd(struct file *file, unsigned long arg)
+{
+ struct comedi_cmd __user *cmd;
+ struct comedi32_cmd_struct __user *cmd32;
+ int rc, err;
+
+ cmd32 = compat_ptr(arg);
+ cmd = compat_alloc_user_space(sizeof(*cmd));
+
+ rc = get_compat_cmd(cmd, cmd32);
+ if (rc)
+ return rc;
+
+ rc = translated_ioctl(file, COMEDI_CMD, (unsigned long)cmd);
+ if (rc == -EAGAIN) {
+ /* Special case: copy cmd back to user. */
+ err = put_compat_cmd(cmd32, cmd);
+ if (err)
+ rc = err;
+ }
+
+ return rc;
+}
+
+/* Handle 32-bit COMEDI_CMDTEST ioctl. */
+static int compat_cmdtest(struct file *file, unsigned long arg)
+{
+ struct comedi_cmd __user *cmd;
+ struct comedi32_cmd_struct __user *cmd32;
+ int rc, err;
+
+ cmd32 = compat_ptr(arg);
+ cmd = compat_alloc_user_space(sizeof(*cmd));
+
+ rc = get_compat_cmd(cmd, cmd32);
+ if (rc)
+ return rc;
+
+ rc = translated_ioctl(file, COMEDI_CMDTEST, (unsigned long)cmd);
+ if (rc < 0)
+ return rc;
+
+ err = put_compat_cmd(cmd32, cmd);
+ if (err)
+ rc = err;
+
+ return rc;
+}
+
+/* Copy 32-bit insn structure to native insn structure. */
+static int get_compat_insn(struct comedi_insn __user *insn,
+ struct comedi32_insn_struct __user *insn32)
+{
+ int err;
+ union {
+ unsigned int uint;
+ compat_uptr_t uptr;
+ } temp;
+
+ /* Copy insn structure. Ignore the unused members. */
+ err = 0;
+ if (!access_ok(insn32, sizeof(*insn32)) ||
+ !access_ok(insn, sizeof(*insn)))
+ return -EFAULT;
+
+ err |= __get_user(temp.uint, &insn32->insn);
+ err |= __put_user(temp.uint, &insn->insn);
+ err |= __get_user(temp.uint, &insn32->n);
+ err |= __put_user(temp.uint, &insn->n);
+ err |= __get_user(temp.uptr, &insn32->data);
+ err |= __put_user(compat_ptr(temp.uptr), &insn->data);
+ err |= __get_user(temp.uint, &insn32->subdev);
+ err |= __put_user(temp.uint, &insn->subdev);
+ err |= __get_user(temp.uint, &insn32->chanspec);
+ err |= __put_user(temp.uint, &insn->chanspec);
+ return err ? -EFAULT : 0;
+}
+
+/* Handle 32-bit COMEDI_INSNLIST ioctl. */
+static int compat_insnlist(struct file *file, unsigned long arg)
+{
+ struct combined_insnlist {
+ struct comedi_insnlist insnlist;
+ struct comedi_insn insn[1];
+ } __user *s;
+ struct comedi32_insnlist_struct __user *insnlist32;
+ struct comedi32_insn_struct __user *insn32;
+ compat_uptr_t uptr;
+ unsigned int n_insns, n;
+ int err, rc;
+
+ insnlist32 = compat_ptr(arg);
+
+ /* Get 32-bit insnlist structure. */
+ if (!access_ok(insnlist32, sizeof(*insnlist32)))
+ return -EFAULT;
+
+ err = 0;
+ err |= __get_user(n_insns, &insnlist32->n_insns);
+ err |= __get_user(uptr, &insnlist32->insns);
+ insn32 = compat_ptr(uptr);
+ if (err)
+ return -EFAULT;
+
+ /* Allocate user memory to copy insnlist and insns into. */
+ s = compat_alloc_user_space(offsetof(struct combined_insnlist,
+ insn[n_insns]));
+
+ /* Set native insnlist structure. */
+ if (!access_ok(&s->insnlist, sizeof(s->insnlist)))
+ return -EFAULT;
+
+ err |= __put_user(n_insns, &s->insnlist.n_insns);
+ err |= __put_user(&s->insn[0], &s->insnlist.insns);
+ if (err)
+ return -EFAULT;
+
+ /* Copy insn structures. */
+ for (n = 0; n < n_insns; n++) {
+ rc = get_compat_insn(&s->insn[n], &insn32[n]);
+ if (rc)
+ return rc;
+ }
+
+ return translated_ioctl(file, COMEDI_INSNLIST,
+ (unsigned long)&s->insnlist);
+}
+
+/* Handle 32-bit COMEDI_INSN ioctl. */
+static int compat_insn(struct file *file, unsigned long arg)
+{
+ struct comedi_insn __user *insn;
+ struct comedi32_insn_struct __user *insn32;
+ int rc;
+
+ insn32 = compat_ptr(arg);
+ insn = compat_alloc_user_space(sizeof(*insn));
+
+ rc = get_compat_insn(insn, insn32);
+ if (rc)
+ return rc;
+
+ return translated_ioctl(file, COMEDI_INSN, (unsigned long)insn);
+}
+
+/*
+ * compat_ioctl file operation.
+ *
+ * Returns -ENOIOCTLCMD for unrecognised ioctl codes.
+ */
+static long comedi_compat_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
+{
+ int rc;
+
+ switch (cmd) {
+ case COMEDI_DEVCONFIG:
+ case COMEDI_DEVINFO:
+ case COMEDI_SUBDINFO:
+ case COMEDI_BUFCONFIG:
+ case COMEDI_BUFINFO:
+ /* Just need to translate the pointer argument. */
+ arg = (unsigned long)compat_ptr(arg);
+ rc = translated_ioctl(file, cmd, arg);
+ break;
+ case COMEDI_LOCK:
+ case COMEDI_UNLOCK:
+ case COMEDI_CANCEL:
+ case COMEDI_POLL:
+ case COMEDI_SETRSUBD:
+ case COMEDI_SETWSUBD:
+ /* No translation needed. */
+ rc = translated_ioctl(file, cmd, arg);
+ break;
+ case COMEDI32_CHANINFO:
+ rc = compat_chaninfo(file, arg);
+ break;
+ case COMEDI32_RANGEINFO:
+ rc = compat_rangeinfo(file, arg);
+ break;
+ case COMEDI32_CMD:
+ rc = compat_cmd(file, arg);
+ break;
+ case COMEDI32_CMDTEST:
+ rc = compat_cmdtest(file, arg);
+ break;
+ case COMEDI32_INSNLIST:
+ rc = compat_insnlist(file, arg);
+ break;
+ case COMEDI32_INSN:
+ rc = compat_insn(file, arg);
+ break;
+ default:
+ rc = -ENOIOCTLCMD;
+ break;
+ }
+ return rc;
+}
+#else
+#define comedi_compat_ioctl NULL
+#endif
+
static const struct file_operations comedi_fops = {
.owner = THIS_MODULE,
.unlocked_ioctl = comedi_unlocked_ioctl,
--
2.11.0
From: Al Viro <[email protected]>
Signed-off-by: Al Viro <[email protected]>
---
drivers/staging/comedi/comedi_fops.c | 20 ++++++++++++--------
1 file changed, 12 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c
index d80a416e70b2..e85a143057a1 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -1649,17 +1649,11 @@ static int do_insn_ioctl(struct comedi_device *dev,
}
static int __comedi_get_user_cmd(struct comedi_device *dev,
- struct comedi_cmd __user *arg,
struct comedi_cmd *cmd)
{
struct comedi_subdevice *s;
lockdep_assert_held(&dev->mutex);
- if (copy_from_user(cmd, arg, sizeof(*cmd))) {
- dev_dbg(dev->class_dev, "bad cmd address\n");
- return -EFAULT;
- }
-
if (cmd->subdev >= dev->n_subdevices) {
dev_dbg(dev->class_dev, "%d no such subdevice\n", cmd->subdev);
return -ENODEV;
@@ -1757,8 +1751,13 @@ static int do_cmd_ioctl(struct comedi_device *dev,
lockdep_assert_held(&dev->mutex);
+ if (copy_from_user(&cmd, arg, sizeof(cmd))) {
+ dev_dbg(dev->class_dev, "bad cmd address\n");
+ return -EFAULT;
+ }
+
/* get the user's cmd and do some simple validation */
- ret = __comedi_get_user_cmd(dev, arg, &cmd);
+ ret = __comedi_get_user_cmd(dev, &cmd);
if (ret)
return ret;
@@ -1866,8 +1865,13 @@ static int do_cmdtest_ioctl(struct comedi_device *dev,
lockdep_assert_held(&dev->mutex);
+ if (copy_from_user(&cmd, arg, sizeof(cmd))) {
+ dev_dbg(dev->class_dev, "bad cmd address\n");
+ return -EFAULT;
+ }
+
/* get the user's cmd and do some simple validation */
- ret = __comedi_get_user_cmd(dev, arg, &cmd);
+ ret = __comedi_get_user_cmd(dev, &cmd);
if (ret)
return ret;
--
2.11.0
From: Al Viro <[email protected]>
Signed-off-by: Al Viro <[email protected]>
---
drivers/staging/comedi/comedi_fops.c | 48 ++++++++++++++++++------------------
1 file changed, 24 insertions(+), 24 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c
index a40a865ed45c..f5ecfbfcdaf5 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -1741,9 +1741,8 @@ static int __comedi_get_user_chanlist(struct comedi_device *dev,
* possibly modified comedi_cmd structure (when -EAGAIN returned)
*/
static int do_cmd_ioctl(struct comedi_device *dev,
- struct comedi_cmd __user *arg, void *file)
+ struct comedi_cmd *cmd, bool *copy, void *file)
{
- struct comedi_cmd cmd;
struct comedi_subdevice *s;
struct comedi_async *async;
unsigned int __user *user_chanlist;
@@ -1751,20 +1750,15 @@ static int do_cmd_ioctl(struct comedi_device *dev,
lockdep_assert_held(&dev->mutex);
- if (copy_from_user(&cmd, arg, sizeof(cmd))) {
- dev_dbg(dev->class_dev, "bad cmd address\n");
- return -EFAULT;
- }
-
- /* get the user's cmd and do some simple validation */
- ret = __comedi_get_user_cmd(dev, &cmd);
+ /* do some simple cmd validation */
+ ret = __comedi_get_user_cmd(dev, cmd);
if (ret)
return ret;
/* save user's chanlist pointer so it can be restored later */
- user_chanlist = (unsigned int __user *)cmd.chanlist;
+ user_chanlist = (unsigned int __user *)cmd->chanlist;
- s = &dev->subdevices[cmd.subdev];
+ s = &dev->subdevices[cmd->subdev];
async = s->async;
/* are we locked? (ioctl lock) */
@@ -1780,13 +1774,13 @@ static int do_cmd_ioctl(struct comedi_device *dev,
}
/* make sure channel/gain list isn't too short */
- if (cmd.chanlist_len < 1) {
+ if (cmd->chanlist_len < 1) {
dev_dbg(dev->class_dev, "channel/gain list too short %u < 1\n",
- cmd.chanlist_len);
+ cmd->chanlist_len);
return -EINVAL;
}
- async->cmd = cmd;
+ async->cmd = *cmd;
async->cmd.data = NULL;
/* load channel/gain list */
@@ -1798,15 +1792,11 @@ static int do_cmd_ioctl(struct comedi_device *dev,
if (async->cmd.flags & CMDF_BOGUS || ret) {
dev_dbg(dev->class_dev, "test returned %d\n", ret);
- cmd = async->cmd;
+ *cmd = async->cmd;
/* restore chanlist pointer before copying back */
- cmd.chanlist = (unsigned int __force *)user_chanlist;
- cmd.data = NULL;
- if (copy_to_user(arg, &cmd, sizeof(cmd))) {
- dev_dbg(dev->class_dev, "fault writing cmd\n");
- ret = -EFAULT;
- goto cleanup;
- }
+ cmd->chanlist = (unsigned int __force *)user_chanlist;
+ cmd->data = NULL;
+ *copy = true;
ret = -EAGAIN;
goto cleanup;
}
@@ -2207,9 +2197,19 @@ static long comedi_unlocked_ioctl(struct file *file, unsigned int cmd,
case COMEDI_CANCEL:
rc = do_cancel_ioctl(dev, arg, file);
break;
- case COMEDI_CMD:
- rc = do_cmd_ioctl(dev, (struct comedi_cmd __user *)arg, file);
+ case COMEDI_CMD: {
+ struct comedi_cmd cmd;
+ bool copy = false;
+
+ if (copy_from_user(&cmd, (void __user *)arg, sizeof(cmd))) {
+ rc = -EFAULT;
+ break;
+ }
+ rc = do_cmd_ioctl(dev, &cmd, ©, file);
+ if (copy && copy_to_user((void __user *)arg, &cmd, sizeof(cmd)))
+ rc = -EFAULT;
break;
+ }
case COMEDI_CMDTEST: {
struct comedi_cmd cmd;
bool copy = false;
--
2.11.0
Low-hanging fruit in i915 uaccess-related stuff.
There's some subtler stuff remaining after that; these
are the simple ones.
Branch in uaccess.i915, based at uaccess.base.
Al Viro (5):
i915: switch query_{topology,engine}_info() to copy_to_user()
i915: switch copy_perf_config_registers_or_number() to unsafe_put_user()
i915 compat ioctl(): just use drm_ioctl_kernel()
i915: alloc_oa_regs(): get rid of pointless access_ok()
i915:get_engines(): get rid of pointless access_ok()
drivers/gpu/drm/i915/gem/i915_gem_context.c | 5 ---
drivers/gpu/drm/i915/i915_ioc32.c | 14 +++----
drivers/gpu/drm/i915/i915_perf.c | 3 --
drivers/gpu/drm/i915/i915_query.c | 62 ++++++++++-------------------
drivers/gpu/drm/i915/i915_reg.h | 2 +-
5 files changed, 28 insertions(+), 58 deletions(-)
From: Al Viro <[email protected]>
Signed-off-by: Al Viro <[email protected]>
---
drivers/gpu/drm/i915/i915_query.c | 16 ++++++----------
1 file changed, 6 insertions(+), 10 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c
index ef25ce6e395e..ad8e55fe1e59 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -25,10 +25,6 @@ static int copy_query_item(void *query_hdr, size_t query_sz,
query_sz))
return -EFAULT;
- if (!access_ok(u64_to_user_ptr(query_item->data_ptr),
- total_length))
- return -EFAULT;
-
return 0;
}
@@ -72,20 +68,20 @@ static int query_topology_info(struct drm_i915_private *dev_priv,
topo.eu_offset = slice_length + subslice_length;
topo.eu_stride = sseu->eu_stride;
- if (__copy_to_user(u64_to_user_ptr(query_item->data_ptr),
+ if (copy_to_user(u64_to_user_ptr(query_item->data_ptr),
&topo, sizeof(topo)))
return -EFAULT;
- if (__copy_to_user(u64_to_user_ptr(query_item->data_ptr + sizeof(topo)),
+ if (copy_to_user(u64_to_user_ptr(query_item->data_ptr + sizeof(topo)),
&sseu->slice_mask, slice_length))
return -EFAULT;
- if (__copy_to_user(u64_to_user_ptr(query_item->data_ptr +
+ if (copy_to_user(u64_to_user_ptr(query_item->data_ptr +
sizeof(topo) + slice_length),
sseu->subslice_mask, subslice_length))
return -EFAULT;
- if (__copy_to_user(u64_to_user_ptr(query_item->data_ptr +
+ if (copy_to_user(u64_to_user_ptr(query_item->data_ptr +
sizeof(topo) +
slice_length + subslice_length),
sseu->eu_mask, eu_length))
@@ -131,14 +127,14 @@ query_engine_info(struct drm_i915_private *i915,
info.engine.engine_instance = engine->uabi_instance;
info.capabilities = engine->uabi_capabilities;
- if (__copy_to_user(info_ptr, &info, sizeof(info)))
+ if (copy_to_user(info_ptr, &info, sizeof(info)))
return -EFAULT;
query.num_engines++;
info_ptr++;
}
- if (__copy_to_user(query_ptr, &query, sizeof(query)))
+ if (copy_to_user(query_ptr, &query, sizeof(query)))
return -EFAULT;
return len;
--
2.11.0
From: Al Viro <[email protected]>
... and the rest of query_perf_config_data() to normal uaccess primitives
Signed-off-by: Al Viro <[email protected]>
---
drivers/gpu/drm/i915/i915_query.c | 46 ++++++++++++++-------------------------
drivers/gpu/drm/i915/i915_reg.h | 2 +-
2 files changed, 17 insertions(+), 31 deletions(-)
diff --git a/drivers/gpu/drm/i915/i915_query.c b/drivers/gpu/drm/i915/i915_query.c
index ad8e55fe1e59..e75c528ebbe0 100644
--- a/drivers/gpu/drm/i915/i915_query.c
+++ b/drivers/gpu/drm/i915/i915_query.c
@@ -154,10 +154,6 @@ static int can_copy_perf_config_registers_or_number(u32 user_n_regs,
if (user_n_regs < kernel_n_regs)
return -EINVAL;
- if (!access_ok(u64_to_user_ptr(user_regs_ptr),
- 2 * sizeof(u32) * kernel_n_regs))
- return -EFAULT;
-
return 0;
}
@@ -166,6 +162,7 @@ static int copy_perf_config_registers_or_number(const struct i915_oa_reg *kernel
u64 user_regs_ptr,
u32 *user_n_regs)
{
+ u32 __user *p = u64_to_user_ptr(user_regs_ptr);
u32 r;
if (*user_n_regs == 0) {
@@ -175,25 +172,19 @@ static int copy_perf_config_registers_or_number(const struct i915_oa_reg *kernel
*user_n_regs = kernel_n_regs;
- for (r = 0; r < kernel_n_regs; r++) {
- u32 __user *user_reg_ptr =
- u64_to_user_ptr(user_regs_ptr + sizeof(u32) * r * 2);
- u32 __user *user_val_ptr =
- u64_to_user_ptr(user_regs_ptr + sizeof(u32) * r * 2 +
- sizeof(u32));
- int ret;
-
- ret = __put_user(i915_mmio_reg_offset(kernel_regs[r].addr),
- user_reg_ptr);
- if (ret)
- return -EFAULT;
+ if (!user_write_access_begin(p, 2 * sizeof(u32) * kernel_n_regs))
+ return -EFAULT;
- ret = __put_user(kernel_regs[r].value, user_val_ptr);
- if (ret)
- return -EFAULT;
+ for (r = 0; r < kernel_n_regs; r++, p += 2) {
+ unsafe_put_user(i915_mmio_reg_offset(kernel_regs[r].addr),
+ p, Efault);
+ unsafe_put_user(kernel_regs[r].value, p + 1, Efault);
}
-
+ user_write_access_end();
return 0;
+Efault:
+ user_write_access_end();
+ return -EFAULT;
}
static int query_perf_config_data(struct drm_i915_private *i915,
@@ -229,10 +220,7 @@ static int query_perf_config_data(struct drm_i915_private *i915,
return -EINVAL;
}
- if (!access_ok(user_query_config_ptr, total_size))
- return -EFAULT;
-
- if (__get_user(flags, &user_query_config_ptr->flags))
+ if (get_user(flags, &user_query_config_ptr->flags))
return -EFAULT;
if (flags != 0)
@@ -245,7 +233,7 @@ static int query_perf_config_data(struct drm_i915_private *i915,
BUILD_BUG_ON(sizeof(user_query_config_ptr->uuid) >= sizeof(uuid));
memset(&uuid, 0, sizeof(uuid));
- if (__copy_from_user(uuid, user_query_config_ptr->uuid,
+ if (copy_from_user(uuid, user_query_config_ptr->uuid,
sizeof(user_query_config_ptr->uuid)))
return -EFAULT;
@@ -259,7 +247,7 @@ static int query_perf_config_data(struct drm_i915_private *i915,
}
rcu_read_unlock();
} else {
- if (__get_user(config_id, &user_query_config_ptr->config))
+ if (get_user(config_id, &user_query_config_ptr->config))
return -EFAULT;
oa_config = i915_perf_get_oa_config(perf, config_id);
@@ -267,8 +255,7 @@ static int query_perf_config_data(struct drm_i915_private *i915,
if (!oa_config)
return -ENOENT;
- if (__copy_from_user(&user_config, user_config_ptr,
- sizeof(user_config))) {
+ if (copy_from_user(&user_config, user_config_ptr, sizeof(user_config))) {
ret = -EFAULT;
goto out;
}
@@ -314,8 +301,7 @@ static int query_perf_config_data(struct drm_i915_private *i915,
memcpy(user_config.uuid, oa_config->uuid, sizeof(user_config.uuid));
- if (__copy_to_user(user_config_ptr, &user_config,
- sizeof(user_config))) {
+ if (copy_to_user(user_config_ptr, &user_config, sizeof(user_config))) {
ret = -EFAULT;
goto out;
}
diff --git a/drivers/gpu/drm/i915/i915_reg.h b/drivers/gpu/drm/i915/i915_reg.h
index 59e64acc2c56..3733b9e20976 100644
--- a/drivers/gpu/drm/i915/i915_reg.h
+++ b/drivers/gpu/drm/i915/i915_reg.h
@@ -186,7 +186,7 @@ typedef struct {
#define INVALID_MMIO_REG _MMIO(0)
-static inline u32 i915_mmio_reg_offset(i915_reg_t reg)
+static __always_inline u32 i915_mmio_reg_offset(i915_reg_t reg)
{
return reg.reg;
}
--
2.11.0
On Fri, 29 May 2020, Al Viro <[email protected]> wrote:
> Low-hanging fruit in i915 uaccess-related stuff.
> There's some subtler stuff remaining after that; these
> are the simple ones.
Please Cc: [email protected] for i915 changes.
Also added Chris who I believe will be able to best review the changes.
BR,
Jani.
--
Jani Nikula, Intel Open Source Graphics Center
On 29/05/2020 01:35, Al Viro wrote:
> From: Al Viro <[email protected]>
>
> Signed-off-by: Al Viro <[email protected]>
> ---
> drivers/staging/comedi/comedi_fops.c | 20 ++++++++++++--------
> 1 file changed, 12 insertions(+), 8 deletions(-)
Signed-off-by: Ian Abbott <[email protected]>
--
-=( Ian Abbott <[email protected]> || Web: http://www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address: )=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
On 29/05/2020 01:35, Al Viro wrote:
> From: Al Viro <[email protected]>
>
> mechanical move
>
> Signed-off-by: Al Viro <[email protected]>
> ---
> drivers/staging/comedi/Makefile | 1 -
> drivers/staging/comedi/comedi_compat32.c | 455 -------------------------------
> drivers/staging/comedi/comedi_compat32.h | 28 --
> drivers/staging/comedi/comedi_fops.c | 451 +++++++++++++++++++++++++++++-
> 4 files changed, 448 insertions(+), 487 deletions(-)
> delete mode 100644 drivers/staging/comedi/comedi_compat32.c
> delete mode 100644 drivers/staging/comedi/comedi_compat32.h
>
Signed-off-by: Ian Abbott <[email protected]>
--
-=( Ian Abbott <[email protected]> || Web: http://www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address: )=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
On 29/05/2020 01:35, Al Viro wrote:
> From: Al Viro <[email protected]>
>
> Signed-off-by: Al Viro <[email protected]>
> ---
> drivers/staging/comedi/comedi_fops.c | 48 ++++++++++++++++++------------------
> 1 file changed, 24 insertions(+), 24 deletions(-)
Signed-off-by: Ian Abbott <[email protected]>
--
-=( Ian Abbott <[email protected]> || Web: http://www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address: )=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
On 29/05/2020 01:34, Al Viro wrote:
> The way comedi compat ioctls are done is wrong.
> Instead of having ->compat_ioctl() copying the 32bit
> stuff in, then passing the kernel copies to helpers shared
> with native ->ioctl() and doing copyout with conversion if
> needed, it's playing silly buggers with creating a 64bit
> copy on user stack, then calling native ioctl (which copies
> that copy into the kernel), then fetching it from user stack,
> converting to 32bit variant and copying that to user.
> Extra headache for no good reason. And the single
> largest remaining pile of __put_user()/__get_user() this side
> of arch/*. IMO compat_alloc_user_space() should die...
>
> NOTE: this is only compile-tested - I simply don't
> have the hardware in question.
>
> Anyway, the branch lives in #uaccess.comedi, based
> at v5.7-rc1
>
> Al Viro (10):
> comedi: move compat ioctl handling to native fops
> comedi: get rid of indirection via translated_ioctl()
> comedi: get rid of compat_alloc_user_space() mess in COMEDI_CHANINFO compat
> comedi: get rid of compat_alloc_user_space() mess in COMEDI_RANGEINFO compat
> comedi: get rid of compat_alloc_user_space() mess in COMEDI_INSN compat
> comedi: get rid of compat_alloc_user_space() mess in COMEDI_INSNLIST compat
> comedi: lift copy_from_user() into callers of __comedi_get_user_cmd()
> comedi: do_cmdtest_ioctl(): lift copyin/copyout into the caller
> comedi: do_cmd_ioctl(): lift copyin/copyout into the caller
> comedi: get rid of compat_alloc_user_space() mess in COMEDI_CMD{,TEST} compat
There is a bug in patch 05. Patch 10 doesn't seem to have been sent yet
(I didn't receive it and I can't see it in the thread in the LKML
archives). I've signed off on 01-04, 06-09.
These should be Cc'd to Greg KH and to [email protected].
Cheers,
Ian
--
-=( Ian Abbott <[email protected]> || Web: http://www.mev.co.uk )=-
-=( MEV Ltd. is a company registered in England & Wales. )=-
-=( Registered number: 02862268. Registered address: )=-
-=( 15 West Park Road, Bramhall, STOCKPORT, SK7 3JZ, UK. )=-
On Fri, May 29, 2020 at 11:48:51AM +0100, Ian Abbott wrote:
> > Al Viro (10):
> > comedi: move compat ioctl handling to native fops
> > comedi: get rid of indirection via translated_ioctl()
> > comedi: get rid of compat_alloc_user_space() mess in COMEDI_CHANINFO compat
> > comedi: get rid of compat_alloc_user_space() mess in COMEDI_RANGEINFO compat
> > comedi: get rid of compat_alloc_user_space() mess in COMEDI_INSN compat
> > comedi: get rid of compat_alloc_user_space() mess in COMEDI_INSNLIST compat
> > comedi: lift copy_from_user() into callers of __comedi_get_user_cmd()
> > comedi: do_cmdtest_ioctl(): lift copyin/copyout into the caller
> > comedi: do_cmd_ioctl(): lift copyin/copyout into the caller
> > comedi: get rid of compat_alloc_user_space() mess in COMEDI_CMD{,TEST} compat
>
> There is a bug in patch 05. Patch 10 doesn't seem to have been sent yet (I
> didn't receive it and I can't see it in the thread in the LKML archives).
> I've signed off on 01-04, 06-09.
#5 fixed, force-pushed to the same branch. As for s-o-b... are you sure that's
the header you have in mind? Normally it's for the chain of transmission...
Do you offer to take that series through comedi (or staging, or...) git tree?
In that case s-o-b would make sense and I'd be happy to have it taken off
my hands. Otherwise it probably should be Acked-by: or Reviewed-by: or
Read-through-and-managed-not-to-throw-up: - up to you...
> These should be Cc'd to Greg KH and to [email protected].
FWIW, 10/10 seems to have been really lost; follows here:
From 88833127a8f00da422ddef03425ad9b19eb65558 Mon Sep 17 00:00:00 2001
From: Al Viro <[email protected]>
Date: Sun, 26 Apr 2020 09:27:23 -0400
Subject: [PATCH 10/10] comedi: get rid of compat_alloc_user_space() mess in
COMEDI_CMD{,TEST} compat
Signed-off-by: Al Viro <[email protected]>
---
drivers/staging/comedi/comedi_fops.c | 181 +++++++++++++----------------------
1 file changed, 66 insertions(+), 115 deletions(-)
diff --git a/drivers/staging/comedi/comedi_fops.c b/drivers/staging/comedi/comedi_fops.c
index f5ecfbfcdaf5..bcdb059e6bb6 100644
--- a/drivers/staging/comedi/comedi_fops.c
+++ b/drivers/staging/comedi/comedi_fops.c
@@ -2930,155 +2930,106 @@ static int compat_rangeinfo(struct file *file, unsigned long arg)
}
/* Copy 32-bit cmd structure to native cmd structure. */
-static int get_compat_cmd(struct comedi_cmd __user *cmd,
+static int get_compat_cmd(struct comedi_cmd *cmd,
struct comedi32_cmd_struct __user *cmd32)
{
- int err;
- union {
- unsigned int uint;
- compat_uptr_t uptr;
- } temp;
-
- /* Copy cmd structure. */
- if (!access_ok(cmd32, sizeof(*cmd32)) ||
- !access_ok(cmd, sizeof(*cmd)))
+ struct comedi32_cmd_struct v32;
+
+ if (copy_from_user(&v32, cmd32, sizeof(v32)))
return -EFAULT;
- err = 0;
- err |= __get_user(temp.uint, &cmd32->subdev);
- err |= __put_user(temp.uint, &cmd->subdev);
- err |= __get_user(temp.uint, &cmd32->flags);
- err |= __put_user(temp.uint, &cmd->flags);
- err |= __get_user(temp.uint, &cmd32->start_src);
- err |= __put_user(temp.uint, &cmd->start_src);
- err |= __get_user(temp.uint, &cmd32->start_arg);
- err |= __put_user(temp.uint, &cmd->start_arg);
- err |= __get_user(temp.uint, &cmd32->scan_begin_src);
- err |= __put_user(temp.uint, &cmd->scan_begin_src);
- err |= __get_user(temp.uint, &cmd32->scan_begin_arg);
- err |= __put_user(temp.uint, &cmd->scan_begin_arg);
- err |= __get_user(temp.uint, &cmd32->convert_src);
- err |= __put_user(temp.uint, &cmd->convert_src);
- err |= __get_user(temp.uint, &cmd32->convert_arg);
- err |= __put_user(temp.uint, &cmd->convert_arg);
- err |= __get_user(temp.uint, &cmd32->scan_end_src);
- err |= __put_user(temp.uint, &cmd->scan_end_src);
- err |= __get_user(temp.uint, &cmd32->scan_end_arg);
- err |= __put_user(temp.uint, &cmd->scan_end_arg);
- err |= __get_user(temp.uint, &cmd32->stop_src);
- err |= __put_user(temp.uint, &cmd->stop_src);
- err |= __get_user(temp.uint, &cmd32->stop_arg);
- err |= __put_user(temp.uint, &cmd->stop_arg);
- err |= __get_user(temp.uptr, &cmd32->chanlist);
- err |= __put_user((unsigned int __force *)compat_ptr(temp.uptr),
- &cmd->chanlist);
- err |= __get_user(temp.uint, &cmd32->chanlist_len);
- err |= __put_user(temp.uint, &cmd->chanlist_len);
- err |= __get_user(temp.uptr, &cmd32->data);
- err |= __put_user(compat_ptr(temp.uptr), &cmd->data);
- err |= __get_user(temp.uint, &cmd32->data_len);
- err |= __put_user(temp.uint, &cmd->data_len);
- return err ? -EFAULT : 0;
+ cmd->subdev = v32.subdev;
+ cmd->flags = v32.flags;
+ cmd->start_src = v32.start_src;
+ cmd->start_arg = v32.start_arg;
+ cmd->scan_begin_src = v32.scan_begin_src;
+ cmd->scan_begin_arg = v32.scan_begin_arg;
+ cmd->convert_src = v32.convert_src;
+ cmd->convert_arg = v32.convert_arg;
+ cmd->scan_end_src = v32.scan_end_src;
+ cmd->scan_end_arg = v32.scan_end_arg;
+ cmd->stop_src = v32.stop_src;
+ cmd->stop_arg = v32.stop_arg;
+ cmd->chanlist = compat_ptr(v32.chanlist);
+ cmd->chanlist_len = v32.chanlist_len;
+ cmd->data = compat_ptr(v32.data);
+ cmd->data_len = v32.data_len;
+ return 0;
}
/* Copy native cmd structure to 32-bit cmd structure. */
static int put_compat_cmd(struct comedi32_cmd_struct __user *cmd32,
- struct comedi_cmd __user *cmd)
-{
- int err;
- unsigned int temp;
-
- /*
- * Copy back most of cmd structure.
- *
- * Assume the pointer values are already valid.
- * (Could use ptr_to_compat() to set them.)
- */
- if (!access_ok(cmd, sizeof(*cmd)) ||
- !access_ok(cmd32, sizeof(*cmd32)))
- return -EFAULT;
-
- err = 0;
- err |= __get_user(temp, &cmd->subdev);
- err |= __put_user(temp, &cmd32->subdev);
- err |= __get_user(temp, &cmd->flags);
- err |= __put_user(temp, &cmd32->flags);
- err |= __get_user(temp, &cmd->start_src);
- err |= __put_user(temp, &cmd32->start_src);
- err |= __get_user(temp, &cmd->start_arg);
- err |= __put_user(temp, &cmd32->start_arg);
- err |= __get_user(temp, &cmd->scan_begin_src);
- err |= __put_user(temp, &cmd32->scan_begin_src);
- err |= __get_user(temp, &cmd->scan_begin_arg);
- err |= __put_user(temp, &cmd32->scan_begin_arg);
- err |= __get_user(temp, &cmd->convert_src);
- err |= __put_user(temp, &cmd32->convert_src);
- err |= __get_user(temp, &cmd->convert_arg);
- err |= __put_user(temp, &cmd32->convert_arg);
- err |= __get_user(temp, &cmd->scan_end_src);
- err |= __put_user(temp, &cmd32->scan_end_src);
- err |= __get_user(temp, &cmd->scan_end_arg);
- err |= __put_user(temp, &cmd32->scan_end_arg);
- err |= __get_user(temp, &cmd->stop_src);
- err |= __put_user(temp, &cmd32->stop_src);
- err |= __get_user(temp, &cmd->stop_arg);
- err |= __put_user(temp, &cmd32->stop_arg);
+ struct comedi_cmd *cmd)
+{
+ struct comedi32_cmd_struct v32;
+
+ memset(&v32, 0, sizeof(v32));
+ v32.subdev = cmd->subdev;
+ v32.flags = cmd->flags;
+ v32.start_src = cmd->start_src;
+ v32.start_arg = cmd->start_arg;
+ v32.scan_begin_src = cmd->scan_begin_src;
+ v32.scan_begin_arg = cmd->scan_begin_arg;
+ v32.convert_src = cmd->convert_src;
+ v32.convert_arg = cmd->convert_arg;
+ v32.scan_end_src = cmd->scan_end_src;
+ v32.scan_end_arg = cmd->scan_end_arg;
+ v32.stop_src = cmd->stop_src;
+ v32.stop_arg = cmd->stop_arg;
/* Assume chanlist pointer is unchanged. */
- err |= __get_user(temp, &cmd->chanlist_len);
- err |= __put_user(temp, &cmd32->chanlist_len);
- /* Assume data pointer is unchanged. */
- err |= __get_user(temp, &cmd->data_len);
- err |= __put_user(temp, &cmd32->data_len);
- return err ? -EFAULT : 0;
+ v32.chanlist = ptr_to_compat(cmd->chanlist);
+ v32.chanlist_len = cmd->chanlist_len;
+ v32.data = ptr_to_compat(cmd->data);
+ v32.data_len = cmd->data_len;
+ return copy_to_user(cmd32, &v32, sizeof(v32));
}
/* Handle 32-bit COMEDI_CMD ioctl. */
static int compat_cmd(struct file *file, unsigned long arg)
{
- struct comedi_cmd __user *cmd;
- struct comedi32_cmd_struct __user *cmd32;
+ struct comedi_file *cfp = file->private_data;
+ struct comedi_device *dev = cfp->dev;
+ struct comedi_cmd cmd;
+ bool copy = false;
int rc, err;
- cmd32 = compat_ptr(arg);
- cmd = compat_alloc_user_space(sizeof(*cmd));
-
- rc = get_compat_cmd(cmd, cmd32);
+ rc = get_compat_cmd(&cmd, compat_ptr(arg));
if (rc)
return rc;
- rc = comedi_unlocked_ioctl(file, COMEDI_CMD, (unsigned long)cmd);
- if (rc == -EAGAIN) {
+ mutex_lock(&dev->mutex);
+ rc = do_cmd_ioctl(dev, &cmd, ©, file);
+ mutex_unlock(&dev->mutex);
+ if (copy) {
/* Special case: copy cmd back to user. */
- err = put_compat_cmd(cmd32, cmd);
+ err = put_compat_cmd(compat_ptr(arg), &cmd);
if (err)
rc = err;
}
-
return rc;
}
/* Handle 32-bit COMEDI_CMDTEST ioctl. */
static int compat_cmdtest(struct file *file, unsigned long arg)
{
- struct comedi_cmd __user *cmd;
- struct comedi32_cmd_struct __user *cmd32;
+ struct comedi_file *cfp = file->private_data;
+ struct comedi_device *dev = cfp->dev;
+ struct comedi_cmd cmd;
+ bool copy = false;
int rc, err;
- cmd32 = compat_ptr(arg);
- cmd = compat_alloc_user_space(sizeof(*cmd));
-
- rc = get_compat_cmd(cmd, cmd32);
+ rc = get_compat_cmd(&cmd, compat_ptr(arg));
if (rc)
return rc;
- rc = comedi_unlocked_ioctl(file, COMEDI_CMDTEST, (unsigned long)cmd);
- if (rc < 0)
- return rc;
-
- err = put_compat_cmd(cmd32, cmd);
- if (err)
- rc = err;
-
+ mutex_lock(&dev->mutex);
+ rc = do_cmdtest_ioctl(dev, &cmd, ©, file);
+ mutex_unlock(&dev->mutex);
+ if (copy) {
+ err = put_compat_cmd(compat_ptr(arg), &cmd);
+ if (err)
+ rc = err;
+ }
return rc;
}
--
2.11.0
On Fri, May 29, 2020 at 08:06:14AM +0300, Jani Nikula wrote:
> On Fri, 29 May 2020, Al Viro <[email protected]> wrote:
> > Low-hanging fruit in i915 uaccess-related stuff.
> > There's some subtler stuff remaining after that; these
> > are the simple ones.
>
> Please Cc: [email protected] for i915 changes.
Will do. Do you want me to resend those (or post them there separately, or...)?
> Also added Chris who I believe will be able to best review the changes.
FWIW, the branch is in
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git #uaccess.i915
The stuff that doesn't fit anywhere else. Hopefully
saner marshalling for weird 7-argument syscalls (pselect6()),
low-hanging fruit in several binfmt, unsafe_put_user-based
x86 cp_stat64(), etc. - there's really no common topic here.
BTW, after that series there's no more __clear_user()
callers outside of arch/* and damn few in arch/*, other than
clear_user() instances themselves...
Branch is uaccess.misc, based at uaccess.base.
Al Viro (9):
pselect6() and friends: take handling the combined 6th/7th args into helper
binfmt_elf: don't bother with __{put,copy_to}_user()
binfmt_elf_fdpic: don't use __... uaccess primitives
binfmt_flat: don't use __put_user()
x86: switch cp_stat64() to unsafe_put_user()
TEST_ACCESS_OK _never_ had been checked anywhere
user_regset_copyout_zero(): use clear_user()
x86: kvm_hv_set_msr(): use __put_user() instead of 32bit __clear_user()
bpf: make bpf_check_uarg_tail_zero() use check_zeroed_user()
arch/x86/include/asm/pgtable_32.h | 7 ---
arch/x86/kernel/sys_ia32.c | 40 ++++++++------
arch/x86/kvm/hyperv.c | 2 +-
fs/binfmt_elf.c | 14 ++---
fs/binfmt_elf_fdpic.c | 31 +++++++----
fs/binfmt_flat.c | 22 +++++---
fs/select.c | 112 ++++++++++++++++++++++----------------
include/linux/regset.h | 2 +-
kernel/bpf/syscall.c | 25 ++-------
9 files changed, 134 insertions(+), 121 deletions(-)
From: Al Viro <[email protected]>
... and use unsafe_get_user(), while we are at it.
Signed-off-by: Al Viro <[email protected]>
---
fs/select.c | 112 ++++++++++++++++++++++++++++++++++--------------------------
1 file changed, 64 insertions(+), 48 deletions(-)
diff --git a/fs/select.c b/fs/select.c
index 11d0285d46b7..7aef49552d4c 100644
--- a/fs/select.c
+++ b/fs/select.c
@@ -766,22 +766,38 @@ static long do_pselect(int n, fd_set __user *inp, fd_set __user *outp,
* which has a pointer to the sigset_t itself followed by a size_t containing
* the sigset size.
*/
+struct sigset_argpack {
+ sigset_t __user *p;
+ size_t size;
+};
+
+static inline int get_sigset_argpack(struct sigset_argpack *to,
+ struct sigset_argpack __user *from)
+{
+ // the path is hot enough for overhead of copy_from_user() to matter
+ if (from) {
+ if (!user_read_access_begin(from, sizeof(*from)))
+ return -EFAULT;
+ unsafe_get_user(to->p, &from->p, Efault);
+ unsafe_get_user(to->size, &from->size, Efault);
+ user_read_access_end();
+ }
+ return 0;
+Efault:
+ user_access_end();
+ return -EFAULT;
+}
+
SYSCALL_DEFINE6(pselect6, int, n, fd_set __user *, inp, fd_set __user *, outp,
fd_set __user *, exp, struct __kernel_timespec __user *, tsp,
void __user *, sig)
{
- size_t sigsetsize = 0;
- sigset_t __user *up = NULL;
-
- if (sig) {
- if (!access_ok(sig, sizeof(void *)+sizeof(size_t))
- || __get_user(up, (sigset_t __user * __user *)sig)
- || __get_user(sigsetsize,
- (size_t __user *)(sig+sizeof(void *))))
- return -EFAULT;
- }
+ struct sigset_argpack x = {NULL, 0};
+
+ if (get_sigset_argpack(&x, sig))
+ return -EFAULT;
- return do_pselect(n, inp, outp, exp, tsp, up, sigsetsize, PT_TIMESPEC);
+ return do_pselect(n, inp, outp, exp, tsp, x.p, x.size, PT_TIMESPEC);
}
#if defined(CONFIG_COMPAT_32BIT_TIME) && !defined(CONFIG_64BIT)
@@ -790,18 +806,12 @@ SYSCALL_DEFINE6(pselect6_time32, int, n, fd_set __user *, inp, fd_set __user *,
fd_set __user *, exp, struct old_timespec32 __user *, tsp,
void __user *, sig)
{
- size_t sigsetsize = 0;
- sigset_t __user *up = NULL;
-
- if (sig) {
- if (!access_ok(sig, sizeof(void *)+sizeof(size_t))
- || __get_user(up, (sigset_t __user * __user *)sig)
- || __get_user(sigsetsize,
- (size_t __user *)(sig+sizeof(void *))))
- return -EFAULT;
- }
+ struct sigset_argpack x = {NULL, 0};
+
+ if (get_sigset_argpack(&x, sig))
+ return -EFAULT;
- return do_pselect(n, inp, outp, exp, tsp, up, sigsetsize, PT_OLD_TIMESPEC);
+ return do_pselect(n, inp, outp, exp, tsp, x.p, x.size, PT_OLD_TIMESPEC);
}
#endif
@@ -1325,24 +1335,37 @@ static long do_compat_pselect(int n, compat_ulong_t __user *inp,
return poll_select_finish(&end_time, tsp, type, ret);
}
+struct compat_sigset_argpack {
+ compat_uptr_t p;
+ compat_size_t size;
+};
+static inline int get_compat_sigset_argpack(struct compat_sigset_argpack *to,
+ struct compat_sigset_argpack __user *from)
+{
+ if (from) {
+ if (!user_read_access_begin(from, sizeof(*from)))
+ return -EFAULT;
+ unsafe_get_user(to->p, &from->p, Efault);
+ unsafe_get_user(to->size, &from->size, Efault);
+ user_read_access_end();
+ }
+ return 0;
+Efault:
+ user_access_end();
+ return -EFAULT;
+}
+
COMPAT_SYSCALL_DEFINE6(pselect6_time64, int, n, compat_ulong_t __user *, inp,
compat_ulong_t __user *, outp, compat_ulong_t __user *, exp,
struct __kernel_timespec __user *, tsp, void __user *, sig)
{
- compat_size_t sigsetsize = 0;
- compat_uptr_t up = 0;
-
- if (sig) {
- if (!access_ok(sig,
- sizeof(compat_uptr_t)+sizeof(compat_size_t)) ||
- __get_user(up, (compat_uptr_t __user *)sig) ||
- __get_user(sigsetsize,
- (compat_size_t __user *)(sig+sizeof(up))))
- return -EFAULT;
- }
+ struct compat_sigset_argpack x = {0, 0};
+
+ if (get_compat_sigset_argpack(&x, sig))
+ return -EFAULT;
- return do_compat_pselect(n, inp, outp, exp, tsp, compat_ptr(up),
- sigsetsize, PT_TIMESPEC);
+ return do_compat_pselect(n, inp, outp, exp, tsp, compat_ptr(x.p),
+ x.size, PT_TIMESPEC);
}
#if defined(CONFIG_COMPAT_32BIT_TIME)
@@ -1351,20 +1374,13 @@ COMPAT_SYSCALL_DEFINE6(pselect6_time32, int, n, compat_ulong_t __user *, inp,
compat_ulong_t __user *, outp, compat_ulong_t __user *, exp,
struct old_timespec32 __user *, tsp, void __user *, sig)
{
- compat_size_t sigsetsize = 0;
- compat_uptr_t up = 0;
-
- if (sig) {
- if (!access_ok(sig,
- sizeof(compat_uptr_t)+sizeof(compat_size_t)) ||
- __get_user(up, (compat_uptr_t __user *)sig) ||
- __get_user(sigsetsize,
- (compat_size_t __user *)(sig+sizeof(up))))
- return -EFAULT;
- }
+ struct compat_sigset_argpack x = {0, 0};
+
+ if (get_compat_sigset_argpack(&x, sig))
+ return -EFAULT;
- return do_compat_pselect(n, inp, outp, exp, tsp, compat_ptr(up),
- sigsetsize, PT_OLD_TIMESPEC);
+ return do_compat_pselect(n, inp, outp, exp, tsp, compat_ptr(x.p),
+ x.size, PT_OLD_TIMESPEC);
}
#endif
--
2.11.0
From: Al Viro <[email protected]>
... and check the return value
Signed-off-by: Al Viro <[email protected]>
---
fs/binfmt_flat.c | 22 ++++++++++++++--------
1 file changed, 14 insertions(+), 8 deletions(-)
diff --git a/fs/binfmt_flat.c b/fs/binfmt_flat.c
index 831a2b25ba79..7b663ed5247b 100644
--- a/fs/binfmt_flat.c
+++ b/fs/binfmt_flat.c
@@ -138,35 +138,40 @@ static int create_flat_tables(struct linux_binprm *bprm, unsigned long arg_start
current->mm->start_stack = (unsigned long)sp & -FLAT_STACK_ALIGN;
sp = (unsigned long __user *)current->mm->start_stack;
- __put_user(bprm->argc, sp++);
+ if (put_user(bprm->argc, sp++))
+ return -EFAULT;
if (IS_ENABLED(CONFIG_BINFMT_FLAT_ARGVP_ENVP_ON_STACK)) {
unsigned long argv, envp;
argv = (unsigned long)(sp + 2);
envp = (unsigned long)(sp + 2 + bprm->argc + 1);
- __put_user(argv, sp++);
- __put_user(envp, sp++);
+ if (put_user(argv, sp++) || put_user(envp, sp++))
+ return -EFAULT;
}
current->mm->arg_start = (unsigned long)p;
for (i = bprm->argc; i > 0; i--) {
- __put_user((unsigned long)p, sp++);
+ if (put_user((unsigned long)p, sp++))
+ return -EFAULT;
len = strnlen_user(p, MAX_ARG_STRLEN);
if (!len || len > MAX_ARG_STRLEN)
return -EINVAL;
p += len;
}
- __put_user(0, sp++);
+ if (put_user(0, sp++))
+ return -EFAULT;
current->mm->arg_end = (unsigned long)p;
current->mm->env_start = (unsigned long) p;
for (i = bprm->envc; i > 0; i--) {
- __put_user((unsigned long)p, sp++);
+ if (put_user((unsigned long)p, sp++))
+ return -EFAULT;
len = strnlen_user(p, MAX_ARG_STRLEN);
if (!len || len > MAX_ARG_STRLEN)
return -EINVAL;
p += len;
}
- __put_user(0, sp++);
+ if (put_user(0, sp++))
+ return -EFAULT;
current->mm->env_end = (unsigned long)p;
return 0;
@@ -998,7 +1003,8 @@ static int load_flat_binary(struct linux_binprm *bprm)
unsigned long __user *sp;
current->mm->start_stack -= sizeof(unsigned long);
sp = (unsigned long __user *)current->mm->start_stack;
- __put_user(start_addr, sp);
+ if (put_user(start_addr, sp))
+ return -EFAULT;
start_addr = libinfo.lib_list[i].entry;
}
}
--
2.11.0
From: Al Viro <[email protected]>
Signed-off-by: Al Viro <[email protected]>
---
fs/binfmt_elf_fdpic.c | 31 +++++++++++++++++++------------
1 file changed, 19 insertions(+), 12 deletions(-)
diff --git a/fs/binfmt_elf_fdpic.c b/fs/binfmt_elf_fdpic.c
index 240f66663543..ab6fb117b33c 100644
--- a/fs/binfmt_elf_fdpic.c
+++ b/fs/binfmt_elf_fdpic.c
@@ -537,7 +537,7 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
platform_len = strlen(k_platform) + 1;
sp -= platform_len;
u_platform = (char __user *) sp;
- if (__copy_to_user(u_platform, k_platform, platform_len) != 0)
+ if (copy_to_user(u_platform, k_platform, platform_len) != 0)
return -EFAULT;
}
@@ -552,7 +552,7 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
platform_len = strlen(k_base_platform) + 1;
sp -= platform_len;
u_base_platform = (char __user *) sp;
- if (__copy_to_user(u_base_platform, k_base_platform, platform_len) != 0)
+ if (copy_to_user(u_base_platform, k_base_platform, platform_len) != 0)
return -EFAULT;
}
@@ -604,11 +604,13 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
/* put the ELF interpreter info on the stack */
#define NEW_AUX_ENT(id, val) \
do { \
- struct { unsigned long _id, _val; } __user *ent; \
+ struct { unsigned long _id, _val; } __user *ent, v; \
\
ent = (void __user *) csp; \
- __put_user((id), &ent[nr]._id); \
- __put_user((val), &ent[nr]._val); \
+ v._id = (id); \
+ v._val = (val); \
+ if (copy_to_user(ent + nr, &v)) \
+ return -EFAULT; \
nr++; \
} while (0)
@@ -675,7 +677,8 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
/* stack argc */
csp -= sizeof(unsigned long);
- __put_user(bprm->argc, (unsigned long __user *) csp);
+ if (put_user(bprm->argc, (unsigned long __user *) csp))
+ return -EFAULT;
BUG_ON(csp != sp);
@@ -689,25 +692,29 @@ static int create_elf_fdpic_tables(struct linux_binprm *bprm,
p = (char __user *) current->mm->arg_start;
for (loop = bprm->argc; loop > 0; loop--) {
- __put_user((elf_caddr_t) p, argv++);
+ if (put_user((elf_caddr_t) p, argv++))
+ return -EFAULT;
len = strnlen_user(p, MAX_ARG_STRLEN);
if (!len || len > MAX_ARG_STRLEN)
return -EINVAL;
p += len;
}
- __put_user(NULL, argv);
+ if (put_user(NULL, argv))
+ return -EFAULT;
current->mm->arg_end = (unsigned long) p;
/* fill in the envv[] array */
current->mm->env_start = (unsigned long) p;
for (loop = bprm->envc; loop > 0; loop--) {
- __put_user((elf_caddr_t)(unsigned long) p, envp++);
+ if (put_user((elf_caddr_t)(unsigned long) p, envp++))
+ return -EFAULT;
len = strnlen_user(p, MAX_ARG_STRLEN);
if (!len || len > MAX_ARG_STRLEN)
return -EINVAL;
p += len;
}
- __put_user(NULL, envp);
+ if (put_user(NULL, envp))
+ return -EFAULT;
current->mm->env_end = (unsigned long) p;
mm->start_stack = (unsigned long) sp;
@@ -849,8 +856,8 @@ static int elf_fdpic_map_file(struct elf_fdpic_params *params,
tmp = phdr->p_memsz / sizeof(Elf32_Dyn);
dyn = (Elf32_Dyn __user *)params->dynamic_addr;
- __get_user(d_tag, &dyn[tmp - 1].d_tag);
- if (d_tag != 0)
+ if (get_user(d_tag, &dyn[tmp - 1].d_tag) ||
+ d_tag != 0)
goto dynamic_error;
break;
}
--
2.11.0
From: Al Viro <[email protected]>
Signed-off-by: Al Viro <[email protected]>
---
fs/binfmt_elf.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 13f25e241ac4..782d218c3bb9 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -202,7 +202,7 @@ create_elf_tables(struct linux_binprm *bprm, const struct elfhdr *exec,
size_t len = strlen(k_platform) + 1;
u_platform = (elf_addr_t __user *)STACK_ALLOC(p, len);
- if (__copy_to_user(u_platform, k_platform, len))
+ if (copy_to_user(u_platform, k_platform, len))
return -EFAULT;
}
@@ -215,7 +215,7 @@ create_elf_tables(struct linux_binprm *bprm, const struct elfhdr *exec,
size_t len = strlen(k_base_platform) + 1;
u_base_platform = (elf_addr_t __user *)STACK_ALLOC(p, len);
- if (__copy_to_user(u_base_platform, k_base_platform, len))
+ if (copy_to_user(u_base_platform, k_base_platform, len))
return -EFAULT;
}
@@ -225,7 +225,7 @@ create_elf_tables(struct linux_binprm *bprm, const struct elfhdr *exec,
get_random_bytes(k_rand_bytes, sizeof(k_rand_bytes));
u_rand_bytes = (elf_addr_t __user *)
STACK_ALLOC(p, sizeof(k_rand_bytes));
- if (__copy_to_user(u_rand_bytes, k_rand_bytes, sizeof(k_rand_bytes)))
+ if (copy_to_user(u_rand_bytes, k_rand_bytes, sizeof(k_rand_bytes)))
return -EFAULT;
/* Create the ELF interpreter info */
@@ -308,14 +308,14 @@ create_elf_tables(struct linux_binprm *bprm, const struct elfhdr *exec,
return -EFAULT;
/* Now, let's put argc (and argv, envp if appropriate) on the stack */
- if (__put_user(argc, sp++))
+ if (put_user(argc, sp++))
return -EFAULT;
/* Populate list of argv pointers back to argv strings. */
p = mm->arg_end = mm->arg_start;
while (argc-- > 0) {
size_t len;
- if (__put_user((elf_addr_t)p, sp++))
+ if (put_user((elf_addr_t)p, sp++))
return -EFAULT;
len = strnlen_user((void __user *)p, MAX_ARG_STRLEN);
if (!len || len > MAX_ARG_STRLEN)
@@ -330,14 +330,14 @@ create_elf_tables(struct linux_binprm *bprm, const struct elfhdr *exec,
mm->env_end = mm->env_start = p;
while (envc-- > 0) {
size_t len;
- if (__put_user((elf_addr_t)p, sp++))
+ if (put_user((elf_addr_t)p, sp++))
return -EFAULT;
len = strnlen_user((void __user *)p, MAX_ARG_STRLEN);
if (!len || len > MAX_ARG_STRLEN)
return -EINVAL;
p += len;
}
- if (__put_user(0, sp++))
+ if (put_user(0, sp++))
return -EFAULT;
mm->env_end = p;
--
2.11.0
From: Al Viro <[email protected]>
that's the only caller of __clear_user() in generic code, and it's
not hot enough to bother with skipping access_ok().
Signed-off-by: Al Viro <[email protected]>
---
include/linux/regset.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/regset.h b/include/linux/regset.h
index bf0243779738..46d6ae68c455 100644
--- a/include/linux/regset.h
+++ b/include/linux/regset.h
@@ -320,7 +320,7 @@ static inline int user_regset_copyout_zero(unsigned int *pos,
if (*kbuf) {
memset(*kbuf, 0, copy);
*kbuf += copy;
- } else if (__clear_user(*ubuf, copy))
+ } else if (clear_user(*ubuf, copy))
return -EFAULT;
else
*ubuf += copy;
--
2.11.0
From: Al Viro <[email protected]>
Signed-off-by: Al Viro <[email protected]>
---
arch/x86/kvm/hyperv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/kvm/hyperv.c b/arch/x86/kvm/hyperv.c
index bcefa9d4e57e..b85b211d4676 100644
--- a/arch/x86/kvm/hyperv.c
+++ b/arch/x86/kvm/hyperv.c
@@ -1129,7 +1129,7 @@ static int kvm_hv_set_msr(struct kvm_vcpu *vcpu, u32 msr, u64 data, bool host)
* only, there can be valuable data in the rest which needs
* to be preserved e.g. on migration.
*/
- if (__clear_user((void __user *)addr, sizeof(u32)))
+ if (__put_user(0, (u32 __user *)addr))
return 1;
hv_vcpu->hv_vapic = data;
kvm_vcpu_mark_page_dirty(vcpu, gfn);
--
2.11.0
From: Al Viro <[email protected]>
Signed-off-by: Al Viro <[email protected]>
---
arch/x86/kernel/sys_ia32.c | 40 ++++++++++++++++++++++------------------
1 file changed, 22 insertions(+), 18 deletions(-)
diff --git a/arch/x86/kernel/sys_ia32.c b/arch/x86/kernel/sys_ia32.c
index ab03fede1422..f8d65c99feb8 100644
--- a/arch/x86/kernel/sys_ia32.c
+++ b/arch/x86/kernel/sys_ia32.c
@@ -135,26 +135,30 @@ static int cp_stat64(struct stat64 __user *ubuf, struct kstat *stat)
typeof(ubuf->st_gid) gid = 0;
SET_UID(uid, from_kuid_munged(current_user_ns(), stat->uid));
SET_GID(gid, from_kgid_munged(current_user_ns(), stat->gid));
- if (!access_ok(ubuf, sizeof(struct stat64)) ||
- __put_user(huge_encode_dev(stat->dev), &ubuf->st_dev) ||
- __put_user(stat->ino, &ubuf->__st_ino) ||
- __put_user(stat->ino, &ubuf->st_ino) ||
- __put_user(stat->mode, &ubuf->st_mode) ||
- __put_user(stat->nlink, &ubuf->st_nlink) ||
- __put_user(uid, &ubuf->st_uid) ||
- __put_user(gid, &ubuf->st_gid) ||
- __put_user(huge_encode_dev(stat->rdev), &ubuf->st_rdev) ||
- __put_user(stat->size, &ubuf->st_size) ||
- __put_user(stat->atime.tv_sec, &ubuf->st_atime) ||
- __put_user(stat->atime.tv_nsec, &ubuf->st_atime_nsec) ||
- __put_user(stat->mtime.tv_sec, &ubuf->st_mtime) ||
- __put_user(stat->mtime.tv_nsec, &ubuf->st_mtime_nsec) ||
- __put_user(stat->ctime.tv_sec, &ubuf->st_ctime) ||
- __put_user(stat->ctime.tv_nsec, &ubuf->st_ctime_nsec) ||
- __put_user(stat->blksize, &ubuf->st_blksize) ||
- __put_user(stat->blocks, &ubuf->st_blocks))
+ if (!user_write_access_begin(ubuf, sizeof(struct stat64)))
return -EFAULT;
+ unsafe_put_user(huge_encode_dev(stat->dev), &ubuf->st_dev, Efault);
+ unsafe_put_user(stat->ino, &ubuf->__st_ino, Efault);
+ unsafe_put_user(stat->ino, &ubuf->st_ino, Efault);
+ unsafe_put_user(stat->mode, &ubuf->st_mode, Efault);
+ unsafe_put_user(stat->nlink, &ubuf->st_nlink, Efault);
+ unsafe_put_user(uid, &ubuf->st_uid, Efault);
+ unsafe_put_user(gid, &ubuf->st_gid, Efault);
+ unsafe_put_user(huge_encode_dev(stat->rdev), &ubuf->st_rdev, Efault);
+ unsafe_put_user(stat->size, &ubuf->st_size, Efault);
+ unsafe_put_user(stat->atime.tv_sec, &ubuf->st_atime, Efault);
+ unsafe_put_user(stat->atime.tv_nsec, &ubuf->st_atime_nsec, Efault);
+ unsafe_put_user(stat->mtime.tv_sec, &ubuf->st_mtime, Efault);
+ unsafe_put_user(stat->mtime.tv_nsec, &ubuf->st_mtime_nsec, Efault);
+ unsafe_put_user(stat->ctime.tv_sec, &ubuf->st_ctime, Efault);
+ unsafe_put_user(stat->ctime.tv_nsec, &ubuf->st_ctime_nsec, Efault);
+ unsafe_put_user(stat->blksize, &ubuf->st_blksize, Efault);
+ unsafe_put_user(stat->blocks, &ubuf->st_blocks, Efault);
+ user_access_end();
return 0;
+Efault:
+ user_write_access_end();
+ return -EFAULT;
}
COMPAT_SYSCALL_DEFINE2(ia32_stat64, const char __user *, filename,
--
2.11.0
From: Al Viro <[email protected]>
Once upon a time the predecessor of that thing (TEST_VERIFY_AREA)
used to be. However, that had been gone for years now (and
the patch that introduced TEST_ACCESS_OK has not touched any
ifdefs - they got gradually removed later). Just bury it...
Signed-off-by: Al Viro <[email protected]>
---
arch/x86/include/asm/pgtable_32.h | 7 -------
1 file changed, 7 deletions(-)
diff --git a/arch/x86/include/asm/pgtable_32.h b/arch/x86/include/asm/pgtable_32.h
index 0dca7f7aeff2..fb10f2f8f4f0 100644
--- a/arch/x86/include/asm/pgtable_32.h
+++ b/arch/x86/include/asm/pgtable_32.h
@@ -32,13 +32,6 @@ extern pmd_t initial_pg_pmd[];
void paging_init(void);
void sync_initial_page_table(void);
-/*
- * Define this if things work differently on an i386 and an i486:
- * it will (on an i486) warn about kernel memory accesses that are
- * done without a 'access_ok( ..)'
- */
-#undef TEST_ACCESS_OK
-
#ifdef CONFIG_X86_PAE
# include <asm/pgtable-3level.h>
#else
--
2.11.0
From: Al Viro <[email protected]>
... rather than open-coding it, and badly, at that.
Signed-off-by: Al Viro <[email protected]>
---
kernel/bpf/syscall.c | 25 ++++++-------------------
1 file changed, 6 insertions(+), 19 deletions(-)
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 64783da34202..41ba746ecbc2 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -67,32 +67,19 @@ int bpf_check_uarg_tail_zero(void __user *uaddr,
size_t expected_size,
size_t actual_size)
{
- unsigned char __user *addr;
- unsigned char __user *end;
- unsigned char val;
- int err;
+ unsigned char __user *addr = uaddr + expected_size;
+ int res;
if (unlikely(actual_size > PAGE_SIZE)) /* silly large */
return -E2BIG;
- if (unlikely(!access_ok(uaddr, actual_size)))
- return -EFAULT;
-
if (actual_size <= expected_size)
return 0;
- addr = uaddr + expected_size;
- end = uaddr + actual_size;
-
- for (; addr < end; addr++) {
- err = get_user(val, addr);
- if (err)
- return err;
- if (val)
- return -E2BIG;
- }
-
- return 0;
+ res = check_zeroed_user(addr, actual_size - expected_size);
+ if (res < 0)
+ return res;
+ return res ? 0 : -E2BIG;
}
const struct bpf_map_ops bpf_map_offload_ops = {
--
2.11.0
hpsa compat ioctl done (hopefully) saner. I really want
to kill compat_alloc_user_space() off - it's always trouble and
for a driver-private ioctls it's absolutely pointless.
Note that this is only compile-tested - I don't have the
hardware to test it on *or* userland to issue the ioctls in
question. So this series definitely needs a review and testing
from hpsa maintainers before it might go anywhere.
The series is in vfs.git #uaccess.hpsa, based at v5.7-rc1
Al Viro (4):
hpsa passthrough: lift {BIG_,}IOCTL_Command_struct copy{in,out} into hpsa_ioctl()
hpsa: don't bother with vmalloc for BIG_IOCTL_Command_struct
hpsa: get rid of compat_alloc_user_space()
hpsa_ioctl(): tidy up a bit
drivers/scsi/hpsa.c | 199 ++++++++++++++++++++++++----------------------------
1 file changed, 90 insertions(+), 109 deletions(-)
From: Al Viro <[email protected]>
Signed-off-by: Al Viro <[email protected]>
---
drivers/scsi/hpsa.c | 116 +++++++++++++++++++++++++---------------------------
1 file changed, 56 insertions(+), 60 deletions(-)
diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 1e9302e99d05..3344a06c938e 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -6358,37 +6358,33 @@ static int hpsa_getdrivver_ioctl(struct ctlr_info *h, void __user *argp)
return 0;
}
-static int hpsa_passthru_ioctl(struct ctlr_info *h, void __user *argp)
+static int hpsa_passthru_ioctl(struct ctlr_info *h,
+ IOCTL_Command_struct *iocommand)
{
- IOCTL_Command_struct iocommand;
struct CommandList *c;
char *buff = NULL;
u64 temp64;
int rc = 0;
- if (!argp)
- return -EINVAL;
if (!capable(CAP_SYS_RAWIO))
return -EPERM;
- if (copy_from_user(&iocommand, argp, sizeof(iocommand)))
- return -EFAULT;
- if ((iocommand.buf_size < 1) &&
- (iocommand.Request.Type.Direction != XFER_NONE)) {
+ if ((iocommand->buf_size < 1) &&
+ (iocommand->Request.Type.Direction != XFER_NONE)) {
return -EINVAL;
}
- if (iocommand.buf_size > 0) {
- buff = kmalloc(iocommand.buf_size, GFP_KERNEL);
+ if (iocommand->buf_size > 0) {
+ buff = kmalloc(iocommand->buf_size, GFP_KERNEL);
if (buff == NULL)
return -ENOMEM;
- if (iocommand.Request.Type.Direction & XFER_WRITE) {
+ if (iocommand->Request.Type.Direction & XFER_WRITE) {
/* Copy the data into the buffer we created */
- if (copy_from_user(buff, iocommand.buf,
- iocommand.buf_size)) {
+ if (copy_from_user(buff, iocommand->buf,
+ iocommand->buf_size)) {
rc = -EFAULT;
goto out_kfree;
}
} else {
- memset(buff, 0, iocommand.buf_size);
+ memset(buff, 0, iocommand->buf_size);
}
}
c = cmd_alloc(h);
@@ -6398,23 +6394,23 @@ static int hpsa_passthru_ioctl(struct ctlr_info *h, void __user *argp)
c->scsi_cmd = SCSI_CMD_BUSY;
/* Fill in Command Header */
c->Header.ReplyQueue = 0; /* unused in simple mode */
- if (iocommand.buf_size > 0) { /* buffer to fill */
+ if (iocommand->buf_size > 0) { /* buffer to fill */
c->Header.SGList = 1;
c->Header.SGTotal = cpu_to_le16(1);
} else { /* no buffers to fill */
c->Header.SGList = 0;
c->Header.SGTotal = cpu_to_le16(0);
}
- memcpy(&c->Header.LUN, &iocommand.LUN_info, sizeof(c->Header.LUN));
+ memcpy(&c->Header.LUN, &iocommand->LUN_info, sizeof(c->Header.LUN));
/* Fill in Request block */
- memcpy(&c->Request, &iocommand.Request,
+ memcpy(&c->Request, &iocommand->Request,
sizeof(c->Request));
/* Fill in the scatter gather information */
- if (iocommand.buf_size > 0) {
+ if (iocommand->buf_size > 0) {
temp64 = dma_map_single(&h->pdev->dev, buff,
- iocommand.buf_size, DMA_BIDIRECTIONAL);
+ iocommand->buf_size, DMA_BIDIRECTIONAL);
if (dma_mapping_error(&h->pdev->dev, (dma_addr_t) temp64)) {
c->SG[0].Addr = cpu_to_le64(0);
c->SG[0].Len = cpu_to_le32(0);
@@ -6422,12 +6418,12 @@ static int hpsa_passthru_ioctl(struct ctlr_info *h, void __user *argp)
goto out;
}
c->SG[0].Addr = cpu_to_le64(temp64);
- c->SG[0].Len = cpu_to_le32(iocommand.buf_size);
+ c->SG[0].Len = cpu_to_le32(iocommand->buf_size);
c->SG[0].Ext = cpu_to_le32(HPSA_SG_LAST); /* not chaining */
}
rc = hpsa_scsi_do_simple_cmd(h, c, DEFAULT_REPLY_QUEUE,
NO_TIMEOUT);
- if (iocommand.buf_size > 0)
+ if (iocommand->buf_size > 0)
hpsa_pci_unmap(h->pdev, c, 1, DMA_BIDIRECTIONAL);
check_ioctl_unit_attention(h, c);
if (rc) {
@@ -6436,16 +6432,12 @@ static int hpsa_passthru_ioctl(struct ctlr_info *h, void __user *argp)
}
/* Copy the error information out */
- memcpy(&iocommand.error_info, c->err_info,
- sizeof(iocommand.error_info));
- if (copy_to_user(argp, &iocommand, sizeof(iocommand))) {
- rc = -EFAULT;
- goto out;
- }
- if ((iocommand.Request.Type.Direction & XFER_READ) &&
- iocommand.buf_size > 0) {
+ memcpy(&iocommand->error_info, c->err_info,
+ sizeof(iocommand->error_info));
+ if ((iocommand->Request.Type.Direction & XFER_READ) &&
+ iocommand->buf_size > 0) {
/* Copy the data out of the buffer we created */
- if (copy_to_user(iocommand.buf, buff, iocommand.buf_size)) {
+ if (copy_to_user(iocommand->buf, buff, iocommand->buf_size)) {
rc = -EFAULT;
goto out;
}
@@ -6457,9 +6449,9 @@ static int hpsa_passthru_ioctl(struct ctlr_info *h, void __user *argp)
return rc;
}
-static int hpsa_big_passthru_ioctl(struct ctlr_info *h, void __user *argp)
+static int hpsa_big_passthru_ioctl(struct ctlr_info *h,
+ BIG_IOCTL_Command_struct *ioc)
{
- BIG_IOCTL_Command_struct *ioc;
struct CommandList *c;
unsigned char **buff = NULL;
int *buff_size = NULL;
@@ -6470,29 +6462,17 @@ static int hpsa_big_passthru_ioctl(struct ctlr_info *h, void __user *argp)
u32 sz;
BYTE __user *data_ptr;
- if (!argp)
- return -EINVAL;
if (!capable(CAP_SYS_RAWIO))
return -EPERM;
- ioc = vmemdup_user(argp, sizeof(*ioc));
- if (IS_ERR(ioc)) {
- status = PTR_ERR(ioc);
- goto cleanup1;
- }
+
if ((ioc->buf_size < 1) &&
- (ioc->Request.Type.Direction != XFER_NONE)) {
- status = -EINVAL;
- goto cleanup1;
- }
+ (ioc->Request.Type.Direction != XFER_NONE))
+ return -EINVAL;
/* Check kmalloc limits using all SGs */
- if (ioc->malloc_size > MAX_KMALLOC_SIZE) {
- status = -EINVAL;
- goto cleanup1;
- }
- if (ioc->buf_size > ioc->malloc_size * SG_ENTRIES_IN_CMD) {
- status = -EINVAL;
- goto cleanup1;
- }
+ if (ioc->malloc_size > MAX_KMALLOC_SIZE)
+ return -EINVAL;
+ if (ioc->buf_size > ioc->malloc_size * SG_ENTRIES_IN_CMD)
+ return -EINVAL;
buff = kcalloc(SG_ENTRIES_IN_CMD, sizeof(char *), GFP_KERNEL);
if (!buff) {
status = -ENOMEM;
@@ -6565,10 +6545,6 @@ static int hpsa_big_passthru_ioctl(struct ctlr_info *h, void __user *argp)
/* Copy the error information out */
memcpy(&ioc->error_info, c->err_info, sizeof(ioc->error_info));
- if (copy_to_user(argp, ioc, sizeof(*ioc))) {
- status = -EFAULT;
- goto cleanup0;
- }
if ((ioc->Request.Type.Direction & XFER_READ) && ioc->buf_size > 0) {
int i;
@@ -6594,7 +6570,6 @@ static int hpsa_big_passthru_ioctl(struct ctlr_info *h, void __user *argp)
kfree(buff);
}
kfree(buff_size);
- kvfree(ioc);
return status;
}
@@ -6628,18 +6603,39 @@ static int hpsa_ioctl(struct scsi_device *dev, unsigned int cmd,
return hpsa_getpciinfo_ioctl(h, argp);
case CCISS_GETDRIVVER:
return hpsa_getdrivver_ioctl(h, argp);
- case CCISS_PASSTHRU:
+ case CCISS_PASSTHRU: {
+ IOCTL_Command_struct iocommand;
+
+ if (!argp)
+ return -EINVAL;
+ if (copy_from_user(&iocommand, argp, sizeof(iocommand)))
+ return -EFAULT;
if (atomic_dec_if_positive(&h->passthru_cmds_avail) < 0)
return -EAGAIN;
- rc = hpsa_passthru_ioctl(h, argp);
+ rc = hpsa_passthru_ioctl(h, &iocommand);
atomic_inc(&h->passthru_cmds_avail);
+ if (!rc && copy_to_user(argp, &iocommand, sizeof(iocommand)))
+ rc = -EFAULT;
return rc;
- case CCISS_BIG_PASSTHRU:
+ }
+ case CCISS_BIG_PASSTHRU: {
+ BIG_IOCTL_Command_struct *ioc;
+ if (!argp)
+ return -EINVAL;
if (atomic_dec_if_positive(&h->passthru_cmds_avail) < 0)
return -EAGAIN;
- rc = hpsa_big_passthru_ioctl(h, argp);
+ ioc = vmemdup_user(argp, sizeof(*ioc));
+ if (IS_ERR(ioc)) {
+ atomic_inc(&h->passthru_cmds_avail);
+ return PTR_ERR(ioc);
+ }
+ rc = hpsa_big_passthru_ioctl(h, ioc);
atomic_inc(&h->passthru_cmds_avail);
+ if (!rc && copy_to_user(argp, ioc, sizeof(*ioc)))
+ rc = -EFAULT;
+ kvfree(ioc);
return rc;
+ }
default:
return -ENOTTY;
}
--
2.11.0
On Fri, May 29, 2020 at 4:27 PM Al Viro <[email protected]> wrote:
> a/arch/x86/kvm/hyperv.c
> - if (__clear_user((void __user *)addr, sizeof(u32)))
> + if (__put_user(0, (u32 __user *)addr))
I'm not doubting that this is a correct transformation and an
improvement, but why is it using that double-underscore version in the
first place?
There's a __copy_to_user() in kvm_hv_set_msr_pw() in addition to this
one in kvm_hv_set_msr(). Both go back to 2011 and commit 8b0cedff040b
("KVM: use __copy_to_user/__clear_user to write guest page") and both
look purely like "pointlessly avoid the access_ok".
All these KVM "optimizations" seem entirely pointless, since
access_ok() isn't the problem. And the address _claims_ to be
verified, but I'm not seeing it. There is not a single 'access_ok()'
anywhere in arch/x86/kvm/ that I can see.
It looks like the argument for the address being validated is that it
comes from "gfn_to_hva()", which should only return
host-virtual-addresses. That may be true.
But "should" is not "does", and honestly, the cost of gfn_to_hva() is
high enough that then using that as an argument for removing
"access_ok()" smells.
So I would suggest just removing all these completely bogus
double-underscore versions. It's pointless, it's wrong, and it's
unsafe.
This isn't even some critical path, but even if it was, that user
address check isn't where the problems are.
Linus
On Fri, May 29, 2020 at 4:26 PM Al Viro <[email protected]> wrote:
>
> The stuff that doesn't fit anywhere else. Hopefully
> saner marshalling for weird 7-argument syscalls (pselect6()),
That looked fine to me, btw. Looks like an improvement even outside
the "avoid __get_user()" and double STAC/CLAC issue.
> low-hanging fruit in several binfmt, unsafe_put_user-based
> x86 cp_stat64(), etc. - there's really no common topic here.
My only complaint was that kvm thing that I think should have gone even further.
Linus
On Fri, May 29, 2020 at 4:54 PM Linus Torvalds
<[email protected]> wrote:
>
> My only complaint was that kvm thing that I think should have gone even further.
Oh... And the cc list looks a bit odd.
You cc'd fsdevel, but none of the patches were really to any
filesystem code (ok, the pselect and binfmt thing is in fs/, but
that's kind of incidental and more of a "we split core system calls up
by area too" than any "it's filesystem code").
The kvm patch didn't have anybody from kvm-land cc'd, for example. I
added some to my "this shouldn't use double underscores" reply, but..
Linus
On Fri, May 29, 2020 at 04:52:59PM -0700, Linus Torvalds wrote:
> On Fri, May 29, 2020 at 4:27 PM Al Viro <[email protected]> wrote:
> > a/arch/x86/kvm/hyperv.c
> > - if (__clear_user((void __user *)addr, sizeof(u32)))
> > + if (__put_user(0, (u32 __user *)addr))
>
> I'm not doubting that this is a correct transformation and an
> improvement, but why is it using that double-underscore version in the
> first place?
>
> There's a __copy_to_user() in kvm_hv_set_msr_pw() in addition to this
> one in kvm_hv_set_msr(). Both go back to 2011 and commit 8b0cedff040b
> ("KVM: use __copy_to_user/__clear_user to write guest page") and both
> look purely like "pointlessly avoid the access_ok".
>
> All these KVM "optimizations" seem entirely pointless, since
> access_ok() isn't the problem. And the address _claims_ to be
> verified, but I'm not seeing it. There is not a single 'access_ok()'
> anywhere in arch/x86/kvm/ that I can see.
>
> It looks like the argument for the address being validated is that it
> comes from "gfn_to_hva()", which should only return
> host-virtual-addresses. That may be true.
>
> But "should" is not "does", and honestly, the cost of gfn_to_hva() is
> high enough that then using that as an argument for removing
> "access_ok()" smells.
>
> So I would suggest just removing all these completely bogus
> double-underscore versions. It's pointless, it's wrong, and it's
> unsafe.
It's a bit trickier than that, but I want to deal with that at the same
time as the rest of kvm/vhost stuff. So for this series I just went
for minimal change. There's quite a pile of vhost and kvm stuff,
but it's not ready yet - wait for the next cycle.
On Sat, May 30, 2020 at 03:31:47PM +0100, Al Viro wrote:
> It's a bit trickier than that, but I want to deal with that at the same
> time as the rest of kvm/vhost stuff. So for this series I just went
> for minimal change. There's quite a pile of vhost and kvm stuff,
> but it's not ready yet - wait for the next cycle.
BTW, regarding uaccess plans for the next cycle:
* regset mess (at least the ->get() side)
* killing more compat_alloc_user_space() call sites (_maybe_
all of it, if we are lucky enough; v4l2 is a bitch in that respect,
but I've some ideas on how to deal with that - need to discuss with
mchehab)
* sorting the remaining (harder) parts of i915 out
* kvm/vhost
* fault_in_pages_...() series
That should get rid of almost all __... ones outside of arch/*; might
actually kill copy_in_user() off as well.
* finally lifting stac/clac out of raw_copy_{to,from}_user().
> On Fri, May 29, 2020 at 04:52:59PM -0700, Linus Torvalds wrote:
>> It looks like the argument for the address being validated is that it
>> comes from "gfn_to_hva()", which should only return
>> host-virtual-addresses. That may be true.
Yes, the access_ok is done in __kvm_set_memory_region and gfn_to_hva()
returns a page-aligned address so it's obviously ok for a u32.
But I have no objections to removing the __ because if a read or write
is in the hot path it will use kvm_write_guest_cached and similar.
Paolo
>> But "should" is not "does", and honestly, the cost of gfn_to_hva() is
>> high enough that then using that as an argument for removing
>> "access_ok()" smells.
On Sat, May 30, 2020 at 9:20 AM Paolo Bonzini <[email protected]> wrote:
>
> Yes, the access_ok is done in __kvm_set_memory_region and gfn_to_hva()
> returns a page-aligned address so it's obviously ok for a u32.
It's not that it's "obviously ok for an u32".
It is _not_ obviously ok for a user address. There's actually no
access_ok() done in the lookup path at all, and what gfn_to_hva()
actually ends up doing in the end is __gfn_to_hva_memslot(), which has
zero overflow protection at all, and just does
slot->userspace_addr + (gfn - slot->base_gfn) * PAGE_SIZE;
without us having _ever_ checked that 'gfn' parameter.
Yes, at some point in the very very distant past,
__kvm_set_memory_region() has validated
mem->{userspace_addr,memory_size}. But even that validation is
actually very questionable, since it's not even done for all of the
memory slots, only the "user" ones.
So if at any point we have a non-user slot, of it at any point the gfn
thing was mis-calculated and {over,under}flows, there are no
protections what-so-ever.
In other words, it really looks like kvm is entirely dependent on
magic and luck and a desperate hope that there are no other bugs to
keep the end result as a user address.
Because if _any_ bug or oversight in that kvm_memory_slot handling
ever happens, you end up with random crap.
So no. I disagree. There is absolutely nothing "obviously ok" about
any of that kvm code. Quite the reverse.
I'd argue that it's very much obviously *NOT* ok, even while it might
just happen to work.
That double underscore needs to go away. It's either actively buggy
right now and I see no proof it isn't, or it's a bug just waiting to
happen in the future.
Linus
On Sat, May 30, 2020 at 10:57:24AM -0700, Linus Torvalds wrote:
> So no. I disagree. There is absolutely nothing "obviously ok" about
> any of that kvm code. Quite the reverse.
>
> I'd argue that it's very much obviously *NOT* ok, even while it might
> just happen to work.
Actually, it's somewhat less brittle than you think (on non-mips, at least)
and not due to those long-ago access_ok().
> That double underscore needs to go away. It's either actively buggy
> right now and I see no proof it isn't, or it's a bug just waiting to
> happen in the future.
FWIW, the kvm side of things (vhost is yet another pile of fun) is
[x86] kvm_hv_set_msr_pw():
arch/x86/kvm/hyperv.c:1027: if (__copy_to_user((void __user *)addr, instructions, 4))
HV_X64_MSR_HYPERCALL
arch/x86/kvm/hyperv.c:1132: if (__clear_user((void __user *)addr, sizeof(u32)))
HV_X64_MSR_VP_ASSIST_PAGE
in both cases addr comes from
gfn = data >> HV_X64_MSR_VP_ASSIST_PAGE_ADDRESS_SHIFT;
addr = kvm_vcpu_gfn_to_hva(vcpu, gfn);
if (kvm_is_error_hva(addr))
return 1;
[x86] FNAME(walk_addr_generic), very hot:
arch/x86/kvm/mmu/paging_tmpl.h:403: if (unlikely(__get_user(pte, ptep_user)))
index = PT_INDEX(addr, walker->level);
...
offset = index * sizeof(pt_element_t);
...
host_addr = kvm_vcpu_gfn_to_hva_prot(vcpu, real_gfn,
&walker->pte_writable[walker->level - 1]);
if (unlikely(kvm_is_error_hva(host_addr)))
goto error;
ptep_user = (pt_element_t __user *)((void *)host_addr + offset);
__kvm_read_guest_page():
virt/kvm/kvm_main.c:2252: r = __copy_from_user(data, (void __user *)addr + offset, len);
addr = gfn_to_hva_memslot_prot(slot, gfn, NULL);
if (kvm_is_error_hva(addr))
return -EFAULT;
__kvm_read_guest_atomic():
virt/kvm/kvm_main.c:2326: r = __copy_from_user_inatomic(data, (void __user *)addr + offset, len);
addr = gfn_to_hva_memslot_prot(slot, gfn, NULL);
if (kvm_is_error_hva(addr))
return -EFAULT;
__kvm_write_guest_page():
virt/kvm/kvm_main.c:2353: r = __copy_to_user((void __user *)addr + offset, data, len);
addr = gfn_to_hva_memslot(memslot, gfn);
if (kvm_is_error_hva(addr))
return -EFAULT;
kvm_write_guest_offset_cached():
virt/kvm/kvm_main.c:2490: r = __copy_to_user((void __user *)ghc->hva + offset, data, len);
if (kvm_is_error_hva(ghc->hva))
return -EFAULT;
kvm_read_guest_cached():
virt/kvm/kvm_main.c:2525: r = __copy_from_user(data, (void __user *)ghc->hva, len);
if (kvm_is_error_hva(ghc->hva))
return -EFAULT;
default kvm_is_error_hva() is addr >= PAGE_OFFSET; however, on mips and s390 it's
IS_ERR_VALUE().
Sure, we can use non-__ variants, but is access_ok() the right primitive here?
We want userland memory, set_fs() be damned.
On Sat, May 30, 2020 at 11:39 AM Al Viro <[email protected]> wrote:
>
> Actually, it's somewhat less brittle than you think (on non-mips, at least)
> and not due to those long-ago access_ok().
It really isn't.
Your very first statement shows how broken it is:
> FWIW, the kvm side of things (vhost is yet another pile of fun) is
>
> [x86] kvm_hv_set_msr_pw():
> arch/x86/kvm/hyperv.c:1027: if (__copy_to_user((void __user *)addr, instructions, 4))
> HV_X64_MSR_HYPERCALL
> arch/x86/kvm/hyperv.c:1132: if (__clear_user((void __user *)addr, sizeof(u32)))
> HV_X64_MSR_VP_ASSIST_PAGE
> in both cases addr comes from
> gfn = data >> HV_X64_MSR_VP_ASSIST_PAGE_ADDRESS_SHIFT;
> addr = kvm_vcpu_gfn_to_hva(vcpu, gfn);
> if (kvm_is_error_hva(addr))
> return 1;
Just look at that. You have _zero_ indication that 'adds" is a user
space address. It could be a kernel address.
That kvm_vcpu_gfn_to_hva() function is a complicated mess that first
looks for the right 'memslot', and basically uses a search with a
default slot to try to figure it out. It doesn't even use locking for
any of it, but assumes the arrays are stable, and that it can use
atomics to reliably read and set the last successfully found slot.
And none of that code verifies that the end result is a user address.
It _literally_ all depends on this optimistically lock-free code being
bug-free, and never using a slot that isn't a user slot. And as
mentioned, there _are_ non-user memslots.
It's fragile as hell.
And it's all completely and utterly pointless. ALL of the above is
incredibly much more expensive than just checking the damn address
range.
So the optimization is completely bogus to begin with, and all it
results in is that any bug in this _incredibly_ subtle code will be a
security proiblem.
And I don't understand why you mention set_fs() vs access_ok(). None
of this code has anything that messes with set_fs(). The access_ok()
is garbage and shouldn't exist, and those user accesses should all use
the checking versions and the double underscores are wrong.
I have no idea why you think the double underscores could _possibly_
be worth defending.
Linus
On Sat, May 30, 2020 at 11:52:44AM -0700, Linus Torvalds wrote:
>
> It really isn't.
>
> Your very first statement shows how broken it is:
>
> > FWIW, the kvm side of things (vhost is yet another pile of fun) is
> >
> > [x86] kvm_hv_set_msr_pw():
> > arch/x86/kvm/hyperv.c:1027: if (__copy_to_user((void __user *)addr, instructions, 4))
> > HV_X64_MSR_HYPERCALL
> > arch/x86/kvm/hyperv.c:1132: if (__clear_user((void __user *)addr, sizeof(u32)))
> > HV_X64_MSR_VP_ASSIST_PAGE
> > in both cases addr comes from
> > gfn = data >> HV_X64_MSR_VP_ASSIST_PAGE_ADDRESS_SHIFT;
> > addr = kvm_vcpu_gfn_to_hva(vcpu, gfn);
> > if (kvm_is_error_hva(addr))
> > return 1;
>
> Just look at that. You have _zero_ indication that 'adds" is a user
> space address. It could be a kernel address.
>
> That kvm_vcpu_gfn_to_hva() function is a complicated mess that first
> looks for the right 'memslot', and basically uses a search with a
> default slot to try to figure it out. It doesn't even use locking for
> any of it, but assumes the arrays are stable, and that it can use
> atomics to reliably read and set the last successfully found slot.
>
> And none of that code verifies that the end result is a user address.
kvm_is_error_hva() is
return addr >= PAGE_OFFSET;
On Sat, May 30, 2020 at 11:52:44AM -0700, Linus Torvalds wrote:
> And I don't understand why you mention set_fs() vs access_ok(). None
> of this code has anything that messes with set_fs(). The access_ok()
> is garbage and shouldn't exist, and those user accesses should all use
> the checking versions and the double underscores are wrong.
>
> I have no idea why you think the double underscores could _possibly_
> be worth defending.
I do not. What I'm saying is that this just might be a beast different
from *both* __... and the normal ones. I'm not saying that this
__put_user() (or __clear_user(), etc.) is the right primitive here.
If anything, it's closer to the situation for (x86) copy_stack_trace().
On Sat, May 30, 2020 at 12:14 PM Al Viro <[email protected]> wrote:
>
> > And none of that code verifies that the end result is a user address.
>
> kvm_is_error_hva() is
> return addr >= PAGE_OFFSET;
Ahh, that's what I missed. It won't work on other architectures, but
within x86 it's fine.
Linus
On Sat, May 30, 2020 at 08:19:40PM +0100, Al Viro wrote:
> On Sat, May 30, 2020 at 11:52:44AM -0700, Linus Torvalds wrote:
>
> > And I don't understand why you mention set_fs() vs access_ok(). None
> > of this code has anything that messes with set_fs(). The access_ok()
> > is garbage and shouldn't exist, and those user accesses should all use
> > the checking versions and the double underscores are wrong.
> >
> > I have no idea why you think the double underscores could _possibly_
> > be worth defending.
>
> I do not. What I'm saying is that this just might be a beast different
> from *both* __... and the normal ones. I'm not saying that this
> __put_user() (or __clear_user(), etc.) is the right primitive here.
> If anything, it's closer to the situation for (x86) copy_stack_trace().
... and no, I'm not saying that copy_stack_trace() should stay with
__get_user() either. It feels like we are lacking primitives needed
to express that cleanly and copy_stack_trace() currently cobbles something
up out of what we have. Which works for arch-specific code, but yes,
that kind of thing is brittle for arch-independent places like virt/kvm;
I wonder if e.g. s390 is really OK there.
On Sat, May 30, 2020 at 12:20:54PM -0700, Linus Torvalds wrote:
> On Sat, May 30, 2020 at 12:14 PM Al Viro <[email protected]> wrote:
> >
> > > And none of that code verifies that the end result is a user address.
> >
> > kvm_is_error_hva() is
> > return addr >= PAGE_OFFSET;
>
> Ahh, that's what I missed. It won't work on other architectures, but
> within x86 it's fine.
FWIW, we use virt/kvm on x86, powerpc, mips, s390 and arm64.
For x86 and powerpc the check is, AFAICS, OK (ppc kernel might start
higher than PAGE_OFFSET, but not lower than it). For arm64... not
sure - I'm not familiar with the virtual address space layout we use
there. mips does *NOT* get that protection at all - there kvm_is_error_hva()
is IS_ERR_VALUE() (thus the "at least on non-mips" upthread). And
for s390 it's also IS_ERR_VALUE(), but that's an separate can of worms -
there access_ok() is constant true; if we ever hit any of that code in
virt/kvm while under KERNEL_DS, we are well and truly fucked there.
On Sat, May 30, 2020 at 08:42:32PM +0100, Al Viro wrote:
> On Sat, May 30, 2020 at 12:20:54PM -0700, Linus Torvalds wrote:
> > On Sat, May 30, 2020 at 12:14 PM Al Viro <[email protected]> wrote:
> > >
> > > > And none of that code verifies that the end result is a user address.
> > >
> > > kvm_is_error_hva() is
> > > return addr >= PAGE_OFFSET;
> >
> > Ahh, that's what I missed. It won't work on other architectures, but
> > within x86 it's fine.
>
> FWIW, we use virt/kvm on x86, powerpc, mips, s390 and arm64.
>
> For x86 and powerpc the check is, AFAICS, OK (ppc kernel might start
> higher than PAGE_OFFSET, but not lower than it). For arm64... not
> sure - I'm not familiar with the virtual address space layout we use
> there. mips does *NOT* get that protection at all - there kvm_is_error_hva()
> is IS_ERR_VALUE() (thus the "at least on non-mips" upthread). And
> for s390 it's also IS_ERR_VALUE(), but that's an separate can of worms -
> there access_ok() is constant true; if we ever hit any of that code in
> virt/kvm while under KERNEL_DS, we are well and truly fucked there.
Anyway, I really think it's too big to handle this cycle, what with the
amount of other stuff already in queue. If anything, that __put_user()
is a useful marker of the things that will need attention. That's arch/x86
and the test excluding the kernel space is just upstream of that call,
so IMO that's robust enough for now. Crossing the limit just into the
beginning of kernel space is theoretically possible, but that would
depend upon slot->userspace_addr not being page-aligned (and would attempt
to zero up to 3 bytes past the PAGE_OFFSET in any case). If we get
memory corruption going on, we have much worse problems than that.
And it would have to be memory corruption - ->userspace_addr is assign-once,
there's only one place doing the assignments and alignment check is
shortly upstream of it, so all instances must have that field page-aligned
all the time.
We'll need to sort the kvm-related issues out, but let's leave it for the
next cycle.
On Sat, May 30, 2020 at 12:28:14AM +0100, Al Viro wrote:
> From: Al Viro <[email protected]>
>
> ... rather than open-coding it, and badly, at that.
>
> Signed-off-by: Al Viro <[email protected]>
> ---
> kernel/bpf/syscall.c | 25 ++++++-------------------
> 1 file changed, 6 insertions(+), 19 deletions(-)
lgtm
Acked-by: Alexei Starovoitov <[email protected]>
> hpsa compat ioctl done (hopefully) saner. I really want
> to kill compat_alloc_user_space() off - it's always trouble and
> for a driver-private ioctls it's absolutely pointless.
>
> Note that this is only compile-tested - I don't have the
> hardware to test it on *or* userland to issue the ioctls in
> question. So this series definitely needs a review and testing
> from hpsa maintainers before it might go anywhere.
Don: Please test and review. Thanks!
--
Martin K. Petersen Oracle Linux Engineering
-----Original Message-----
From: [email protected] [mailto:[email protected]] On Behalf Of Al Viro
Sent: Friday, May 29, 2020 6:39 PM
To: Linus Torvalds <[email protected]>
Cc: [email protected]; [email protected]; Don Brace <[email protected]>; [email protected]
Subject: [PATCHES] uaccess hpsa
hpsa compat ioctl done (hopefully) saner. I really want to kill compat_alloc_user_space() off - it's always trouble and for a driver-private ioctls it's absolutely pointless.
The series is in vfs.git #uaccess.hpsa, based at v5.7-rc1
Al Viro (4):
hpsa passthrough: lift {BIG_,}IOCTL_Command_struct copy{in,out} into hpsa_ioctl()
hpsa: don't bother with vmalloc for BIG_IOCTL_Command_struct
hpsa: get rid of compat_alloc_user_space()
hpsa_ioctl(): tidy up a bit
Acked-by: Don Brace <[email protected]>
Tested-by: Don Brace <[email protected]>
drivers/scsi/hpsa.c | 199 ++++++++++++++++++++++++----------------------------
1 file changed, 90 insertions(+), 109 deletions(-)
On Wed, Jun 03, 2020 at 06:37:11PM +0000, [email protected] wrote:
> -----Original Message-----
> From: [email protected] [mailto:[email protected]] On Behalf Of Al Viro
> Sent: Friday, May 29, 2020 6:39 PM
> To: Linus Torvalds <[email protected]>
> Cc: [email protected]; [email protected]; Don Brace <[email protected]>; [email protected]
> Subject: [PATCHES] uaccess hpsa
>
> hpsa compat ioctl done (hopefully) saner. I really want to kill compat_alloc_user_space() off - it's always trouble and for a driver-private ioctls it's absolutely pointless.
>
> The series is in vfs.git #uaccess.hpsa, based at v5.7-rc1
>
> Al Viro (4):
> hpsa passthrough: lift {BIG_,}IOCTL_Command_struct copy{in,out} into hpsa_ioctl()
> hpsa: don't bother with vmalloc for BIG_IOCTL_Command_struct
> hpsa: get rid of compat_alloc_user_space()
> hpsa_ioctl(): tidy up a bit
>
> Acked-by: Don Brace <[email protected]>
> Tested-by: Don Brace <[email protected]>
OK... Acked-by/Tested-by added, branch re-pushed (commits are otherwise
identical). Which tree would you prefer that to go through - vfs.git,
scsi.git, something else?
On Wed, Jun 03, 2020 at 04:53:11PM -0400, Martin K. Petersen wrote:
>
> Hi Al!
>
> > OK... Acked-by/Tested-by added, branch re-pushed (commits are otherwise
> > identical). Which tree would you prefer that to go through - vfs.git,
> > scsi.git, something else?
>
> I don't have anything queued for 5.8 for hpsa so there shouldn't be any
> conflicts if it goes through vfs.git. But I'm perfectly happy to take
> the changes through SCSI if that's your preference.
Up to you; if you need a pull request, just say so.
Hi Al!
> OK... Acked-by/Tested-by added, branch re-pushed (commits are otherwise
> identical). Which tree would you prefer that to go through - vfs.git,
> scsi.git, something else?
I don't have anything queued for 5.8 for hpsa so there shouldn't be any
conflicts if it goes through vfs.git. But I'm perfectly happy to take
the changes through SCSI if that's your preference.
--
Martin K. Petersen Oracle Linux Engineering
Al,
>> I don't have anything queued for 5.8 for hpsa so there shouldn't be any
>> conflicts if it goes through vfs.git. But I'm perfectly happy to take
>> the changes through SCSI if that's your preference.
>
> Up to you; if you need a pull request, just say so.
OK, I queued these up for 5.8.
Thanks!
--
Martin K. Petersen Oracle Linux Engineering