linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCHES] uaccess simple access_ok() removals
@ 2020-05-09 23:41 Al Viro
  2020-05-09 23:45 ` [PATCH 01/20] dlmfs_file_write(): get rid of pointless access_ok() Al Viro
                   ` (2 more replies)
  0 siblings, 3 replies; 42+ messages in thread
From: Al Viro @ 2020-05-09 23:41 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, linux-fsdevel

	One of the uaccess-related branches; this one is just the
cases when access_ok() calls are trivially pointless - the address
in question gets fed only to primitives that do access_ok() checks
themselves.  This stuff sits in
git://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git #uaccess.access_ok
and it's on top of #fixes (which is already merged into mainline on
Apr 28).
	Individual patches in followups; if nobody screams - into #for-next
it goes...

	There are other uaccess-related branches; I'll be posting them
for review over the next few days.  My apologies if this one comes with
a buggered Cc - this is the first time I have to deal with a series with
Cc lists varying that much; I hope I manage to get git-send-email do
the right thing, but...

Shortlog:
Al Viro (20):
      dlmfs_file_write(): get rid of pointless access_ok()
      fat_dir_ioctl(): hadn't needed that access_ok() for more than a decade...
      btrfs_ioctl_send(): don't bother with access_ok()
      FIEMAP: don't bother with access_ok()
      tomoyo_write_control(): get rid of pointless access_ok()
      n_hdlc_tty_read(): remove pointless access_ok()
      nvram: drop useless access_ok()
      cm4000_cs.c cmm_ioctl(): get rid of pointless access_ok()
      drivers/fpga/dfl-fme-pr.c: get rid of pointless access_ok()
      drivers/fpga/dfl-afu-dma-region.c: get rid of pointless access_ok()
      amifb: get rid of pointless access_ok() calls
      omapfb: get rid of pointless access_ok() calls
      drivers/crypto/ccp/sev-dev.c: get rid of pointless access_ok()
      via-pmu: don't bother with access_ok()
      drm_read(): get rid of pointless access_ok()
      efi_test: get rid of pointless access_ok()
      lpfc_debugfs: get rid of pointless access_ok()
      usb: get rid of pointless access_ok() calls
      hfi1: get rid of pointless access_ok()
      vmci_host: get rid of pointless access_ok()
Diffstat:
 drivers/char/nvram.c                            |  4 ----
 drivers/char/pcmcia/cm4000_cs.c                 | 14 --------------
 drivers/crypto/ccp/sev-dev.c                    | 15 +++------------
 drivers/firmware/efi/test/efi_test.c            | 12 ------------
 drivers/fpga/dfl-afu-dma-region.c               |  4 ----
 drivers/fpga/dfl-fme-pr.c                       |  4 ----
 drivers/gpu/drm/drm_file.c                      |  3 ---
 drivers/infiniband/hw/hfi1/user_exp_rcv.c       |  7 -------
 drivers/macintosh/via-pmu.c                     |  2 --
 drivers/misc/vmw_vmci/vmci_host.c               |  2 --
 drivers/scsi/lpfc/lpfc_debugfs.c                | 12 ------------
 drivers/tty/n_hdlc.c                            |  7 -------
 drivers/usb/core/devices.c                      |  2 --
 drivers/usb/core/devio.c                        |  9 ---------
 drivers/usb/gadget/function/f_hid.c             |  6 ------
 drivers/video/fbdev/amifb.c                     |  4 ----
 drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c |  3 ---
 fs/btrfs/send.c                                 |  7 -------
 fs/ext4/ioctl.c                                 |  5 -----
 fs/fat/dir.c                                    |  4 ----
 fs/ioctl.c                                      |  5 -----
 fs/ocfs2/dlmfs/dlmfs.c                          |  3 ---
 security/tomoyo/common.c                        |  2 --
 23 files changed, 3 insertions(+), 133 deletions(-)


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 01/20] dlmfs_file_write(): get rid of pointless access_ok()
  2020-05-09 23:41 [PATCHES] uaccess simple access_ok() removals Al Viro
@ 2020-05-09 23:45 ` Al Viro
  2020-05-09 23:45   ` [PATCH 02/20] fat_dir_ioctl(): hadn't needed that access_ok() for more than a decade Al Viro
                     ` (18 more replies)
  2020-05-10  0:34 ` [PATCHES] uaccess simple access_ok() removals Linus Torvalds
  2020-05-10 14:34 ` David Laight
  2 siblings, 19 replies; 42+ messages in thread
From: Al Viro @ 2020-05-09 23:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, linux-fsdevel

From: Al Viro <viro@zeniv.linux.org.uk>

address passed only to copy_from_user()

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/ocfs2/dlmfs/dlmfs.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/fs/ocfs2/dlmfs/dlmfs.c b/fs/ocfs2/dlmfs/dlmfs.c
index 1de77f1a600b..a06f19b67d3b 100644
--- a/fs/ocfs2/dlmfs/dlmfs.c
+++ b/fs/ocfs2/dlmfs/dlmfs.c
@@ -291,9 +291,6 @@ static ssize_t dlmfs_file_write(struct file *filp,
 	if (!count)
 		return 0;
 
-	if (!access_ok(buf, count))
-		return -EFAULT;
-
 	lvb_buf = kmalloc(count, GFP_NOFS);
 	if (!lvb_buf)
 		return -ENOMEM;
-- 
2.11.0


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 02/20] fat_dir_ioctl(): hadn't needed that access_ok() for more than a decade...
  2020-05-09 23:45 ` [PATCH 01/20] dlmfs_file_write(): get rid of pointless access_ok() Al Viro
@ 2020-05-09 23:45   ` Al Viro
  2020-05-09 23:45   ` [PATCH 03/20] btrfs_ioctl_send(): don't bother with access_ok() Al Viro
                     ` (17 subsequent siblings)
  18 siblings, 0 replies; 42+ messages in thread
From: Al Viro @ 2020-05-09 23:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, linux-fsdevel

From: Al Viro <viro@zeniv.linux.org.uk>

address is passed only to put_user() and copy_to_user()

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/fat/dir.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/fs/fat/dir.c b/fs/fat/dir.c
index 054acd9fd033..b4ddf48fa444 100644
--- a/fs/fat/dir.c
+++ b/fs/fat/dir.c
@@ -804,8 +804,6 @@ static long fat_dir_ioctl(struct file *filp, unsigned int cmd,
 		return fat_generic_ioctl(filp, cmd, arg);
 	}
 
-	if (!access_ok(d1, sizeof(struct __fat_dirent[2])))
-		return -EFAULT;
 	/*
 	 * Yes, we don't need this put_user() absolutely. However old
 	 * code didn't return the right value. So, app use this value,
@@ -844,8 +842,6 @@ static long fat_compat_dir_ioctl(struct file *filp, unsigned cmd,
 		return fat_generic_ioctl(filp, cmd, (unsigned long)arg);
 	}
 
-	if (!access_ok(d1, sizeof(struct compat_dirent[2])))
-		return -EFAULT;
 	/*
 	 * Yes, we don't need this put_user() absolutely. However old
 	 * code didn't return the right value. So, app use this value,
-- 
2.11.0


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 03/20] btrfs_ioctl_send(): don't bother with access_ok()
  2020-05-09 23:45 ` [PATCH 01/20] dlmfs_file_write(): get rid of pointless access_ok() Al Viro
  2020-05-09 23:45   ` [PATCH 02/20] fat_dir_ioctl(): hadn't needed that access_ok() for more than a decade Al Viro
@ 2020-05-09 23:45   ` Al Viro
  2020-05-09 23:45   ` [PATCH 04/20] FIEMAP: " Al Viro
                     ` (16 subsequent siblings)
  18 siblings, 0 replies; 42+ messages in thread
From: Al Viro @ 2020-05-09 23:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, linux-fsdevel

From: Al Viro <viro@zeniv.linux.org.uk>

we do copy_from_user() on that range anyway

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/btrfs/send.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/fs/btrfs/send.c b/fs/btrfs/send.c
index c5f41bd86765..6a92ecf9eaa2 100644
--- a/fs/btrfs/send.c
+++ b/fs/btrfs/send.c
@@ -7065,13 +7065,6 @@ long btrfs_ioctl_send(struct file *mnt_file, struct btrfs_ioctl_send_args *arg)
 		goto out;
 	}
 
