Re: [GIT PULL] SCSI fixes for 4.18-rc3

[Linus Torvalds <torvalds@linux-foundation.org>]

[-- Attachment #1: Type: text/plain, Size: 516 bytes --]

On Fri, Jul 6, 2018 at 10:22 PM James Bottomley
<James.Bottomley@hansenpartnership.com> wrote:
>
> We did discuss removing the r/w interface, but, as you say, it's been
> around for ages so it's not clear what regressions would surface if we
> did.

So since nobody else followed up on this, the attached patch is what I
was thinking of just committing.

It removes the warnings from the access check, and just puts them
(unconditionally) at the top of the read/write function instead.

Hmm?

                 Linus

[-- Attachment #2: patch.diff --]
[-- Type: text/x-patch, Size: 1955 bytes --]

 drivers/scsi/sg.c | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index cd2fdac000c9..09325b8fbc9f 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -222,18 +222,12 @@ static void sg_device_destroy(struct kref *kref);
  * This function provides protection for the legacy API by restricting the
  * calling context.
  */
-static int sg_check_file_access(struct file *filp, const char *caller)
+static int sg_check_file_access(struct file *filp)
 {
-	if (filp->f_cred != current_real_cred()) {
-		pr_err_once("%s: process %d (%s) changed security contexts after opening file descriptor, this is not allowed.\n",
-			caller, task_tgid_vnr(current), current->comm);
+	if (filp->f_cred != current_real_cred())
 		return -EPERM;
-	}
-	if (uaccess_kernel()) {
-		pr_err_once("%s: process %d (%s) called from kernel context, this is not allowed.\n",
-			caller, task_tgid_vnr(current), current->comm);
+	if (uaccess_kernel())
 		return -EACCES;
-	}
 	return 0;
 }
 
@@ -421,11 +415,14 @@ sg_read(struct file *filp, char __user *buf, size_t count, loff_t * ppos)
 	struct sg_header *old_hdr = NULL;
 	int retval = 0;
 
+	pr_err_once("process %d (%s) does direct read on /dev/sg",
+		task_tgid_vnr(current), current->comm);
+
 	/*
 	 * This could cause a response to be stranded. Close the associated
 	 * file descriptor to free up any resources being held.
 	 */
-	retval = sg_check_file_access(filp, __func__);
+	retval = sg_check_file_access(filp);
 	if (retval)
 		return retval;
 
@@ -618,7 +615,10 @@ sg_write(struct file *filp, const char __user *buf, size_t count, loff_t * ppos)
 	unsigned char cmnd[SG_MAX_CDB_SIZE];
 	int retval;
 
-	retval = sg_check_file_access(filp, __func__);
+	pr_err_once("process %d (%s) does direct write on /dev/sg",
+		task_tgid_vnr(current), current->comm);
+
+	retval = sg_check_file_access(filp);
 	if (retval)
 		return retval;