All of lore.kernel.org
 help / color / mirror / Atom feed
From: Todd Poynor <toddpoynor@gmail.com>
To: Rob Springer <rspringer@google.com>,
	John Joseph <jnjoseph@google.com>,
	Ben Chan <benchan@chromium.org>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Cc: Zhongze Hu <frankhu@chromium.org>, Simon Que <sque@chromium.org>,
	Dmitry Torokhov <dtor@chromium.org>,
	Guenter Roeck <groeck@chromium.org>,
	devel@driverdev.osuosl.org, linux-kernel@vger.kernel.org,
	Todd Poynor <toddpoynor@google.com>
Subject: [PATCH 16/20] staging: gasket: always allow root open for write
Date: Thu, 19 Jul 2018 20:49:16 -0700	[thread overview]
Message-ID: <20180720034920.77320-17-toddpoynor@gmail.com> (raw)
In-Reply-To: <20180720034920.77320-1-toddpoynor@gmail.com>

From: Todd Poynor <toddpoynor@google.com>

Always allow root to open device for writing.

Drop special-casing of ioctl permissions for root vs. owner.

Convert to bool types as appropriate.

Reported-by: Dmitry Torokhov <dtor@chromium.org>
Signed-off-by: Zhongze Hu <frankhu@chromium.org>
Signed-off-by: Todd Poynor <toddpoynor@google.com>
---
 drivers/staging/gasket/apex_driver.c  | 15 ++++----------
 drivers/staging/gasket/gasket_core.c  |  8 ++++---
 drivers/staging/gasket/gasket_ioctl.c | 30 +++++++++++++--------------
 3 files changed, 23 insertions(+), 30 deletions(-)

diff --git a/drivers/staging/gasket/apex_driver.c b/drivers/staging/gasket/apex_driver.c
index a01b1f2b827ea..4c00f3609f081 100644
--- a/drivers/staging/gasket/apex_driver.c
+++ b/drivers/staging/gasket/apex_driver.c
@@ -140,7 +140,7 @@ static int apex_reset(struct gasket_dev *gasket_dev, uint type);
 
 static int apex_get_status(struct gasket_dev *gasket_dev);
 
-static uint apex_ioctl_check_permissions(struct file *file, uint cmd);
+static bool apex_ioctl_check_permissions(struct file *file, uint cmd);
 
 static long apex_ioctl(struct file *file, uint cmd, ulong arg);
 
@@ -625,18 +625,11 @@ static bool is_gcb_in_reset(struct gasket_dev *gasket_dev)
  * @file: File pointer from ioctl.
  * @cmd: ioctl command.
  *
- * Returns 1 if the current user may execute this ioctl, and 0 otherwise.
+ * Returns true if the current user may execute this ioctl, and false otherwise.
  */
-static uint apex_ioctl_check_permissions(struct file *filp, uint cmd)
+static bool apex_ioctl_check_permissions(struct file *filp, uint cmd)
 {
-	struct gasket_dev *gasket_dev = filp->private_data;
-	int root = capable(CAP_SYS_ADMIN);
-	int is_owner = gasket_dev->dev_info.ownership.is_owned &&
-		       current->tgid == gasket_dev->dev_info.ownership.owner;
-
-	if (root || is_owner)
-		return 1;
-	return 0;
+	return !!(filp->f_mode & FMODE_WRITE);
 }
 
 /*
diff --git a/drivers/staging/gasket/gasket_core.c b/drivers/staging/gasket/gasket_core.c
index ba48a379b0ada..254fb392c05c1 100644
--- a/drivers/staging/gasket/gasket_core.c
+++ b/drivers/staging/gasket/gasket_core.c
@@ -1072,6 +1072,7 @@ static int gasket_open(struct inode *inode, struct file *filp)
 	char task_name[TASK_COMM_LEN];
 	struct gasket_cdev_info *dev_info =
 	    container_of(inode->i_cdev, struct gasket_cdev_info, cdev);
+	int is_root = capable(CAP_SYS_ADMIN);
 
 	gasket_dev = dev_info->gasket_dev_ptr;
 	driver_desc = gasket_dev->internal_desc->driver_desc;
@@ -1085,7 +1086,7 @@ static int gasket_open(struct inode *inode, struct file *filp)
 		"Attempting to open with tgid %u (%s) (f_mode: 0%03o, "
 		"fmode_write: %d is_root: %u)",
 		current->tgid, task_name, filp->f_mode,
-		(filp->f_mode & FMODE_WRITE), capable(CAP_SYS_ADMIN));
+		(filp->f_mode & FMODE_WRITE), is_root);
 
 	/* Always allow non-writing accesses. */
 	if (!(filp->f_mode & FMODE_WRITE)) {
@@ -1099,8 +1100,9 @@ static int gasket_open(struct inode *inode, struct file *filp)
 		gasket_dev, "Current owner open count (owning tgid %u): %d.",
 		ownership->owner, ownership->write_open_count);
 
