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

Linus Torvalds
2018-07-10T00:41:56Z

[-- 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;