From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752765AbcHUHVG (ORCPT ); Sun, 21 Aug 2016 03:21:06 -0400 Received: from mout.web.de ([212.227.17.11]:62593 "EHLO mout.web.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751121AbcHUHVE (ORCPT ); Sun, 21 Aug 2016 03:21:04 -0400 Subject: [PATCH 2/7] aacraid: One function call less in aac_send_raw_srb() after error detection To: linux-scsi@vger.kernel.org, aacraid@microsemi.com, "James E. J. Bottomley" , "Martin K. Petersen" References: <566ABCD9.1060404@users.sourceforge.net> <40d8607f-3934-c31f-3791-ef6a67d65d45@users.sourceforge.net> Cc: LKML , kernel-janitors@vger.kernel.org, Julia Lawall From: SF Markus Elfring Message-ID: <1e07bef4-1bdd-f2af-b316-6d577c616d99@users.sourceforge.net> Date: Sun, 21 Aug 2016 09:20:45 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2 MIME-Version: 1.0 In-Reply-To: <40d8607f-3934-c31f-3791-ef6a67d65d45@users.sourceforge.net> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-Provags-ID: V03:K0:qCWrDICATOBk1ugxL1KIz8fytYV+d5+kHeDOupwGJGcKr1oGEXF saU81jonCiPUCl956YNYtVyPrymKQBulf9PQAL/2A2elO2S1TfTdP5BR7mWZcwIc+/WzV/A D/yTB0WCNFhabVWcWT36uPQKNjxQUe/ZP40nF/MlasmgK+wx62VDPT7sMaYWC0fFL/Ssg6N toDCo1gJ38ttkM9qi4b0Q== X-UI-Out-Filterresults: notjunk:1;V01:K0:nAmbIdeGIy8=:icDkvILmOso/g7rtnvKrfu IUTnPRKDLNlEWnifJ8+E1qZPeXwXVsjtvSk9rrZhL2b3B6Z5ced1swXey+NmkU+okY8NJHDjl CnWKEQ8oVvzi7oyzS5wNsbVXVMFvJqnTonjCEjC36yIoZD7bOj2wtDeXSiaHlDIQH9t96AlVq 6ekLItdEQO5SSGs2BRGBjCTYnkbYkPvH42eMQl2+J68uWfxGTS//3RQP9tSDiqqQZheds//2w iyf3tK0Z1Ba/XCoUewrgHMLvYmZaSo2A/Wnojo4vChwYF9VjjRhXzx1WOo846Dk6LHHEDTy8K 6lv3ecgEFGZPF5dglpO2acw093qgWpkt6QRZfJ4yB+WRh8O0l609YWLRgN2cnrZMTMqxlx2oE Yk7GiZbpeMOKVjYWCM6PofkJVaP7QqhjoLlGIBDCI00FNnm0QWoZSixOZ4qVNBO7PQL0TxRsR aGVPF85aqOpEC2BdLGG1b+S2TaUK5C7HiMH3v3iECaXTLeI1DMcYRu+HLzmvYMcqr7z4ROUOI Oyntaz1cnnO+wc+J9/JcgCPxZsYEsWevnB+JRDpGU+wVMAWFgSJowvk0AC0kJYgw5rDc1JGNu NVBPeCVUoEu6i0tIoiZBUPLfa1bLnkEIbcJGlvvW8Xo73JP936UrR3Pt7Y0bIDMqDPriS7Zd8 xKgc/rpZjZbqbbIl54xHpvJky80+YrQyuCwn8+RA/3hI9+MjDS1dBh27t1+0aaiX9UUOh3/NS Ji9trwqThjpgT+S3qg3Wx7MCIxTlk4pZst2S8Zo1jezBe9xvQjJV6sCwhP0uHuhL23C81RT9b TliSM6k Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org >>From e8187662ee30aab709a260c72fb86c51673f8e0d Mon Sep 17 00:00:00 2001 From: Markus Elfring Date: Sat, 20 Aug 2016 20:40:47 +0200 Subject: [PATCH 2/7] aacraid: One function call less in aac_send_raw_srb() after error detection The kfree() function was called in a few cases by the aac_send_raw_srb() function during error handling even if the variable "user_srbcmd" contained eventually an inappropriate pointer value. Signed-off-by: Markus Elfring --- drivers/scsi/aacraid/commctrl.c | 49 ++++++++++++++++++++--------------------- 1 file changed, 24 insertions(+), 25 deletions(-) diff --git a/drivers/scsi/aacraid/commctrl.c b/drivers/scsi/aacraid/commctrl.c index 1af3084..6dcdf91 100644 --- a/drivers/scsi/aacraid/commctrl.c +++ b/drivers/scsi/aacraid/commctrl.c @@ -517,19 +517,19 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg) if(copy_from_user(&fibsize, &user_srb->count,sizeof(u32))){ dprintk((KERN_DEBUG"aacraid: Could not copy data size from user\n")); rcode = -EFAULT; - goto cleanup; + goto free_sg_list; } if ((fibsize < (sizeof(struct user_aac_srb) - sizeof(struct user_sgentry))) || (fibsize > (dev->max_fib_size - sizeof(struct aac_fibhdr)))) { rcode = -EINVAL; - goto cleanup; + goto free_sg_list; } user_srbcmd = memdup_user(user_srb, fibsize); if (IS_ERR(user_srbcmd)) { rcode = PTR_ERR(user_srbcmd); - goto cleanup; + goto free_sg_list; } user_reply = arg+fibsize; @@ -564,7 +564,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg) dprintk((KERN_DEBUG"aacraid: too many sg entries %d\n", le32_to_cpu(srbcmd->sg.count))); rcode = -EINVAL; - goto cleanup; + goto free_user_srbcmd; } actual_fibsize = sizeof(struct aac_srb) - sizeof(struct sgentry) + ((user_srbcmd->sg.count & 0xff) * sizeof(struct sgentry)); @@ -580,12 +580,12 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg) sizeof(struct aac_srb), sizeof(struct sgentry), sizeof(struct sgentry64), fibsize)); rcode = -EINVAL; - goto cleanup; + goto free_user_srbcmd; } if ((data_dir == DMA_NONE) && user_srbcmd->sg.count) { dprintk((KERN_DEBUG"aacraid: SG with no direction specified in Raw SRB command\n")); rcode = -EINVAL; - goto cleanup; + goto free_user_srbcmd; } byte_count = 0; if (dev->adapter_info.options & AAC_OPT_SGMAP_HOST64) { @@ -606,7 +606,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg) (dev->scsi_host_ptr->max_sectors << 9) : 65536)) { rcode = -EINVAL; - goto cleanup; + goto free_user_srbcmd; } /* Does this really need to be GFP_DMA? */ p = kmalloc(upsg->sg[i].count,GFP_KERNEL|__GFP_DMA); @@ -614,7 +614,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg) dprintk((KERN_DEBUG"aacraid: Could not allocate SG buffer - size = %d buffer number %d of %d\n", upsg->sg[i].count,i,upsg->count)); rcode = -ENOMEM; - goto cleanup; + goto free_user_srbcmd; } addr = (u64)upsg->sg[i].addr[0]; addr += ((u64)upsg->sg[i].addr[1]) << 32; @@ -626,7 +626,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg) if(copy_from_user(p,sg_user[i],upsg->sg[i].count)){ dprintk((KERN_DEBUG"aacraid: Could not copy sg data from user\n")); rcode = -EFAULT; - goto cleanup; + goto free_user_srbcmd; } } addr = pci_map_single(dev->pdev, p, upsg->sg[i].count, data_dir); @@ -644,7 +644,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg) if (!usg) { dprintk((KERN_DEBUG"aacraid: Allocation error in Raw SRB command\n")); rcode = -ENOMEM; - goto cleanup; + goto free_user_srbcmd; } actual_fibsize = actual_fibsize64; @@ -658,7 +658,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg) 65536)) { kfree(usg); rcode = -EINVAL; - goto cleanup; + goto free_user_srbcmd; } /* Does this really need to be GFP_DMA? */ p = kmalloc(usg->sg[i].count,GFP_KERNEL|__GFP_DMA); @@ -667,7 +667,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg) usg->sg[i].count,i,usg->count)); kfree(usg); rcode = -ENOMEM; - goto cleanup; + goto free_user_srbcmd; } sg_user[i] = (void __user *)(uintptr_t)usg->sg[i].addr; sg_list[i] = p; // save so we can clean up later @@ -678,7 +678,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg) kfree (usg); dprintk((KERN_DEBUG"aacraid: Could not copy sg data from user\n")); rcode = -EFAULT; - goto cleanup; + goto free_user_srbcmd; } } addr = pci_map_single(dev->pdev, p, usg->sg[i].count, data_dir); @@ -711,7 +711,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg) (dev->scsi_host_ptr->max_sectors << 9) : 65536)) { rcode = -EINVAL; - goto cleanup; + goto free_user_srbcmd; } /* Does this really need to be GFP_DMA? */ p = kmalloc(usg->sg[i].count,GFP_KERNEL|__GFP_DMA); @@ -719,7 +719,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg) dprintk((KERN_DEBUG"aacraid: Could not allocate SG buffer - size = %d buffer number %d of %d\n", usg->sg[i].count,i,usg->count)); rcode = -ENOMEM; - goto cleanup; + goto free_user_srbcmd; } addr = (u64)usg->sg[i].addr[0]; addr += ((u64)usg->sg[i].addr[1]) << 32; @@ -731,7 +731,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg) if(copy_from_user(p,sg_user[i],usg->sg[i].count)){ dprintk((KERN_DEBUG"aacraid: Could not copy sg data from user\n")); rcode = -EFAULT; - goto cleanup; + goto free_user_srbcmd; } } addr = pci_map_single(dev->pdev, p, usg->sg[i].count, data_dir); @@ -750,14 +750,14 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg) (dev->scsi_host_ptr->max_sectors << 9) : 65536)) { rcode = -EINVAL; - goto cleanup; + goto free_user_srbcmd; } p = kmalloc(upsg->sg[i].count, GFP_KERNEL); if (!p) { dprintk((KERN_DEBUG"aacraid: Could not allocate SG buffer - size = %d buffer number %d of %d\n", upsg->sg[i].count, i, upsg->count)); rcode = -ENOMEM; - goto cleanup; + goto free_user_srbcmd; } sg_user[i] = (void __user *)(uintptr_t)upsg->sg[i].addr; sg_list[i] = p; // save so we can clean up later @@ -768,7 +768,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg) upsg->sg[i].count)) { dprintk((KERN_DEBUG"aacraid: Could not copy sg data from user\n")); rcode = -EFAULT; - goto cleanup; + goto free_user_srbcmd; } } addr = pci_map_single(dev->pdev, p, @@ -788,13 +788,13 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg) } if (status == -ERESTARTSYS) { rcode = -ERESTARTSYS; - goto cleanup; + goto free_user_srbcmd; } if (status != 0){ dprintk((KERN_DEBUG"aacraid: Could not send raw srb fib to hba\n")); rcode = -ENXIO; - goto cleanup; + goto free_user_srbcmd; } if (flags & SRB_DataIn) { @@ -806,7 +806,7 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg) if(copy_to_user(sg_user[i], sg_list[i], byte_count)){ dprintk((KERN_DEBUG"aacraid: Could not copy sg data to user\n")); rcode = -EFAULT; - goto cleanup; + goto free_user_srbcmd; } } @@ -816,11 +816,10 @@ static int aac_send_raw_srb(struct aac_dev* dev, void __user * arg) if(copy_to_user(user_reply,reply,sizeof(struct aac_srb_reply))){ dprintk((KERN_DEBUG"aacraid: Could not copy reply to user\n")); rcode = -EFAULT; - goto cleanup; } - -cleanup: +free_user_srbcmd: kfree(user_srbcmd); +free_sg_list: for(i=0; i <= sg_indx; i++){ kfree(sg_list[i]); } -- 2.9.3