-	/* Opening a node owned by another TGID is an error (even root.) */
-	if (ownership->is_owned && ownership->owner != current->tgid) {
+	/* Opening a node owned by another TGID is an error (unless root) */
+	if (ownership->is_owned && ownership->owner != current->tgid &&
+	    !is_root) {
 		gasket_log_error(
 			gasket_dev,
 			"Process %u is opening a node held by %u.",
diff --git a/drivers/staging/gasket/gasket_ioctl.c b/drivers/staging/gasket/gasket_ioctl.c
index d0142ed048a65..8fd44979fe713 100644
--- a/drivers/staging/gasket/gasket_ioctl.c
+++ b/drivers/staging/gasket/gasket_ioctl.c
@@ -22,7 +22,7 @@
 #define trace_gasket_ioctl_config_coherent_allocator(x, ...)
 #endif
 
-static uint gasket_ioctl_check_permissions(struct file *filp, uint cmd);
+static bool gasket_ioctl_check_permissions(struct file *filp, uint cmd);
 static int gasket_set_event_fd(struct gasket_dev *dev, ulong arg);
 static int gasket_read_page_table_size(
 	struct gasket_dev *gasket_dev, ulong arg);
@@ -167,12 +167,13 @@ long gasket_is_supported_ioctl(uint cmd)
  * @filp: File structure pointer describing this node usage session.
  * @cmd: ioctl number to handle.
  *
- * Standard permissions checker.
+ * Check permissions for Gasket ioctls.
+ * Returns true if the file opener may execute this ioctl, or false otherwise.
  */
-static uint gasket_ioctl_check_permissions(struct file *filp, uint cmd)
+static bool gasket_ioctl_check_permissions(struct file *filp, uint cmd)
 {
-	uint alive, root, device_owner;
-	fmode_t read, write;
+	bool alive;
+	bool read, write;
 	struct gasket_dev *gasket_dev = (struct gasket_dev *)filp->private_data;
 
 	alive = (gasket_dev->status == GASKET_STATUS_ALIVE);
@@ -183,36 +184,33 @@ static uint gasket_ioctl_check_permissions(struct file *filp, uint cmd)
 			alive, gasket_dev->status);
 	}
 
-	root = capable(CAP_SYS_ADMIN);
-	read = filp->f_mode & FMODE_READ;
-	write = filp->f_mode & FMODE_WRITE;
-	device_owner = (gasket_dev->dev_info.ownership.is_owned &&
-			current->tgid == gasket_dev->dev_info.ownership.owner);
+	read = !!(filp->f_mode & FMODE_READ);
+	write = !!(filp->f_mode & FMODE_WRITE);
 
 	switch (cmd) {
 	case GASKET_IOCTL_RESET:
 	case GASKET_IOCTL_CLEAR_INTERRUPT_COUNTS:
-		return root || (write && device_owner);
+		return write;
 
 	case GASKET_IOCTL_PAGE_TABLE_SIZE:
 	case GASKET_IOCTL_SIMPLE_PAGE_TABLE_SIZE:
 	case GASKET_IOCTL_NUMBER_PAGE_TABLES:
-		return root || read;
+		return read;
 
 	case GASKET_IOCTL_PARTITION_PAGE_TABLE:
 	case GASKET_IOCTL_CONFIG_COHERENT_ALLOCATOR:
-		return alive && (root || (write && device_owner));
+		return alive && write;
 
 	case GASKET_IOCTL_MAP_BUFFER:
 	case GASKET_IOCTL_UNMAP_BUFFER:
-		return alive && (root || (write && device_owner));
+		return alive && write;
 
 	case GASKET_IOCTL_CLEAR_EVENTFD:
 	case GASKET_IOCTL_SET_EVENTFD:
-		return alive && (root || (write && device_owner));
+		return alive && write;
 	}
 
-	return 0; /* unknown permissions */
+	return false; /* unknown permissions */
 }
 
 /*
-- 
2.18.0.233.g985f88cf7e-goog


  parent reply	other threads:[~2018-07-20  3:50 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-07-20  3:49 [PATCH 00/20 v4] staging: gasket: sundry fixes and fixups Todd Poynor
2018-07-20  3:49 ` [PATCH 01/20] staging: gasket: allow compile for ARM64 in Kconfig Todd Poynor
2018-07-20  3:49 ` [PATCH 02/20] staging: gasket: gasket_enable_dev remove unnecessary variable Todd Poynor
2018-07-21  6:48   ` Greg Kroah-Hartman
2018-07-20  3:49 ` [PATCH 03/20] staging: gasket: remove code for no physical device Todd Poynor
2018-07-20  3:49 ` [PATCH 04/20] staging: gasket: fix class create bug handling Todd Poynor
2018-07-20  3:49 ` [PATCH 05/20] staging: gasket: remove unnecessary code in coherent allocator Todd Poynor
2018-07-20  3:49 ` [PATCH 06/20] staging: gasket: don't treat no device reset callback as an error Todd Poynor
2018-07-20  3:49 ` [PATCH 07/20] staging: gasket: gasket_mmap return error instead of valid BAR index Todd Poynor
2018-07-20  3:49 ` [PATCH 08/20] staging: gasket: apex_clock_gating simplify logic, reduce indentation Todd Poynor
2018-07-20  3:49 ` [PATCH 09/20] staging: gasket: gasket page table functions use bool return type Todd Poynor
2018-07-20  3:49 ` [PATCH 10/20] staging: gasket: remove else clause after return in if clause Todd Poynor
2018-07-20  3:49 ` [PATCH 11/20] staging: gasket: fix comment syntax in apex.h Todd Poynor
2018-07-20  3:49 ` [PATCH 12/20] staging: gasket: remove unnecessary parens in page table code Todd Poynor
2018-07-20  3:49 ` [PATCH 13/20] staging: gasket: gasket_mmap use PAGE_MASK Todd Poynor
2018-07-20  3:49 ` [PATCH 14/20] staging: gasket: remove extra parens in gasket_write_mappable_regions Todd Poynor
2018-07-20  3:49 ` [PATCH 15/20] staging: gasket: fix multi-line comment syntax in gasket_core.h Todd Poynor
2018-07-20  3:49 ` Todd Poynor [this message]
2018-07-20  3:49 ` [PATCH 17/20] staging: gasket: top ioctl handler add __user annotations Todd Poynor
2018-07-20  3:49 ` [PATCH 18/20] staging: gasket: apex ioctl " Todd Poynor
2018-07-20  3:49 ` [PATCH 19/20] staging: gasket: common ioctl dispatcher " Todd Poynor
2018-07-20  3:49 ` [PATCH 20/20] staging: gasket: common ioctls " Todd Poynor
2018-07-21  6:51 ` [PATCH 00/20 v4] staging: gasket: sundry fixes and fixups Greg Kroah-Hartman
  -- strict thread matches above, loose matches on Subject: below --
2018-07-21 12:56 [PATCH 00/14] staging: gasket: assorted cleanups Todd Poynor
2018-07-21 12:57 ` [PATCH 16/20] staging: gasket: always allow root open for write Todd Poynor

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180720034920.77320-17-toddpoynor@gmail.com \
    --to=toddpoynor@gmail.com \
    --cc=benchan@chromium.org \
    --cc=devel@driverdev.osuosl.org \
    --cc=dtor@chromium.org \
    --cc=frankhu@chromium.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=groeck@chromium.org \
    --cc=jnjoseph@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rspringer@google.com \
    --cc=sque@chromium.org \
    --cc=toddpoynor@google.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.