-	if (!access_ok(arg->clone_sources,
-			sizeof(*arg->clone_sources) *
-			arg->clone_sources_count)) {
-		ret = -EFAULT;
-		goto out;
-	}
-
 	if (arg->flags & ~BTRFS_SEND_FLAG_MASK) {
 		ret = -EINVAL;
 		goto out;
-- 
2.11.0


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 04/20] FIEMAP: don't bother with access_ok()
  2020-05-09 23:45 ` [PATCH 01/20] dlmfs_file_write(): get rid of pointless access_ok() Al Viro
  2020-05-09 23:45   ` [PATCH 02/20] fat_dir_ioctl(): hadn't needed that access_ok() for more than a decade Al Viro
  2020-05-09 23:45   ` [PATCH 03/20] btrfs_ioctl_send(): don't bother with access_ok() Al Viro
@ 2020-05-09 23:45   ` Al Viro
  2020-05-10  7:02     ` Christoph Hellwig
  2020-05-09 23:45   ` [PATCH 05/20] tomoyo_write_control(): get rid of pointless access_ok() Al Viro
                     ` (15 subsequent siblings)
  18 siblings, 1 reply; 42+ messages in thread
From: Al Viro @ 2020-05-09 23:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, linux-fsdevel

From: Al Viro <viro@zeniv.linux.org.uk>

we use copy_to_user() on that thing anyway (and always had).

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/ext4/ioctl.c | 5 -----
 fs/ioctl.c      | 5 -----
 2 files changed, 10 deletions(-)

diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index bfc1281fc4cb..a0afd0338722 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -784,11 +784,6 @@ static int ext4_ioctl_get_es_cache(struct file *filp, unsigned long arg)
 	fieinfo.fi_extents_max = fiemap.fm_extent_count;
 	fieinfo.fi_extents_start = ufiemap->fm_extents;
 
-	if (fiemap.fm_extent_count != 0 &&
-	    !access_ok(fieinfo.fi_extents_start,
-		       fieinfo.fi_extents_max * sizeof(struct fiemap_extent)))
-		return -EFAULT;
-
 	if (fieinfo.fi_flags & FIEMAP_FLAG_SYNC)
 		filemap_write_and_wait(inode->i_mapping);
 
diff --git a/fs/ioctl.c b/fs/ioctl.c
index 282d45be6f45..f8181e2c2168 100644
--- a/fs/ioctl.c
+++ b/fs/ioctl.c
@@ -215,11 +215,6 @@ static int ioctl_fiemap(struct file *filp, struct fiemap __user *ufiemap)
 	fieinfo.fi_extents_max = fiemap.fm_extent_count;
 	fieinfo.fi_extents_start = ufiemap->fm_extents;
 
-	if (fiemap.fm_extent_count != 0 &&
-	    !access_ok(fieinfo.fi_extents_start,
-		       fieinfo.fi_extents_max * sizeof(struct fiemap_extent)))
-		return -EFAULT;
-
 	if (fieinfo.fi_flags & FIEMAP_FLAG_SYNC)
 		filemap_write_and_wait(inode->i_mapping);
 
-- 
2.11.0


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 05/20] tomoyo_write_control(): get rid of pointless access_ok()
  2020-05-09 23:45 ` [PATCH 01/20] dlmfs_file_write(): get rid of pointless access_ok() Al Viro
                     ` (2 preceding siblings ...)
  2020-05-09 23:45   ` [PATCH 04/20] FIEMAP: " Al Viro
@ 2020-05-09 23:45   ` Al Viro
  2020-05-10  0:50     ` Tetsuo Handa
  2020-05-09 23:45   ` [PATCH 06/20] n_hdlc_tty_read(): remove " Al Viro
                     ` (14 subsequent siblings)
  18 siblings, 1 reply; 42+ messages in thread
From: Al Viro @ 2020-05-09 23:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, linux-fsdevel, Tetsuo Handa

From: Al Viro <viro@zeniv.linux.org.uk>

address is passed only to get_user()

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 security/tomoyo/common.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/security/tomoyo/common.c b/security/tomoyo/common.c
index 1b467381986f..f93f8acd05f7 100644
--- a/security/tomoyo/common.c
+++ b/security/tomoyo/common.c
@@ -2662,8 +2662,6 @@ ssize_t tomoyo_write_control(struct tomoyo_io_buffer *head,
 
 	if (!head->write)
 		return -EINVAL;
-	if (!access_ok(buffer, buffer_len))
-		return -EFAULT;
 	if (mutex_lock_interruptible(&head->io_sem))
 		return -EINTR;
 	head->read_user_buf_avail = 0;
-- 
2.11.0


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 06/20] n_hdlc_tty_read(): remove pointless access_ok()
  2020-05-09 23:45 ` [PATCH 01/20] dlmfs_file_write(): get rid of pointless access_ok() Al Viro
                     ` (3 preceding siblings ...)
  2020-05-09 23:45   ` [PATCH 05/20] tomoyo_write_control(): get rid of pointless access_ok() Al Viro
@ 2020-05-09 23:45   ` Al Viro
  2020-05-15 10:53     ` Greg Kroah-Hartman
  2020-05-09 23:45   ` [PATCH 07/20] nvram: drop useless access_ok() Al Viro
                     ` (13 subsequent siblings)
  18 siblings, 1 reply; 42+ messages in thread
From: Al Viro @ 2020-05-09 23:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, linux-fsdevel, Greg Kroah-Hartman

From: Al Viro <viro@zeniv.linux.org.uk>

only copy_to_user() is done to the address in question

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/tty/n_hdlc.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/tty/n_hdlc.c b/drivers/tty/n_hdlc.c
index 991f49ee4026..b09eac4b6d64 100644
--- a/drivers/tty/n_hdlc.c
+++ b/drivers/tty/n_hdlc.c
@@ -423,13 +423,6 @@ static ssize_t n_hdlc_tty_read(struct tty_struct *tty, struct file *file,
 	struct n_hdlc_buf *rbuf;
 	DECLARE_WAITQUEUE(wait, current);
 
-	/* verify user access to buffer */
-	if (!access_ok(buf, nr)) {
-		pr_warn("%s(%d) %s() can't verify user buffer\n",
-				__FILE__, __LINE__, __func__);
-		return -EFAULT;
-	}
-
 	add_wait_queue(&tty->read_wait, &wait);
 
 	for (;;) {
-- 
2.11.0


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 07/20] nvram: drop useless access_ok()
  2020-05-09 23:45 ` [PATCH 01/20] dlmfs_file_write(): get rid of pointless access_ok() Al Viro
                     ` (4 preceding siblings ...)
  2020-05-09 23:45   ` [PATCH 06/20] n_hdlc_tty_read(): remove " Al Viro
@ 2020-05-09 23:45   ` Al Viro
  2020-05-15 10:54     ` Greg Kroah-Hartman
  2020-05-09 23:45   ` [PATCH 08/20] cm4000_cs.c cmm_ioctl(): get rid of pointless access_ok() Al Viro
                     ` (12 subsequent siblings)
  18 siblings, 1 reply; 42+ messages in thread
From: Al Viro @ 2020-05-09 23:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, linux-fsdevel, Greg Kroah-Hartman

From: Al Viro <viro@zeniv.linux.org.uk>

we are using copy_to_user()/memdup_user() anyway

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/char/nvram.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/char/nvram.c b/drivers/char/nvram.c
index 4667844eee69..8206412d25ba 100644
--- a/drivers/char/nvram.c
+++ b/drivers/char/nvram.c
@@ -232,8 +232,6 @@ static ssize_t nvram_misc_read(struct file *file, char __user *buf,
 	ssize_t ret;
 
 
-	if (!access_ok(buf, count))
-		return -EFAULT;
 	if (*ppos >= nvram_size)
 		return 0;
 
@@ -264,8 +262,6 @@ static ssize_t nvram_misc_write(struct file *file, const char __user *buf,
 	char *tmp;
 	ssize_t ret;
 
-	if (!access_ok(buf, count))
-		return -EFAULT;
 	if (*ppos >= nvram_size)
 		return 0;
 
-- 
2.11.0


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 08/20] cm4000_cs.c cmm_ioctl(): get rid of pointless access_ok()
  2020-05-09 23:45 ` [PATCH 01/20] dlmfs_file_write(): get rid of pointless access_ok() Al Viro
                     ` (5 preceding siblings ...)
  2020-05-09 23:45   ` [PATCH 07/20] nvram: drop useless access_ok() Al Viro
@ 2020-05-09 23:45   ` Al Viro
  2020-05-09 23:45   ` [PATCH 09/20] drivers/fpga/dfl-fme-pr.c: " Al Viro
                     ` (11 subsequent siblings)
  18 siblings, 0 replies; 42+ messages in thread
From: Al Viro @ 2020-05-09 23:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, linux-fsdevel, Harald Welte

From: Al Viro <viro@zeniv.linux.org.uk>

copy_to_user()/copy_from_user() for everything

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/char/pcmcia/cm4000_cs.c | 14 --------------
 1 file changed, 14 deletions(-)

diff --git a/drivers/char/pcmcia/cm4000_cs.c b/drivers/char/pcmcia/cm4000_cs.c
index 4edb4174a1e2..89681f07bc78 100644
--- a/drivers/char/pcmcia/cm4000_cs.c
+++ b/drivers/char/pcmcia/cm4000_cs.c
@@ -1404,7 +1404,6 @@ static long cmm_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	unsigned int iobase = dev->p_dev->resource[0]->start;
 	struct inode *inode = file_inode(filp);
 	struct pcmcia_device *link;
-	int size;
 	int rc;
 	void __user *argp = (void __user *)arg;
 #ifdef CM4000_DEBUG
@@ -1441,19 +1440,6 @@ static long cmm_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		DEBUGP(4, dev, "iocnr mismatch\n");
 		goto out;
 	}
-	size = _IOC_SIZE(cmd);
-	rc = -EFAULT;
-	DEBUGP(4, dev, "iocdir=%.4x iocr=%.4x iocw=%.4x iocsize=%d cmd=%.4x\n",
-	      _IOC_DIR(cmd), _IOC_READ, _IOC_WRITE, size, cmd);
-
-	if (_IOC_DIR(cmd) & _IOC_READ) {
-		if (!access_ok(argp, size))
-			goto out;
-	}
-	if (_IOC_DIR(cmd) & _IOC_WRITE) {
-		if (!access_ok(argp, size))
-			goto out;
-	}
 	rc = 0;
 
 	switch (cmd) {
-- 
2.11.0


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 09/20] drivers/fpga/dfl-fme-pr.c: get rid of pointless access_ok()
  2020-05-09 23:45 ` [PATCH 01/20] dlmfs_file_write(): get rid of pointless access_ok() Al Viro
                     ` (6 preceding siblings ...)
  2020-05-09 23:45   ` [PATCH 08/20] cm4000_cs.c cmm_ioctl(): get rid of pointless access_ok() Al Viro
@ 2020-05-09 23:45   ` Al Viro
  2020-05-09 23:45   ` [PATCH 10/20] drivers/fpga/dfl-afu-dma-region.c: " Al Viro
                     ` (10 subsequent siblings)
  18 siblings, 0 replies; 42+ messages in thread
From: Al Viro @ 2020-05-09 23:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, linux-fsdevel, Wu Hao

