Only a smaller percentage of SCSI commands should require a sense
buffer. Allocate as needed and delete as soon as possible.

Signed-off-by: Douglas Gilbert <dgilb...@interlog.com>
---
 drivers/scsi/sg.c | 35 ++++++++++++++++++++++-------------
 1 file changed, 22 insertions(+), 13 deletions(-)

diff --git a/drivers/scsi/sg.c b/drivers/scsi/sg.c
index 72ce51b3198c..a58875198c16 100644
--- a/drivers/scsi/sg.c
+++ b/drivers/scsi/sg.c
@@ -852,20 +852,21 @@ sg_copy_sense(struct sg_request *srp)
            (driver_byte(srp->rq_result) & DRIVER_SENSE)) {
                int sb_len = min_t(int, SCSI_SENSE_BUFFERSIZE, srp->sense_len);
                int mx_sb_len;
+               u8 *sbp = srp->sense_bp;
                void __user *up;
 
+               srp->sense_bp = NULL;
                up = (void __user *)srp->s_hdr3.sbp;
                mx_sb_len = srp->s_hdr3.mx_sb_len;
-               if (up && mx_sb_len > 0 && srp->sense_bp) {
+               if (up && mx_sb_len > 0 && sbp) {
                        sb_len = min_t(int, sb_len, mx_sb_len);
                        /* Additional sense length field */
-                       sb_len_wr = 8 + (int)srp->sense_bp[7];
+                       sb_len_wr = 8 + (int)sbp[7];
                        sb_len_wr = min_t(int, sb_len, sb_len_wr);
-                       if (copy_to_user(up, srp->sense_bp, sb_len_wr))
+                       if (copy_to_user(up, sbp, sb_len_wr))
                                sb_len_wr = -EFAULT;
                }
-               kfree(srp->sense_bp);
-               srp->sense_bp = NULL;
+               kfree(sbp);
        }
        return sb_len_wr;
 }
@@ -972,12 +973,9 @@ sg_rd_v1v2(void __user *buf, int count, struct sg_fd *sfp,
        h2p->driver_status = driver_byte(rq_result);
        if ((CHECK_CONDITION & status_byte(rq_result)) ||
            (DRIVER_SENSE & driver_byte(rq_result))) {
-               if (srp->sense_bp) {
+               if (srp->sense_bp)
                        memcpy(h2p->sense_buffer, srp->sense_bp,
                               sizeof(h2p->sense_buffer));
-                       kfree(srp->sense_bp);
-                       srp->sense_bp = NULL;
-               }
        }
        switch (host_byte(rq_result)) {
        /*
@@ -1013,17 +1011,22 @@ sg_rd_v1v2(void __user *buf, int count, struct sg_fd 
*sfp,
 
        /* Now copy the result back to the user buffer.  */
        if (count >= SZ_SG_HEADER) {
-               if (copy_to_user(buf, h2p, SZ_SG_HEADER))
-                       return -EFAULT;
+               if (copy_to_user(buf, h2p, SZ_SG_HEADER)) {
+                       res = -EFAULT;
+                       goto fini;
+               }
                buf += SZ_SG_HEADER;
                if (count > h2p->reply_len)
                        count = h2p->reply_len;
                if (count > SZ_SG_HEADER) {
-                       if (sg_rd_append(srp, buf, count - SZ_SG_HEADER))
-                               return -EFAULT;
+                       if (sg_rd_append(srp, buf, count - SZ_SG_HEADER)) {
+                               res = -EFAULT;
+                               goto fini;
+                       }
                }
        } else
                res = (h2p->result == 0) ? 0 : -EIO;
+fini:
        atomic_set(&srp->rq_st, SG_RS_DONE_RD);
        sg_finish_scsi_blk_rq(srp);
        sg_deact_request(sfp, srp);
@@ -2991,6 +2994,7 @@ sg_deact_request(struct sg_fd *sfp, struct sg_request 
*srp)
        bool on_fl = false;
        int dlen, buflen;
        unsigned long iflags;
+       u8 *sbp;
        struct sg_request *t_srp;
        struct sg_scatter_hold *schp;
        const char *cp = "head";
@@ -2999,8 +3003,11 @@ sg_deact_request(struct sg_fd *sfp, struct sg_request 
*srp)
                return;
        schp = &srp->sgat_h;    /* make sure it is own data buffer */
        spin_lock_irqsave(&sfp->rq_list_lock, iflags);
+       sbp = srp->sense_bp;
+       srp->sense_bp = NULL;
        atomic_set(&srp->rq_st, SG_RS_BUSY);
        list_del_rcu(&srp->rq_entry);
+       kfree(sbp);     /* maybe orphaned req, thus never read */
        /*
         * N.B. sg_request object is not de-allocated (freed). The contents
         * of the rq_list and rq_fl lists are de-allocated (freed) when
@@ -3145,6 +3152,7 @@ sg_remove_sfp_usercontext(struct work_struct *work)
                list_del(&srp->rq_entry);
                if (srp->sgat_h.buflen > 0)
                        sg_remove_sgat(srp);
+               kfree(srp->sense_bp);   /* abnormal close: device detached */
                SG_LOG(6, sdp, "%s:%s%p --\n", __func__, cp, srp);
                kfree(srp);
        }
@@ -3156,6 +3164,7 @@ sg_remove_sfp_usercontext(struct work_struct *work)
                list_del(&srp->fl_entry);
                if (srp->sgat_h.buflen > 0)
                        sg_remove_sgat(srp);
+               kfree(srp->sense_bp);
                SG_LOG(6, sdp, "%s: fl%s%p --\n", __func__, cp, srp);
                kfree(srp);
        }
-- 
2.17.1

Reply via email to