From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-kernel-owner@vger.kernel.org>
Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand
        id S1753549AbcHVUZJ (ORCPT <rfc822;w@1wt.eu>);
        Mon, 22 Aug 2016 16:25:09 -0400
Received: from mout.web.de ([212.227.17.12]:57231 "EHLO mout.web.de"
        rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP
        id S1751752AbcHVUZH (ORCPT <rfc822;linux-kernel@vger.kernel.org>);
        Mon, 22 Aug 2016 16:25:07 -0400
Subject: Re: [PATCH 1/7] aacraid: Use memdup_user() rather than duplicating
 its implementation
To: David Carroll <david.carroll@microsemi.com>,
        "linux-scsi@vger.kernel.org" <linux-scsi@vger.kernel.org>,
        dl-esc-Aacraid Linux Driver <aacraid@microsemi.com>,
        "James E. J. Bottomley" <jejb@linux.vnet.ibm.com>,
        "Martin K. Petersen" <martin.petersen@oracle.com>
References: <566ABCD9.1060404@users.sourceforge.net>
 <40d8607f-3934-c31f-3791-ef6a67d65d45@users.sourceforge.net>
 <23c56a66-74a6-6033-f209-ac3e4ba83e61@users.sourceforge.net>
 <423FD6710FB8FB4F8728F93591889F9A15FA8E64@avsrvexchmbx1.microsemi.net>
Cc: LKML <linux-kernel@vger.kernel.org>,
        "kernel-janitors@vger.kernel.org" <kernel-janitors@vger.kernel.org>,
        Julia Lawall <julia.lawall@lip6.fr>
From: SF Markus Elfring <elfring@users.sourceforge.net>
Message-ID: <b61d1eef-35c6-5e8c-a342-db312e27cb1f@users.sourceforge.net>
Date: Mon, 22 Aug 2016 22:23:57 +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: <423FD6710FB8FB4F8728F93591889F9A15FA8E64@avsrvexchmbx1.microsemi.net>
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: 7bit
X-Provags-ID: V03:K0:1+Nbm2aQz7jUbKCK0UrSFmIRxMVDEeGCPHus1n3LneVE94hMD62
 mrS8dfLramaGhzucuvg6Xi75h0Ak6x4Zu85tP8nmj6VP2Pbb1aESFbfG/Armg5y3Fz3hz99
 h46+mfjrcRENcx5gsoBbLOHx0QS9ftDXaIV1tLnESDob/h8vzTKgA7X1luK8Psn/Hs6SLK/
 bdhlsBVZtdWUO5y4dpLeQ==
X-UI-Out-Filterresults: notjunk:1;V01:K0:9tbyu3/rHuo=:wSj3HDU1QXJN1OU6iFmUt6
 SPHk7CceaWJfnQrn5xa2itQCf39bj/HL6Sqoi54KstX1BdPwG0uV33Z6eLXEGoaQSlUbgYTSp
 u7HQ+lRCoSd5j2bPXATdxJASm0elZL6J94zvtGc6FFqraSFfzjtAzDTR9jBwPs+5deZlPLkrT
 pEcN6VjjXl0BtGIijGtinetfTa5F6Ip6mnGZReLyv0nBWiRdrskXFrCAfeuY1zKtWBdUsuZ7w
 ESTPRER2PQoVhsGZ7Iw6dIOk5xJ90oP3W4Dpsj4cQKr64hxH9Y3RivevZUhl5K7oRNMszf+s7
 QYydjqhSyQggjTan7CL8FTokCtM16aFy+LuOgeNBQXwHj5JzK9KYPznT+YvbEPZSHqkCtybrZ
 RVCVAB7vEsONmAND+VyhTqQAyf35mauU4+dJtkAdV/c8oJjL/pKYBNn3yRq9UFarDKVWJ8Qkx
 oJOHrgWacayG5GrABnsHCrlGsF1HSXQnpdM4CKfJWTwEgd3/7vycKVgo1xHEeO529t6G2UBWo
 GIcv0tu0l7m4/NVwsq2PKu0pIqPrXBLI6c/AfMH35bNhqNYZ9MK8BDXkDvY/zw/BHTm3j3dfq
 dMrP9v4Swh70xvjRlZ2+6hn4qa06CqFXHwPwNvA+n5niHAuVZzJrRpb9x1+guvhBE+kpSxqhS
 /R8oSwefTnl6FOoFVGEn5LjKEfIOot/50oIZi4na/aS/Au8hQKgPmxHoMARIH59eq1KK0O3yw
 2be8e54PHgi7XFbQVSvfpfZa0aLTY5z1+QQ0r7Fem9PexOogdbC/mMl0rTVQRB7WuZDlWsCN1
 YR70cV6
Sender: linux-kernel-owner@vger.kernel.org
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

>> @@ -526,15 +526,9 @@ static int aac_send_raw_srb(struct aac_dev* dev, void
>> __user * arg)
>>                 goto cleanup;
>>         }
>>
>> -       user_srbcmd = kmalloc(fibsize, GFP_KERNEL);
>> -       if (!user_srbcmd) {
>> -               dprintk((KERN_DEBUG"aacraid: Could not make a copy of the srb\n"));
>> -               rcode = -ENOMEM;
>> -               goto cleanup;
>> -       }
>> -       if(copy_from_user(user_srbcmd, user_srb,fibsize)){
>> -               dprintk((KERN_DEBUG"aacraid: Could not copy srb from user\n"));
>> -               rcode = -EFAULT;
>> +       user_srbcmd = memdup_user(user_srb, fibsize);
>> +       if (IS_ERR(user_srbcmd)) {
>> +               rcode = PTR_ERR(user_srbcmd);
>>                 goto cleanup;
>>         }
>>
>> --
> 
> Hi Markus,
> 
> Patch 2/7 should precede Patch 1/7, as falling into kfree() would not look pretty.

Do you eventually prefer that this source code adjustment should be combined with
the update suggestion "[2/7] aacraid: One function call less in aac_send_raw_srb()
after error detection" in a single update step?

Regards,
Markus