From: Al Viro <viro@zeniv.linux.org.uk>

followed by copy_from_user()

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/fpga/dfl-fme-pr.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/fpga/dfl-fme-pr.c b/drivers/fpga/dfl-fme-pr.c
index a233a53db708..1194c0e850e0 100644
--- a/drivers/fpga/dfl-fme-pr.c
+++ b/drivers/fpga/dfl-fme-pr.c
@@ -97,10 +97,6 @@ static int fme_pr(struct platform_device *pdev, unsigned long arg)
 		return -EINVAL;
 	}
 
-	if (!access_ok((void __user *)(unsigned long)port_pr.buffer_address,
-		       port_pr.buffer_size))
-		return -EFAULT;
-
 	/*
 	 * align PR buffer per PR bandwidth, as HW ignores the extra padding
 	 * data automatically.
-- 
2.11.0


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 10/20] drivers/fpga/dfl-afu-dma-region.c: get rid of pointless access_ok()
  2020-05-09 23:45 ` [PATCH 01/20] dlmfs_file_write(): get rid of pointless access_ok() Al Viro
                     ` (7 preceding siblings ...)
  2020-05-09 23:45   ` [PATCH 09/20] drivers/fpga/dfl-fme-pr.c: " Al Viro
@ 2020-05-09 23:45   ` Al Viro
  2020-05-09 23:45   ` [PATCH 11/20] amifb: get rid of pointless access_ok() calls Al Viro
                     ` (9 subsequent siblings)
  18 siblings, 0 replies; 42+ messages in thread
From: Al Viro @ 2020-05-09 23:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, linux-fsdevel, Wu Hao

From: Al Viro <viro@zeniv.linux.org.uk>

Address is passed to get_user_pages_fast(), which does access_ok().
NB: this is called only from ->ioctl(), and only under USER_DS.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/fpga/dfl-afu-dma-region.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/fpga/dfl-afu-dma-region.c b/drivers/fpga/dfl-afu-dma-region.c
index 62f924489db5..d902acb36d14 100644
--- a/drivers/fpga/dfl-afu-dma-region.c
+++ b/drivers/fpga/dfl-afu-dma-region.c
@@ -324,10 +324,6 @@ int afu_dma_map_region(struct dfl_feature_platform_data *pdata,
 	if (user_addr + length < user_addr)
 		return -EINVAL;
 
-	if (!access_ok((void __user *)(unsigned long)user_addr,
-		       length))
-		return -EINVAL;
-
 	region = kzalloc(sizeof(*region), GFP_KERNEL);
 	if (!region)
 		return -ENOMEM;
-- 
2.11.0


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 11/20] amifb: get rid of pointless access_ok() calls
  2020-05-09 23:45 ` [PATCH 01/20] dlmfs_file_write(): get rid of pointless access_ok() Al Viro
                     ` (8 preceding siblings ...)
  2020-05-09 23:45   ` [PATCH 10/20] drivers/fpga/dfl-afu-dma-region.c: " Al Viro
@ 2020-05-09 23:45   ` Al Viro
  2020-05-14 13:45     ` Bartlomiej Zolnierkiewicz
  2020-05-09 23:45   ` [PATCH 12/20] omapfb: " Al Viro
                     ` (8 subsequent siblings)
  18 siblings, 1 reply; 42+ messages in thread
From: Al Viro @ 2020-05-09 23:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, linux-fsdevel, Bartlomiej Zolnierkiewicz

From: Al Viro <viro@zeniv.linux.org.uk>

addresses passed only to get_user() and put_user()

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/video/fbdev/amifb.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/drivers/video/fbdev/amifb.c b/drivers/video/fbdev/amifb.c
index 20e03e00b66d..6062104f3afb 100644
--- a/drivers/video/fbdev/amifb.c
+++ b/drivers/video/fbdev/amifb.c
@@ -1855,8 +1855,6 @@ static int ami_get_var_cursorinfo(struct fb_var_cursorinfo *var,
 	var->yspot = par->crsr.spot_y;
 	if (size > var->height * var->width)
 		return -ENAMETOOLONG;
-	if (!access_ok(data, size))
-		return -EFAULT;
 	delta = 1 << par->crsr.fmode;
 	lspr = lofsprite + (delta << 1);
 	if (par->bplcon0 & BPC0_LACE)
@@ -1935,8 +1933,6 @@ static int ami_set_var_cursorinfo(struct fb_var_cursorinfo *var,
 		return -EINVAL;
 	if (!var->height)
 		return -EINVAL;
-	if (!access_ok(data, var->width * var->height))
-		return -EFAULT;
 	delta = 1 << fmode;
 	lofsprite = shfsprite = (u_short *)spritememory;
 	lspr = lofsprite + (delta << 1);
-- 
2.11.0


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 12/20] omapfb: get rid of pointless access_ok() calls
  2020-05-09 23:45 ` [PATCH 01/20] dlmfs_file_write(): get rid of pointless access_ok() Al Viro
                     ` (9 preceding siblings ...)
  2020-05-09 23:45   ` [PATCH 11/20] amifb: get rid of pointless access_ok() calls Al Viro
@ 2020-05-09 23:45   ` Al Viro
  2020-05-14 13:39     ` Bartlomiej Zolnierkiewicz
  2020-05-09 23:45   ` [PATCH 13/20] drivers/crypto/ccp/sev-dev.c: get rid of pointless access_ok() Al Viro
                     ` (7 subsequent siblings)
  18 siblings, 1 reply; 42+ messages in thread
From: Al Viro @ 2020-05-09 23:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, linux-fsdevel, Bartlomiej Zolnierkiewicz

From: Al Viro <viro@zeniv.linux.org.uk>

address is passed only to copy_to_user()

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c b/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c
index 56995f44e76d..f40be68d5aac 100644
--- a/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c
+++ b/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c
@@ -482,9 +482,6 @@ static int omapfb_memory_read(struct fb_info *fbi,
 	if (!display || !display->driver->memory_read)
 		return -ENOENT;
 
-	if (!access_ok(mr->buffer, mr->buffer_size))
-		return -EFAULT;
-
 	if (mr->w > 4096 || mr->h > 4096)
 		return -EINVAL;
 
-- 
2.11.0


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 13/20] drivers/crypto/ccp/sev-dev.c: get rid of pointless access_ok()
  2020-05-09 23:45 ` [PATCH 01/20] dlmfs_file_write(): get rid of pointless access_ok() Al Viro
                     ` (10 preceding siblings ...)
  2020-05-09 23:45   ` [PATCH 12/20] omapfb: " Al Viro
@ 2020-05-09 23:45   ` Al Viro
  2020-05-09 23:45   ` [PATCH 14/20] via-pmu: don't bother with access_ok() Al Viro
                     ` (6 subsequent siblings)
  18 siblings, 0 replies; 42+ messages in thread
From: Al Viro @ 2020-05-09 23:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, linux-fsdevel, Tom Lendacky

From: Al Viro <viro@zeniv.linux.org.uk>

Contrary to the comments, those do *NOT* verify anything about
writability of memory, etc.

In all cases addresses are passed only to copy_to_user().

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/crypto/ccp/sev-dev.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/drivers/crypto/ccp/sev-dev.c b/drivers/crypto/ccp/sev-dev.c
index 896f190b9a50..7f97164cbafb 100644
--- a/drivers/crypto/ccp/sev-dev.c
+++ b/drivers/crypto/ccp/sev-dev.c
@@ -371,8 +371,7 @@ static int sev_ioctl_do_pek_csr(struct sev_issue_cmd *argp, bool writable)
 		goto cmd;
 
 	/* allocate a physically contiguous buffer to store the CSR blob */
-	if (!access_ok(input.address, input.length) ||
-	    input.length > SEV_FW_BLOB_MAX_SIZE) {
+	if (input.length > SEV_FW_BLOB_MAX_SIZE) {
 		ret = -EFAULT;
 		goto e_free;
 	}
@@ -609,12 +608,6 @@ static int sev_ioctl_do_get_id2(struct sev_issue_cmd *argp)
 	if (copy_from_user(&input, (void __user *)argp->data, sizeof(input)))
 		return -EFAULT;
 
-	/* Check if we have write access to the userspace buffer */
-	if (input.address &&
-	    input.length &&
-	    !access_ok(input.address, input.length))
-		return -EFAULT;
-
 	data = kzalloc(sizeof(*data), GFP_KERNEL);
 	if (!data)
 		return -ENOMEM;
@@ -730,15 +723,13 @@ static int sev_ioctl_do_pdh_export(struct sev_issue_cmd *argp, bool writable)
 		goto cmd;
 
 	/* Allocate a physically contiguous buffer to store the PDH blob. */
-	if ((input.pdh_cert_len > SEV_FW_BLOB_MAX_SIZE) ||
-	    !access_ok(input.pdh_cert_address, input.pdh_cert_len)) {
+	if (input.pdh_cert_len > SEV_FW_BLOB_MAX_SIZE) {
 		ret = -EFAULT;
 		goto e_free;
 	}
 
 	/* Allocate a physically contiguous buffer to store the cert chain blob. */
-	if ((input.cert_chain_len > SEV_FW_BLOB_MAX_SIZE) ||
-	    !access_ok(input.cert_chain_address, input.cert_chain_len)) {
+	if (input.cert_chain_len > SEV_FW_BLOB_MAX_SIZE) {
 		ret = -EFAULT;
 		goto e_free;
 	}
-- 
2.11.0


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 14/20] via-pmu: don't bother with access_ok()
  2020-05-09 23:45 ` [PATCH 01/20] dlmfs_file_write(): get rid of pointless access_ok() Al Viro
                     ` (11 preceding siblings ...)
  2020-05-09 23:45   ` [PATCH 13/20] drivers/crypto/ccp/sev-dev.c: get rid of pointless access_ok() Al Viro
@ 2020-05-09 23:45   ` Al Viro
  2020-05-09 23:45   ` [PATCH 15/20] drm_read(): get rid of pointless access_ok() Al Viro
                     ` (5 subsequent siblings)
  18 siblings, 0 replies; 42+ messages in thread
From: Al Viro @ 2020-05-09 23:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, linux-fsdevel

From: Al Viro <viro@zeniv.linux.org.uk>

we are using copy_to_user() for actual copying

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/macintosh/via-pmu.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/macintosh/via-pmu.c b/drivers/macintosh/via-pmu.c
index 83eb05bf85ff..8450d7c008d0 100644
--- a/drivers/macintosh/via-pmu.c
+++ b/drivers/macintosh/via-pmu.c
@@ -2184,8 +2184,6 @@ pmu_read(struct file *file, char __user *buf,
 
 	if (count < 1 || !pp)
 		return -EINVAL;
-	if (!access_ok(buf, count))
-		return -EFAULT;
 
 	spin_lock_irqsave(&pp->lock, flags);
 	add_wait_queue(&pp->wait, &wait);
-- 
2.11.0


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 15/20] drm_read(): get rid of pointless access_ok()
  2020-05-09 23:45 ` [PATCH 01/20] dlmfs_file_write(): get rid of pointless access_ok() Al Viro
                     ` (12 preceding siblings ...)
  2020-05-09 23:45   ` [PATCH 14/20] via-pmu: don't bother with access_ok() Al Viro
@ 2020-05-09 23:45   ` Al Viro
  2020-05-09 23:45   ` [PATCH 16/20] efi_test: " Al Viro
                     ` (4 subsequent siblings)
  18 siblings, 0 replies; 42+ messages in thread
From: Al Viro @ 2020-05-09 23:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, linux-fsdevel, Maarten Lankhorst

From: Al Viro <viro@zeniv.linux.org.uk>

address is passed only to copy_to_user()

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/gpu/drm/drm_file.c | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/drivers/gpu/drm/drm_file.c b/drivers/gpu/drm/drm_file.c
index eb009d3ab48f..6a1f6c802415 100644
--- a/drivers/gpu/drm/drm_file.c
+++ b/drivers/gpu/drm/drm_file.c
@@ -569,9 +569,6 @@ ssize_t drm_read(struct file *filp, char __user *buffer,
 	struct drm_device *dev = file_priv->minor->dev;
 	ssize_t ret;
 
-	if (!access_ok(buffer, count))
-		return -EFAULT;
-
 	ret = mutex_lock_interruptible(&file_priv->event_read_lock);
 	if (ret)
 		return ret;
-- 
2.11.0


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 16/20] efi_test: get rid of pointless access_ok()
  2020-05-09 23:45 ` [PATCH 01/20] dlmfs_file_write(): get rid of pointless access_ok() Al Viro
                     ` (13 preceding siblings ...)
  2020-05-09 23:45   ` [PATCH 15/20] drm_read(): get rid of pointless access_ok() Al Viro
@ 2020-05-09 23:45   ` Al Viro
  2020-05-09 23:45   ` [PATCH 17/20] lpfc_debugfs: " Al Viro
                     ` (3 subsequent siblings)
  18 siblings, 0 replies; 42+ messages in thread
From: Al Viro @ 2020-05-09 23:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, linux-fsdevel, Ivan Hu

From: Al Viro <viro@zeniv.linux.org.uk>

really, people - get_user(), copy_from_user(), memdup_user(), etc.
all fail if access_ok() does.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/firmware/efi/test/efi_test.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/firmware/efi/test/efi_test.c b/drivers/firmware/efi/test/efi_test.c
index 7baf48c01e72..ddf9eae396fe 100644
--- a/drivers/firmware/efi/test/efi_test.c
+++ b/drivers/firmware/efi/test/efi_test.c
@@ -70,9 +70,6 @@ copy_ucs2_from_user_len(efi_char16_t **dst, efi_char16_t __user *src,
 		return 0;
 	}
 
-	if (!access_ok(src, 1))
-		return -EFAULT;
-
 	buf = memdup_user(src, len);
 	if (IS_ERR(buf)) {
 		*dst = NULL;
@@ -91,9 +88,6 @@ copy_ucs2_from_user_len(efi_char16_t **dst, efi_char16_t __user *src,
 static inline int
 get_ucs2_strsize_from_user(efi_char16_t __user *src, size_t *len)
 {
-	if (!access_ok(src, 1))
-		return -EFAULT;
-
 	*len = user_ucs2_strsize(src);
 	if (*len == 0)
 		return -EFAULT;
@@ -118,9 +112,6 @@ copy_ucs2_from_user(efi_char16_t **dst, efi_char16_t __user *src)
 {
 	size_t len;
 
-	if (!access_ok(src, 1))
-		return -EFAULT;
-
 	len = user_ucs2_strsize(src);
 	if (len == 0)
 		return -EFAULT;
@@ -142,9 +133,6 @@ copy_ucs2_to_user_len(efi_char16_t __user *dst, efi_char16_t *src, size_t len)
 	if (!src)
 		return 0;
 
-	if (!access_ok(dst, 1))
-		return -EFAULT;
-
 	return copy_to_user(dst, src, len);
 }
 
-- 
2.11.0


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 17/20] lpfc_debugfs: get rid of pointless access_ok()
  2020-05-09 23:45 ` [PATCH 01/20] dlmfs_file_write(): get rid of pointless access_ok() Al Viro
                     ` (14 preceding siblings ...)
  2020-05-09 23:45   ` [PATCH 16/20] efi_test: " Al Viro
@ 2020-05-09 23:45   ` Al Viro
  2020-05-09 23:45   ` [PATCH 18/20] usb: get rid of pointless access_ok() calls Al Viro
                     ` (2 subsequent siblings)
  18 siblings, 0 replies; 42+ messages in thread
From: Al Viro @ 2020-05-09 23:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, linux-fsdevel, James Smart, linux-scsi

From: Al Viro <viro@zeniv.linux.org.uk>

	No, you do NOT need to "protect copy from user" that way.
Incidentally, your userland ABI stinks.  I understand that you
wanted to accept "reset" and "reset\n" as equivalent, but I suspect
that accepting "reset this, you !@^!@!" had been an accident.
Nothing to do about that now - it is a userland ABI...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/scsi/lpfc/lpfc_debugfs.c | 12 ------------
 1 file changed, 12 deletions(-)

diff --git a/drivers/scsi/lpfc/lpfc_debugfs.c b/drivers/scsi/lpfc/lpfc_debugfs.c
index 8a6e02aa553f..5a754fb5f854 100644
--- a/drivers/scsi/lpfc/lpfc_debugfs.c
+++ b/drivers/scsi/lpfc/lpfc_debugfs.c
@@ -2166,10 +2166,6 @@ lpfc_debugfs_lockstat_write(struct file *file, const char __user *buf,
 	char *pbuf;
 	int i;
 
-	/* Protect copy from user */
-	if (!access_ok(buf, nbytes))
-		return -EFAULT;
-
 	memset(mybuf, 0, sizeof(mybuf));
 
 	if (copy_from_user(mybuf, buf, nbytes))
@@ -2621,10 +2617,6 @@ lpfc_debugfs_multixripools_write(struct file *file, const char __user *buf,
 	if (nbytes > 64)
 		nbytes = 64;
 
-	/* Protect copy from user */
-	if (!access_ok(buf, nbytes))
-		return -EFAULT;
-
 	memset(mybuf, 0, sizeof(mybuf));
 
 	if (copy_from_user(mybuf, buf, nbytes))
@@ -2787,10 +2779,6 @@ lpfc_debugfs_scsistat_write(struct file *file, const char __user *buf,
 	char mybuf[6] = {0};
 	int i;
 
-	/* Protect copy from user */
-	if (!access_ok(buf, nbytes))
-		return -EFAULT;
-
 	if (copy_from_user(mybuf, buf, (nbytes >= sizeof(mybuf)) ?
 				       (sizeof(mybuf) - 1) : nbytes))
 		return -EFAULT;
-- 
2.11.0


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 18/20] usb: get rid of pointless access_ok() calls
  2020-05-09 23:45 ` [PATCH 01/20] dlmfs_file_write(): get rid of pointless access_ok() Al Viro
                     ` (15 preceding siblings ...)
  2020-05-09 23:45   ` [PATCH 17/20] lpfc_debugfs: " Al Viro
@ 2020-05-09 23:45   ` Al Viro
  2020-05-15 10:53     ` Greg Kroah-Hartman
  2020-05-09 23:45   ` [PATCH 19/20] hfi1: get rid of pointless access_ok() Al Viro
  2020-05-09 23:45   ` [PATCH 4/4] vmci_host: " Al Viro
  18 siblings, 1 reply; 42+ messages in thread
From: Al Viro @ 2020-05-09 23:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, linux-fsdevel, Greg Kroah-Hartman

From: Al Viro <viro@zeniv.linux.org.uk>

in all affected cases addresses are passed only to
copy_from()_user or copy_to_user().

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/usb/core/devices.c          | 2 --
 drivers/usb/core/devio.c            | 9 ---------
 drivers/usb/gadget/function/f_hid.c | 6 ------
 3 files changed, 17 deletions(-)

diff --git a/drivers/usb/core/devices.c b/drivers/usb/core/devices.c
index 44f28a114c2b..94b6fa6e585e 100644
--- a/drivers/usb/core/devices.c
+++ b/drivers/usb/core/devices.c
@@ -598,8 +598,6 @@ static ssize_t usb_device_read(struct file *file, char __user *buf,
 		return -EINVAL;
 	if (nbytes <= 0)
 		return 0;
-	if (!access_ok(buf, nbytes))
-		return -EFAULT;
 
 	mutex_lock(&usb_bus_idr_lock);
 	/* print devices for all busses */
diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 6833c918abce..544769807ab8 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -1127,11 +1127,6 @@ static int proc_control(struct usb_dev_state *ps, void __user *arg)
 		ctrl.bRequestType, ctrl.bRequest, ctrl.wValue,
 		ctrl.wIndex, ctrl.wLength);
 	if (ctrl.bRequestType & 0x80) {
-		if (ctrl.wLength && !access_ok(ctrl.data,
-					       ctrl.wLength)) {
-			ret = -EINVAL;
-			goto done;
-		}
 		pipe = usb_rcvctrlpipe(dev, 0);
 		snoop_urb(dev, NULL, pipe, ctrl.wLength, tmo, SUBMIT, NULL, 0);
 
@@ -1216,10 +1211,6 @@ static int proc_bulk(struct usb_dev_state *ps, void __user *arg)
 	}
 	tmo = bulk.timeout;
 	if (bulk.ep & 0x80) {
-		if (len1 && !access_ok(bulk.data, len1)) {
-			ret = -EINVAL;
-			goto done;
-		}
 		snoop_urb(dev, NULL, pipe, len1, tmo, SUBMIT, NULL, 0);
 
 		usb_unlock_device(dev);
diff --git a/drivers/usb/gadget/function/f_hid.c b/drivers/usb/gadget/function/f_hid.c
index f3816a5c861e..df671acdd464 100644
--- a/drivers/usb/gadget/function/f_hid.c
+++ b/drivers/usb/gadget/function/f_hid.c
@@ -252,9 +252,6 @@ static ssize_t f_hidg_read(struct file *file, char __user *buffer,
 	if (!count)
 		return 0;
 
-	if (!access_ok(buffer, count))
-		return -EFAULT;
-
 	spin_lock_irqsave(&hidg->read_spinlock, flags);
 
 #define READ_COND (!list_empty(&hidg->completed_out_req))
@@ -339,9 +336,6 @@ static ssize_t f_hidg_write(struct file *file, const char __user *buffer,
 	unsigned long flags;
 	ssize_t status = -ENOMEM;
 
-	if (!access_ok(buffer, count))
-		return -EFAULT;
-
 	spin_lock_irqsave(&hidg->write_spinlock, flags);
 
 #define WRITE_COND (!hidg->write_pending)
-- 
2.11.0


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 19/20] hfi1: get rid of pointless access_ok()
  2020-05-09 23:45 ` [PATCH 01/20] dlmfs_file_write(): get rid of pointless access_ok() Al Viro
                     ` (16 preceding siblings ...)
  2020-05-09 23:45   ` [PATCH 18/20] usb: get rid of pointless access_ok() calls Al Viro
@ 2020-05-09 23:45   ` Al Viro
  2020-05-09 23:45   ` [PATCH 4/4] vmci_host: " Al Viro
  18 siblings, 0 replies; 42+ messages in thread
From: Al Viro @ 2020-05-09 23:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, linux-fsdevel, Mike Marciniszyn

From: Al Viro <viro@zeniv.linux.org.uk>

pin_user_pages_fast() doesn't need that from its caller.
NB: only reachable from ->ioctl(), and only under USER_DS

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/infiniband/hw/hfi1/user_exp_rcv.c | 7 -------
 1 file changed, 7 deletions(-)

diff --git a/drivers/infiniband/hw/hfi1/user_exp_rcv.c b/drivers/infiniband/hw/hfi1/user_exp_rcv.c
index 4da03f823474..f81ca20f4b69 100644
--- a/drivers/infiniband/hw/hfi1/user_exp_rcv.c
+++ b/drivers/infiniband/hw/hfi1/user_exp_rcv.c
@@ -206,13 +206,6 @@ static int pin_rcv_pages(struct hfi1_filedata *fd, struct tid_user_buf *tidbuf)
 		return -EINVAL;
 	}
 
-	/* Verify that access is OK for the user buffer */
-	if (!access_ok((void __user *)vaddr,
-		       npages * PAGE_SIZE)) {
-		dd_dev_err(dd, "Fail vaddr %p, %u pages, !access_ok\n",
-			   (void *)vaddr, npages);
-		return -EFAULT;
-	}
 	/* Allocate the array of struct page pointers needed for pinning */
 	pages = kcalloc(npages, sizeof(*pages), GFP_KERNEL);
 	if (!pages)
-- 
2.11.0


^ permalink raw reply	[flat|nested] 42+ messages in thread

* [PATCH 4/4] vmci_host: get rid of pointless access_ok()
  2020-05-09 23:45 ` [PATCH 01/20] dlmfs_file_write(): get rid of pointless access_ok() Al Viro
                     ` (17 preceding siblings ...)
  2020-05-09 23:45   ` [PATCH 19/20] hfi1: get rid of pointless access_ok() Al Viro
@ 2020-05-09 23:45   ` Al Viro
  2020-05-15 10:53     ` Greg Kroah-Hartman
  18 siblings, 1 reply; 42+ messages in thread
From: Al Viro @ 2020-05-09 23:45 UTC (permalink / raw)
  To: linux-kernel; +Cc: Linus Torvalds, linux-fsdevel, Greg Kroah-Hartman

From: Al Viro <viro@zeniv.linux.org.uk>

get_user_pages_fast() doesn't need the caller to check that.
NB: reachable only from ioctl(2) and only under USER_DS

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/misc/vmw_vmci/vmci_host.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/drivers/misc/vmw_vmci/vmci_host.c b/drivers/misc/vmw_vmci/vmci_host.c
index ce16d6b99295..2d8328d928d5 100644
--- a/drivers/misc/vmw_vmci/vmci_host.c
+++ b/drivers/misc/vmw_vmci/vmci_host.c
@@ -233,8 +233,6 @@ static int vmci_host_setup_notify(struct vmci_ctx *context,
 	 * about the size.
 	 */
 	BUILD_BUG_ON(sizeof(bool) != sizeof(u8));
-	if (!access_ok((void __user *)uva, sizeof(u8)))
-		return VMCI_ERROR_GENERIC;
 
 	/*
 	 * Lock physical page backing a given user VA.
-- 
2.11.0


^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCHES] uaccess simple access_ok() removals
  2020-05-09 23:41 [PATCHES] uaccess simple access_ok() removals Al Viro
  2020-05-09 23:45 ` [PATCH 01/20] dlmfs_file_write(): get rid of pointless access_ok() Al Viro
@ 2020-05-10  0:34 ` Linus Torvalds
  2020-05-10  3:27   ` Al Viro
  2020-05-10 14:34 ` David Laight
  2 siblings, 1 reply; 42+ messages in thread
From: Linus Torvalds @ 2020-05-10  0:34 UTC (permalink / raw)
  To: Al Viro; +Cc: Linux Kernel Mailing List, linux-fsdevel

On Sat, May 9, 2020 at 4:41 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
>         Individual patches in followups; if nobody screams - into #for-next
> it goes...

Looks fine to me, although I only read your commit logs, I didn't
verify that what you stated was actually true (ie the whole "only used
for xyz" parts).

But I'll take your word for it. Particularly the double-underscore
versions are getting rare (and presumably some of the other branches
you have make it rarer still).

               Linus

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 05/20] tomoyo_write_control(): get rid of pointless access_ok()
  2020-05-09 23:45   ` [PATCH 05/20] tomoyo_write_control(): get rid of pointless access_ok() Al Viro
@ 2020-05-10  0:50     ` Tetsuo Handa
  2020-05-10  0:57       ` Linus Torvalds
  0 siblings, 1 reply; 42+ messages in thread
From: Tetsuo Handa @ 2020-05-10  0:50 UTC (permalink / raw)
  To: Al Viro, linux-kernel; +Cc: Linus Torvalds, linux-fsdevel

Hello, Al.

I think that this access_ok() check helps reducing partial writes (either
"whole amount was processed" or "not processed at all" unless -ENOMEM).
Do you think that such attempt is pointless? Then, please go ahead...

On 2020/05/10 8:45, Al Viro wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> address is passed only to get_user()
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  security/tomoyo/common.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/security/tomoyo/common.c b/security/tomoyo/common.c
> index 1b467381986f..f93f8acd05f7 100644
> --- a/security/tomoyo/common.c
> +++ b/security/tomoyo/common.c
> @@ -2662,8 +2662,6 @@ ssize_t tomoyo_write_control(struct tomoyo_io_buffer *head,
>  
>  	if (!head->write)
>  		return -EINVAL;
> -	if (!access_ok(buffer, buffer_len))
> -		return -EFAULT;
>  	if (mutex_lock_interruptible(&head->io_sem))
>  		return -EINTR;
>  	head->read_user_buf_avail = 0;
> 


^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 05/20] tomoyo_write_control(): get rid of pointless access_ok()
  2020-05-10  0:50     ` Tetsuo Handa
@ 2020-05-10  0:57       ` Linus Torvalds
  2020-05-10  1:04         ` Tetsuo Handa
  2020-05-10  3:01         ` Al Viro
  0 siblings, 2 replies; 42+ messages in thread
From: Linus Torvalds @ 2020-05-10  0:57 UTC (permalink / raw)
  To: Tetsuo Handa; +Cc: Al Viro, Linux Kernel Mailing List, linux-fsdevel

On Sat, May 9, 2020 at 5:51 PM Tetsuo Handa
<penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> I think that this access_ok() check helps reducing partial writes (either
> "whole amount was processed" or "not processed at all" unless -ENOMEM).

No it doesn't.

"access_ok()" only checks the range being a valid user address range.

It doesn't actually help at all if the worry is "what if we take a
page fault in the middle".  Because it simply doesn't check those
kinds of things.

Now, if somebody passes actual invalid ranges (ie kernel addresses or
other crazy stuff), they only have themselves to blame. The invalid
range will be noticed when actually doing the user copy, and then
you'll get EFAULT there. But there's no point in trying to figure that
out early - it's only adding overhead, and it doesn't help any normal
case.

                  Linus

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 05/20] tomoyo_write_control(): get rid of pointless access_ok()
  2020-05-10  0:57       ` Linus Torvalds
@ 2020-05-10  1:04         ` Tetsuo Handa
  2020-05-10  3:01         ` Al Viro
  1 sibling, 0 replies; 42+ messages in thread
From: Tetsuo Handa @ 2020-05-10  1:04 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Al Viro, Linux Kernel Mailing List, linux-fsdevel

On 2020/05/10 9:57, Linus Torvalds wrote:
> On Sat, May 9, 2020 at 5:51 PM Tetsuo Handa
> <penguin-kernel@i-love.sakura.ne.jp> wrote:
>>
>> I think that this access_ok() check helps reducing partial writes (either
>> "whole amount was processed" or "not processed at all" unless -ENOMEM).
> 
> No it doesn't.
> 
> "access_ok()" only checks the range being a valid user address range.
> 

I see. Thank you.

Acked-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 05/20] tomoyo_write_control(): get rid of pointless access_ok()
  2020-05-10  0:57       ` Linus Torvalds
  2020-05-10  1:04         ` Tetsuo Handa
@ 2020-05-10  3:01         ` Al Viro
  1 sibling, 0 replies; 42+ messages in thread
From: Al Viro @ 2020-05-10  3:01 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Tetsuo Handa, Linux Kernel Mailing List, linux-fsdevel

On Sat, May 09, 2020 at 05:57:56PM -0700, Linus Torvalds wrote:
> On Sat, May 9, 2020 at 5:51 PM Tetsuo Handa
> <penguin-kernel@i-love.sakura.ne.jp> wrote:
> >
> > I think that this access_ok() check helps reducing partial writes (either
> > "whole amount was processed" or "not processed at all" unless -ENOMEM).
> 
> No it doesn't.
> 
> "access_ok()" only checks the range being a valid user address range.
> 
> It doesn't actually help at all if the worry is "what if we take a
> page fault in the middle".  Because it simply doesn't check those
> kinds of things.
> 
> Now, if somebody passes actual invalid ranges (ie kernel addresses or
> other crazy stuff), they only have themselves to blame. The invalid
> range will be noticed when actually doing the user copy, and then
> you'll get EFAULT there. But there's no point in trying to figure that
> out early - it's only adding overhead, and it doesn't help any normal
> case.

It might be a good idea to add Documentation/what-access_ok-does_not ;-/
In addition to what you've mentioned,

* access_ok() does not fault anything in; never had.

* access_ok() does not verify that memory is readable/writable/there at all;
never had, except for genuine 80386 and (maybe) some of the shittier 486
clones.

* access_ok() does not protect you from the length being insanely large;
even on i386 it can pass with length being a bit under 3Gb.  If you
count upon it to prevent kmalloc() complaints about insanely large
allocation (yes, I've seen that excuse used), you are wrong.

* on a bunch of architectures access_ok() never rejects anything, and
no, that's _not_ MMU-less ones.  sparc64, for example.  Or s390, or
parisc, etc.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCHES] uaccess simple access_ok() removals
  2020-05-10  0:34 ` [PATCHES] uaccess simple access_ok() removals Linus Torvalds
@ 2020-05-10  3:27   ` Al Viro
  0 siblings, 0 replies; 42+ messages in thread
From: Al Viro @ 2020-05-10  3:27 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Linux Kernel Mailing List, linux-fsdevel

On Sat, May 09, 2020 at 05:34:58PM -0700, Linus Torvalds wrote:
> On Sat, May 9, 2020 at 4:41 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> >         Individual patches in followups; if nobody screams - into #for-next
> > it goes...
> 
> Looks fine to me, although I only read your commit logs, I didn't
> verify that what you stated was actually true (ie the whole "only used
> for xyz" parts).
> 
> But I'll take your word for it. Particularly the double-underscore
> versions are getting rare (and presumably some of the other branches
> you have make it rarer still).

I have - FWIW, right now I'm going through the patch series for netdev;
once I'm done with turning commit messages into something printable
(and finish rereading the patches themselves), there it goes, hopefully
tonight.  Then a bunch of small branches + repost of csum one + some
of the weird shit (comedi compat ioctls - the largest pile of
__get_user() and __put_user() outside of arch/* is festering there).
Then regset stuff + (probably) resurrection of i915 stuff.

I've got a bunch of stuff around execve(), but that'll have to wait
until I get through the recently posted execve-related patchsets...

Then there's e.g. mempolicy compat stuff, etc., but I'd rather wait
for several days before posting that, reviewers' bandwidth being
finite...

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 04/20] FIEMAP: don't bother with access_ok()
  2020-05-09 23:45   ` [PATCH 04/20] FIEMAP: " Al Viro
@ 2020-05-10  7:02     ` Christoph Hellwig
  2020-05-13 19:02       ` Al Viro
  0 siblings, 1 reply; 42+ messages in thread
From: Christoph Hellwig @ 2020-05-10  7:02 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-kernel, Linus Torvalds, linux-fsdevel

On Sun, May 10, 2020 at 12:45:41AM +0100, Al Viro wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> we use copy_to_user() on that thing anyway (and always had).

I already have this patch in this series:

https://lore.kernel.org/linux-fsdevel/20200507145924.GA28854@lst.de/T/#t

which is waiting to be picked up [1], and also has some chance for conflicts
due to changes next to the access_ok.

[1] except for the first two patches, which Ted plans to send for 5.7

^ permalink raw reply	[flat|nested] 42+ messages in thread

* RE: [PATCHES] uaccess simple access_ok() removals
  2020-05-09 23:41 [PATCHES] uaccess simple access_ok() removals Al Viro
  2020-05-09 23:45 ` [PATCH 01/20] dlmfs_file_write(): get rid of pointless access_ok() Al Viro
  2020-05-10  0:34 ` [PATCHES] uaccess simple access_ok() removals Linus Torvalds
@ 2020-05-10 14:34 ` David Laight
  2 siblings, 0 replies; 42+ messages in thread
From: David Laight @ 2020-05-10 14:34 UTC (permalink / raw)
  To: 'Al Viro', linux-kernel; +Cc: Linus Torvalds, linux-fsdevel

From: Al Viro
> Sent: 10 May 2020 00:41
> 
> 	One of the uaccess-related branches; this one is just the
> cases when access_ok() calls are trivially pointless - the address
> in question gets fed only to primitives that do access_ok() checks
> themselves.

There is also the check in rw_copy_check_uvector() that should
always be replicated by the copy_to/from_user() in _copy_to/from_iter().

And the strange call to rw_copy_check_uvector() in mm/process_vm_access.c
which carefully avoids the access_ok() check for the target process.
I did a quick look, but failed to see an obvious check further
down the call path.
The code is doing a read/write from another process, not sure when it
is used - not by gdb.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)


^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 04/20] FIEMAP: don't bother with access_ok()
  2020-05-10  7:02     ` Christoph Hellwig
@ 2020-05-13 19:02       ` Al Viro
  2020-05-13 19:38         ` Christoph Hellwig
  0 siblings, 1 reply; 42+ messages in thread
From: Al Viro @ 2020-05-13 19:02 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-kernel, Linus Torvalds, linux-fsdevel

On Sun, May 10, 2020 at 12:02:41AM -0700, Christoph Hellwig wrote:
> On Sun, May 10, 2020 at 12:45:41AM +0100, Al Viro wrote:
> > From: Al Viro <viro@zeniv.linux.org.uk>
> > 
> > we use copy_to_user() on that thing anyway (and always had).
> 
> I already have this patch in this series:
> 
> https://lore.kernel.org/linux-fsdevel/20200507145924.GA28854@lst.de/T/#t
> 
> which is waiting to be picked up [1], and also has some chance for conflicts
> due to changes next to the access_ok.
> 
> [1] except for the first two patches, which Ted plans to send for 5.7

I can drop this commit, of course, it's not a prereq for anything else in there.
Or I could pick your series into never-rebased branch, but it would complicate
the life wrt ext4 tree - up to you and Ted...

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 04/20] FIEMAP: don't bother with access_ok()
  2020-05-13 19:02       ` Al Viro
@ 2020-05-13 19:38         ` Christoph Hellwig
  2020-05-29 15:01           ` Al Viro
  0 siblings, 1 reply; 42+ messages in thread
From: Christoph Hellwig @ 2020-05-13 19:38 UTC (permalink / raw)
  To: Al Viro
  Cc: Christoph Hellwig, linux-kernel, Linus Torvalds, linux-fsdevel, tytso

On Wed, May 13, 2020 at 08:02:07PM +0100, Al Viro wrote:
> > https://lore.kernel.org/linux-fsdevel/20200507145924.GA28854@lst.de/T/#t
> > 
> > which is waiting to be picked up [1], and also has some chance for conflicts
> > due to changes next to the access_ok.
> > 
> > [1] except for the first two patches, which Ted plans to send for 5.7
> 
> I can drop this commit, of course, it's not a prereq for anything else in there.
> Or I could pick your series into never-rebased branch, but it would complicate
> the life wrt ext4 tree - up to you and Ted...

I really don't care - the first two really need to go in ASAP and
Ted promised to pick them up, but I've not seen them in linux-next
yet.  The rest can go wherever once the first ones hit mainline.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 12/20] omapfb: get rid of pointless access_ok() calls
  2020-05-09 23:45   ` [PATCH 12/20] omapfb: " Al Viro
@ 2020-05-14 13:39     ` Bartlomiej Zolnierkiewicz
  0 siblings, 0 replies; 42+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2020-05-14 13:39 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-kernel, Linus Torvalds, linux-fsdevel


On 5/10/20 1:45 AM, Al Viro wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> address is passed only to copy_to_user()
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

> ---
>  drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c b/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c
> index 56995f44e76d..f40be68d5aac 100644
> --- a/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c
> +++ b/drivers/video/fbdev/omap2/omapfb/omapfb-ioctl.c
> @@ -482,9 +482,6 @@ static int omapfb_memory_read(struct fb_info *fbi,
>  	if (!display || !display->driver->memory_read)
>  		return -ENOENT;
>  
> -	if (!access_ok(mr->buffer, mr->buffer_size))
> -		return -EFAULT;
> -
>  	if (mr->w > 4096 || mr->h > 4096)
>  		return -EINVAL;
>  
> 


^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 11/20] amifb: get rid of pointless access_ok() calls
  2020-05-09 23:45   ` [PATCH 11/20] amifb: get rid of pointless access_ok() calls Al Viro
@ 2020-05-14 13:45     ` Bartlomiej Zolnierkiewicz
  2020-05-14 14:07       ` Al Viro
  0 siblings, 1 reply; 42+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2020-05-14 13:45 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-kernel, Linus Torvalds, linux-fsdevel


Hi Al,

On 5/10/20 1:45 AM, Al Viro wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> addresses passed only to get_user() and put_user()

This driver lacks checks for {get,put}_user() return values so it will
now return 0 ("success") even if {get,put}_user() fails.

Am I missing something?

> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  drivers/video/fbdev/amifb.c | 4 ----
>  1 file changed, 4 deletions(-)
> 
> diff --git a/drivers/video/fbdev/amifb.c b/drivers/video/fbdev/amifb.c
> index 20e03e00b66d..6062104f3afb 100644
> --- a/drivers/video/fbdev/amifb.c
> +++ b/drivers/video/fbdev/amifb.c
> @@ -1855,8 +1855,6 @@ static int ami_get_var_cursorinfo(struct fb_var_cursorinfo *var,
>  	var->yspot = par->crsr.spot_y;
>  	if (size > var->height * var->width)
>  		return -ENAMETOOLONG;
> -	if (!access_ok(data, size))
> -		return -EFAULT;
>  	delta = 1 << par->crsr.fmode;
>  	lspr = lofsprite + (delta << 1);
>  	if (par->bplcon0 & BPC0_LACE)
> @@ -1935,8 +1933,6 @@ static int ami_set_var_cursorinfo(struct fb_var_cursorinfo *var,
>  		return -EINVAL;
>  	if (!var->height)
>  		return -EINVAL;
> -	if (!access_ok(data, var->width * var->height))
> -		return -EFAULT;
>  	delta = 1 << fmode;
>  	lofsprite = shfsprite = (u_short *)spritememory;
>  	lspr = lofsprite + (delta << 1);
> 
 
Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 11/20] amifb: get rid of pointless access_ok() calls
  2020-05-14 13:45     ` Bartlomiej Zolnierkiewicz
@ 2020-05-14 14:07       ` Al Viro
  2020-05-14 14:25         ` Bartlomiej Zolnierkiewicz
  0 siblings, 1 reply; 42+ messages in thread
From: Al Viro @ 2020-05-14 14:07 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: linux-kernel, Linus Torvalds, linux-fsdevel

On Thu, May 14, 2020 at 03:45:09PM +0200, Bartlomiej Zolnierkiewicz wrote:
> 
> Hi Al,
> 
> On 5/10/20 1:45 AM, Al Viro wrote:
> > From: Al Viro <viro@zeniv.linux.org.uk>
> > 
> > addresses passed only to get_user() and put_user()
> 
> This driver lacks checks for {get,put}_user() return values so it will
> now return 0 ("success") even if {get,put}_user() fails.
> 
> Am I missing something?

"now" is interesting, considering
/* We let the MMU do all checking */
static inline int access_ok(const void __user *addr,
                            unsigned long size)
{
        return 1;
}
in arch/m68k/include/asm/uaccess_mm.h

Again, access_ok() is *NOT* about checking if memory is readable/writable/there
in the first place.  All it does is a static check that address is in
"userland" range - on architectures that have kernel and userland sharing the
address space.  On architectures where we have separate ASI or equivalents
thereof for kernel and for userland the fscker is always true.

If MMU will prevent access to kernel memory by uaccess insns for given address
range, access_ok() is fine with it.  It does not do anything else.

And yes, get_user()/put_user() callers should handle the fact that those can
fail.  Which they bloody well can _after_ _success_ of access_ok().  And
without any races whatsoever.

IOW, the lack of such checks is a bug, but it's quite independent from the
bogus access_ok() call.  On any architecture.  mmap() something, munmap()
it and pass the address where it used to be to that ioctl().  Failing
get_user()/put_user() is guaranteed, so's succeeding access_ok().

And that code is built only on amiga, so access_ok() always succeeds, anyway.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 11/20] amifb: get rid of pointless access_ok() calls
  2020-05-14 14:07       ` Al Viro
@ 2020-05-14 14:25         ` Bartlomiej Zolnierkiewicz
  2020-05-14 17:41           ` Al Viro
  0 siblings, 1 reply; 42+ messages in thread
From: Bartlomiej Zolnierkiewicz @ 2020-05-14 14:25 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-kernel, Linus Torvalds, linux-fsdevel


On 5/14/20 4:07 PM, Al Viro wrote:
> On Thu, May 14, 2020 at 03:45:09PM +0200, Bartlomiej Zolnierkiewicz wrote:
>>
>> Hi Al,
>>
>> On 5/10/20 1:45 AM, Al Viro wrote:
>>> From: Al Viro <viro@zeniv.linux.org.uk>
>>>
>>> addresses passed only to get_user() and put_user()
>>
>> This driver lacks checks for {get,put}_user() return values so it will
>> now return 0 ("success") even if {get,put}_user() fails.
>>
>> Am I missing something?
> 
> "now" is interesting, considering
> /* We let the MMU do all checking */
> static inline int access_ok(const void __user *addr,
>                             unsigned long size)
> {
>         return 1;
> }
> in arch/m68k/include/asm/uaccess_mm.h
> 
> Again, access_ok() is *NOT* about checking if memory is readable/writable/there
> in the first place.  All it does is a static check that address is in
> "userland" range - on architectures that have kernel and userland sharing the
> address space.  On architectures where we have separate ASI or equivalents
> thereof for kernel and for userland the fscker is always true.
> 
> If MMU will prevent access to kernel memory by uaccess insns for given address
> range, access_ok() is fine with it.  It does not do anything else.
> 
> And yes, get_user()/put_user() callers should handle the fact that those can
> fail.  Which they bloody well can _after_ _success_ of access_ok().  And
> without any races whatsoever.
> 
> IOW, the lack of such checks is a bug, but it's quite independent from the
> bogus access_ok() call.  On any architecture.  mmap() something, munmap()
> it and pass the address where it used to be to that ioctl().  Failing
> get_user()/put_user() is guaranteed, so's succeeding access_ok().
> 
> And that code is built only on amiga, so access_ok() always succeeds, anyway.

Thank you for in-detail explanations, for this patch:

Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>

Could you also please take care of adding missing checks for {get,put}_user()
failures later?

Best regards,
--
Bartlomiej Zolnierkiewicz
Samsung R&D Institute Poland
Samsung Electronics

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 11/20] amifb: get rid of pointless access_ok() calls
  2020-05-14 14:25         ` Bartlomiej Zolnierkiewicz
@ 2020-05-14 17:41           ` Al Viro
  2020-05-14 20:21             ` Geert Uytterhoeven
  0 siblings, 1 reply; 42+ messages in thread
From: Al Viro @ 2020-05-14 17:41 UTC (permalink / raw)
  To: Bartlomiej Zolnierkiewicz; +Cc: linux-kernel, Linus Torvalds, linux-fsdevel

On Thu, May 14, 2020 at 04:25:35PM +0200, Bartlomiej Zolnierkiewicz wrote:
> Thank you for in-detail explanations, for this patch:
> 
> Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> 
> Could you also please take care of adding missing checks for {get,put}_user()
> failures later?

Umm...  OK; put_user() side is trivial -  the interesting part is what to do
about get_user() failures halfway through.  Right now it treats them as
"we'd read zeroes".  On anything else I would say "screw it, memdup_user()
the damn thing on the way in and copy from there", but... Amiga has how
much RAM, again?

OTOH, from my reading of that code it does appear to be limited to
4Kb of data to copy, so it's probably OK...  Hell knows - I'm really
confused by those #ifdef __mc68000__ in there; the driver *is*
amiga-only:
obj-$(CONFIG_FB_AMIGA)            += amifb.o c2p_planar.o
config FB_AMIGA
        tristate "Amiga native chipset support"
        depends on FB && AMIGA
and AMIGA is defined only in arch/m68k/Kconfig.machine.  So how the
hell can it *not* be true?  OTOH, it looks like hand-optimized
asm equivalents of C they have in #else, so that #else might be
meant to document what's going on...

I've no idea how to test any changes to that thing - the only
m68k emulator I'm reasonably familiar with is aranym, and
that's Atari, not Amiga.  Never got around to setting up UAE...
So I can do a patch more or less blindly (memdup_user() after
it has checked the limits on height/width, then dereferencing
from copy instead of get_user()), but I won't be able to test
it.

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 11/20] amifb: get rid of pointless access_ok() calls
  2020-05-14 17:41           ` Al Viro
@ 2020-05-14 20:21             ` Geert Uytterhoeven
  0 siblings, 0 replies; 42+ messages in thread
From: Geert Uytterhoeven @ 2020-05-14 20:21 UTC (permalink / raw)
  To: Al Viro
  Cc: Bartlomiej Zolnierkiewicz, Linux Kernel Mailing List,
	Linus Torvalds, Linux FS Devel

Hi Al,

On Thu, May 14, 2020 at 7:43 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Thu, May 14, 2020 at 04:25:35PM +0200, Bartlomiej Zolnierkiewicz wrote:
> > Thank you for in-detail explanations, for this patch:
> >
> > Acked-by: Bartlomiej Zolnierkiewicz <b.zolnierkie@samsung.com>
> >
> > Could you also please take care of adding missing checks for {get,put}_user()
> > failures later?
>
> Umm...  OK; put_user() side is trivial -  the interesting part is what to do
> about get_user() failures halfway through.  Right now it treats them as
> "we'd read zeroes".  On anything else I would say "screw it, memdup_user()
> the damn thing on the way in and copy from there", but... Amiga has how
> much RAM, again?

In theory, up to 3.5 GiB ;-)
In practice, 16 MiB is already a lot (mine has 12).

> OTOH, from my reading of that code it does appear to be limited to
> 4Kb of data to copy, so it's probably OK...  Hell knows - I'm really
> confused by those #ifdef __mc68000__ in there; the driver *is*
> amiga-only:
> obj-$(CONFIG_FB_AMIGA)            += amifb.o c2p_planar.o
> config FB_AMIGA
>         tristate "Amiga native chipset support"
>         depends on FB && AMIGA
> and AMIGA is defined only in arch/m68k/Kconfig.machine.  So how the
> hell can it *not* be true?  OTOH, it looks like hand-optimized
> asm equivalents of C they have in #else, so that #else might be
> meant to document what's going on...

These #ifdefs are relics from APUS (Amiga Power-Up System), which
added a PPC board.  APUS support was killed off a long time ago,
when arch/ppc/ was still king, but these #ifdefs were missed, because
they didn't test for CONFIG_APUS.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 06/20] n_hdlc_tty_read(): remove pointless access_ok()
  2020-05-09 23:45   ` [PATCH 06/20] n_hdlc_tty_read(): remove " Al Viro
@ 2020-05-15 10:53     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 42+ messages in thread
From: Greg Kroah-Hartman @ 2020-05-15 10:53 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-kernel, Linus Torvalds, linux-fsdevel

On Sun, May 10, 2020 at 12:45:43AM +0100, Al Viro wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> only copy_to_user() is done to the address in question
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  drivers/tty/n_hdlc.c | 7 -------
>  1 file changed, 7 deletions(-)

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 18/20] usb: get rid of pointless access_ok() calls
  2020-05-09 23:45   ` [PATCH 18/20] usb: get rid of pointless access_ok() calls Al Viro
@ 2020-05-15 10:53     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 42+ messages in thread
From: Greg Kroah-Hartman @ 2020-05-15 10:53 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-kernel, Linus Torvalds, linux-fsdevel

On Sun, May 10, 2020 at 12:45:55AM +0100, Al Viro wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> in all affected cases addresses are passed only to
> copy_from()_user or copy_to_user().
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  drivers/usb/core/devices.c          | 2 --
>  drivers/usb/core/devio.c            | 9 ---------
>  drivers/usb/gadget/function/f_hid.c | 6 ------
>  3 files changed, 17 deletions(-)


Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 4/4] vmci_host: get rid of pointless access_ok()
  2020-05-09 23:45   ` [PATCH 4/4] vmci_host: " Al Viro
@ 2020-05-15 10:53     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 42+ messages in thread
From: Greg Kroah-Hartman @ 2020-05-15 10:53 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-kernel, Linus Torvalds, linux-fsdevel

On Sun, May 10, 2020 at 12:45:57AM +0100, Al Viro wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> get_user_pages_fast() doesn't need the caller to check that.
> NB: reachable only from ioctl(2) and only under USER_DS
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  drivers/misc/vmw_vmci/vmci_host.c | 2 --
>  1 file changed, 2 deletions(-)

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 07/20] nvram: drop useless access_ok()
  2020-05-09 23:45   ` [PATCH 07/20] nvram: drop useless access_ok() Al Viro
@ 2020-05-15 10:54     ` Greg Kroah-Hartman
  0 siblings, 0 replies; 42+ messages in thread
From: Greg Kroah-Hartman @ 2020-05-15 10:54 UTC (permalink / raw)
  To: Al Viro; +Cc: linux-kernel, Linus Torvalds, linux-fsdevel

On Sun, May 10, 2020 at 12:45:44AM +0100, Al Viro wrote:
> From: Al Viro <viro@zeniv.linux.org.uk>
> 
> we are using copy_to_user()/memdup_user() anyway
> 
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  drivers/char/nvram.c | 4 ----
>  1 file changed, 4 deletions(-)

Reviewed-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

^ permalink raw reply	[flat|nested] 42+ messages in thread

* Re: [PATCH 04/20] FIEMAP: don't bother with access_ok()
  2020-05-13 19:38         ` Christoph Hellwig
@ 2020-05-29 15:01           ` Al Viro
  0 siblings, 0 replies; 42+ messages in thread
From: Al Viro @ 2020-05-29 15:01 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: linux-kernel, Linus Torvalds, linux-fsdevel, tytso

On Wed, May 13, 2020 at 12:38:01PM -0700, Christoph Hellwig wrote:
> On Wed, May 13, 2020 at 08:02:07PM +0100, Al Viro wrote:
> > > https://lore.kernel.org/linux-fsdevel/20200507145924.GA28854@lst.de/T/#t
> > > 
> > > which is waiting to be picked up [1], and also has some chance for conflicts
> > > due to changes next to the access_ok.
> > > 
> > > [1] except for the first two patches, which Ted plans to send for 5.7
> > 
> > I can drop this commit, of course, it's not a prereq for anything else in there.
> > Or I could pick your series into never-rebased branch, but it would complicate
> > the life wrt ext4 tree - up to you and Ted...
> 
> I really don't care - the first two really need to go in ASAP and
> Ted promised to pick them up, but I've not seen them in linux-next
> yet.  The rest can go wherever once the first ones hit mainline.

OK, dropped

^ permalink raw reply	[flat|nested] 42+ messages in thread

end of thread, other threads:[~2020-05-29 15:01 UTC | newest]

Thread overview: 42+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-09 23:41 [PATCHES] uaccess simple access_ok() removals Al Viro
2020-05-09 23:45 ` [PATCH 01/20] dlmfs_file_write(): get rid of pointless access_ok() Al Viro
2020-05-09 23:45   ` [PATCH 02/20] fat_dir_ioctl(): hadn't needed that access_ok() for more than a decade Al Viro
2020-05-09 23:45   ` [PATCH 03/20] btrfs_ioctl_send(): don't bother with access_ok() Al Viro
2020-05-09 23:45   ` [PATCH 04/20] FIEMAP: " Al Viro
2020-05-10  7:02     ` Christoph Hellwig
2020-05-13 19:02       ` Al Viro
2020-05-13 19:38         ` Christoph Hellwig
2020-05-29 15:01           ` Al Viro
2020-05-09 23:45   ` [PATCH 05/20] tomoyo_write_control(): get rid of pointless access_ok() Al Viro
2020-05-10  0:50     ` Tetsuo Handa
2020-05-10  0:57       ` Linus Torvalds
2020-05-10  1:04         ` Tetsuo Handa
2020-05-10  3:01         ` Al Viro
2020-05-09 23:45   ` [PATCH 06/20] n_hdlc_tty_read(): remove " Al Viro
2020-05-15 10:53     ` Greg Kroah-Hartman
2020-05-09 23:45   ` [PATCH 07/20] nvram: drop useless access_ok() Al Viro
2020-05-15 10:54     ` Greg Kroah-Hartman
2020-05-09 23:45   ` [PATCH 08/20] cm4000_cs.c cmm_ioctl(): get rid of pointless access_ok() Al Viro
2020-05-09 23:45   ` [PATCH 09/20] drivers/fpga/dfl-fme-pr.c: " Al Viro
2020-05-09 23:45   ` [PATCH 10/20] drivers/fpga/dfl-afu-dma-region.c: " Al Viro
2020-05-09 23:45   ` [PATCH 11/20] amifb: get rid of pointless access_ok() calls Al Viro
2020-05-14 13:45     ` Bartlomiej Zolnierkiewicz
2020-05-14 14:07       ` Al Viro
2020-05-14 14:25         ` Bartlomiej Zolnierkiewicz
2020-05-14 17:41           ` Al Viro
2020-05-14 20:21             ` Geert Uytterhoeven
2020-05-09 23:45   ` [PATCH 12/20] omapfb: " Al Viro
2020-05-14 13:39     ` Bartlomiej Zolnierkiewicz
2020-05-09 23:45   ` [PATCH 13/20] drivers/crypto/ccp/sev-dev.c: get rid of pointless access_ok() Al Viro
2020-05-09 23:45   ` [PATCH 14/20] via-pmu: don't bother with access_ok() Al Viro
2020-05-09 23:45   ` [PATCH 15/20] drm_read(): get rid of pointless access_ok() Al Viro
2020-05-09 23:45   ` [PATCH 16/20] efi_test: " Al Viro
2020-05-09 23:45   ` [PATCH 17/20] lpfc_debugfs: " Al Viro
2020-05-09 23:45   ` [PATCH 18/20] usb: get rid of pointless access_ok() calls Al Viro
2020-05-15 10:53     ` Greg Kroah-Hartman
2020-05-09 23:45   ` [PATCH 19/20] hfi1: get rid of pointless access_ok() Al Viro
2020-05-09 23:45   ` [PATCH 4/4] vmci_host: " Al Viro
2020-05-15 10:53     ` Greg Kroah-Hartman
2020-05-10  0:34 ` [PATCHES] uaccess simple access_ok() removals Linus Torvalds
2020-05-10  3:27   ` Al Viro
2020-05-10 14:34 ` David Laight

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).