Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <db5a879e-5e2f-24f0-c0fb-4971679a45cb@os.amperecomputing.com>
Date:   Thu, 17 Mar 2022 14:45:19 +0700
User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:91.0)
 Gecko/20100101 Thunderbird/91.6.0
Subject: Re: [PATCH v6 3/4] ipmi: ssif_bmc: Return -EFAULT if copy_from_user()
 fails
Content-Language: en-US
To:     Dan Carpenter <dan.carpenter@oracle.com>,
        Wolfram Sang <wsa@kernel.org>, Corey Minyard <minyard@acm.org>,
        Rob Herring <robh+dt@kernel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski@canonical.com>,
        Joel Stanley <joel@jms.id.au>,
        Andrew Jeffery <andrew@aj.id.au>,
        Brendan Higgins <brendanhiggins@google.com>,
        Benjamin Herrenschmidt <benh@kernel.crashing.org>,
        openipmi-developer@lists.sourceforge.net,
        devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
        linux-aspeed@lists.ozlabs.org, linux-kernel@vger.kernel.org,
        linux-i2c@vger.kernel.org, openbmc@lists.ozlabs.org,
        Open Source Submission <patches@amperecomputing.com>,
        Phong Vo <phong@os.amperecomputing.com>,
        "Thang Q . Nguyen" <thang@os.amperecomputing.com>
References: <20220310114119.13736-1-quan@os.amperecomputing.com>
 <20220310114119.13736-4-quan@os.amperecomputing.com>
 <YirzJj/BBUQTbKS3@ninjato>
From:   Quan Nguyen <quan@os.amperecomputing.com>
In-Reply-To: <YirzJj/BBUQTbKS3@ninjato>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?N21CV2NwWVZTbGZQNDBGeFJ4SFBHOFNMSnZsczFJaktMdkZiamxoMlBNK2p3?=
 =?utf-8?B?UXpyaU1rZ1g0Z1RWUDhZbnZaL1RaRUcrQVU5ZVFTYzZhb3ZOd3IxQTY3SVRB?=
 =?utf-8?B?Q0t5RkhCTGl0akFRS0lkY3VUeVNVUnBNN2FIRzU2dmMweDVhR3B0c0gwc0FW?=
 =?utf-8?B?cUpOQjVMd3FTUTNaaTdmanZPZnVtMjA2Z2dyTENrbWd2MWVJbThHVlVYeGJH?=
 =?utf-8?B?MktudDRGL0MyZ1FkVm1jZEVyajVRV2h1UmZISGpmVU55NXBnYW1tYkFlbGU1?=
 =?utf-8?B?d0F6dCtWL01WK0VUQTBFd3Z4ZnBvenRQeFZRVmNOVjV0TEh4TXJnaS9IaXRw?=
 =?utf-8?B?VjV6c3BOQzhQNmdGT3orOS9OazgvcDhZTDdENHBhT004UGFBOURlL3pSZlVl?=
 =?utf-8?B?b3JiZDZZVHh0N1Z1T3I1M0R6OEZGcFcxVkhHZVR3aWtlNzRsSG1MaVMxYitS?=
 =?utf-8?B?Yi9QZVRhWDJEU0c1d21zY2hMS1ZWVWFTTTA0UXl6Ym5vbUc1L0Q3WW9McVEr?=
 =?utf-8?B?YVJnemg3N0xNcjFkV1hQa3VrUkh6TnhwQVg0dyt4aWdXNklVaDdCOXNzVTVP?=
 =?utf-8?B?cjM1cjR5Nlk4OUowUDBMMm1jWFd6Z0ttcmp3RVdJYk9tQmJWUzA1YWxobmpj?=
 =?utf-8?B?QVRVczVJbmdURi80MkxjSUNZaXk3U0MveGtjM3Jxa2g2Qm5kWUFwN0l0Z0dx?=
 =?utf-8?B?N3dRMkxoRFl2WW1VZ29CMVgrNjNlczhXek0wcG9ZKzk1b0RqOG5CUzBzd3VZ?=
 =?utf-8?B?c3RRQ2xNVENYUUVab1FBN0hyNTMzRm9ybXQ4NDNRaDc5ZkpGOEtCM0Y0M0R0?=
 =?utf-8?B?YTFaYThhekNCcUhYTWtaVDgyNEsxd3pxei9NVXpweUNSTnNaeVBDTVVldGhr?=
 =?utf-8?B?OU1iNGZVMFJuQm8wN1lCOUdkYW8rY3RZTkthK294dW9zQitTcHBqMmQrVFJ6?=
 =?utf-8?B?MTlHelk0andCVVZjeENGRWMyMy9HQzVDMThsOVdSeHRmN05xMFl0NWxybVVB?=
 =?utf-8?B?WHZtaUdNQVVYNkFmcWU3QnpodEFNOURrV0hCWWJFT3Z0ajFwY2t0YkFQcjh4?=
 =?utf-8?B?YUUzUWkweis0dFg0ZUtSUzFnMnc1OGppdDNiWjBEMW1MWjBVN3ZnbFRoTjdP?=
 =?utf-8?B?aU4xbHZNYXhCRXpkZzgvbW1oVUxETW55a2haaTNBbU13S2xVTWlKZzNpQ3gz?=
 =?utf-8?B?d1ZSZlVhS1p0bTFFcWZOVlBkc0ExYXNjRGlubllKemc3WDhINDRXZzRUY2Ru?=
 =?utf-8?B?SVJpSjZvalVPOXR0UytNbHdiMnFHd3FjUUp5SURadFp4TEFTTW1NeTczcjBH?=
 =?utf-8?B?ZHIzblZtbUNuQmUwajFteUdGSW9NNFV1RStnS0FhRWk3ZmUraW9KQ0lwbHJa?=
 =?utf-8?B?Q0M2SDR4clNXNjkxdndpT0VsdnY2WU1oWUZsS0xvMlJUTUlIWE9JbklNMXM4?=
 =?utf-8?B?eWxsYlRmaHVycWRnRDd3WDRZdzNwNG9qZTFSb1picmQ4VWFmTzNacHNYYS9R?=
 =?utf-8?B?bVJTL2JGZUtUdFVlbjlQMi8walBxUWVvRzIzVGdCT0hBNHBURGtqdlVOWDVB?=
 =?utf-8?B?aWV4ckdwSkFZQnE1dCtReGV6QS9vMkJ0cVluNTVNbU9UTVNuZjIwOEpmdW9z?=
 =?utf-8?B?VEFBMlZlanJDczRSTUhOVkdsc1AxM01UMWY0UjJtbGJvUW9qd0VIdDkyTVlN?=
 =?utf-8?B?NEtJK2FHRU43L0tzQjdPMzRTaW9wSk5kS1k2aEI4THE1aGNOM2syZHlVY1lp?=
 =?utf-8?B?RWM5THNDYTNXZjhGejZmbVo0YUVaZWs0R1VBaStSQWNKd1AzVldzZ0dTbUJS?=
 =?utf-8?B?NzBPS2lrTFloWHk0UG1jODlRcGlPUnMwMFJXVW0wVTh5aWNpSGgvUnRWRWc4?=
 =?utf-8?B?YjI5cU9xaGtRV04zWURac1VyQVVJUlhtdmEwV3NJdXBGUXlYVmRGaXFTQ0VV?=
 =?utf-8?B?eDAvNmgrbjdCYjAvM1QyR29RRk5KTUNlWWwwVklQSVBvUVlDekxoWUtHa3Fo?=
 =?utf-8?Q?OsDK9NnnmoCTcfQO9IJjGRbq77Fi+8=3D?=
X-OriginatorOrg: os.amperecomputing.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 63223b2b-a7e6-4583-7619-08da07ea1c97
X-MS-Exchange-CrossTenant-AuthSource: SJ0PR01MB7282.prod.exchangelabs.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 17 Mar 2022 07:45:32.3692
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3bc2b170-fd94-476d-b0ce-4229bdc904a7
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: F62kzhaywn6eBDU5aMgP2qM4ZxNsHEaaLtYu1cMzxJXl1dm3+9qtDfjZuHM6XpV4n6m0tCj9eGrhhUpbAJz+6GzFK4sR25WjTxcHuqnEVhs=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR01MB4283
X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,NICE_REPLY_A,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Added Dan as I have missed Dan's email address in the first place.
My apologize,
- Quan

On 11/03/2022 13:58, Wolfram Sang wrote:
> On Thu, Mar 10, 2022 at 06:41:18PM +0700, Quan Nguyen wrote:
>> From: Dan Carpenter <dan.carpenter@oracle.com>
>>
>> The copy_from_user() function returns the number of bytes remaining to
>> be copied but we should return -EFAULT here.
>>
>> Fixes: 501c25b59508 ("ipmi: ssif_bmc: Add SSIF BMC driver")
>> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>> Signed-off-by: Corey Minyard <cminyard@mvista.com>
>> Signed-off-by: Quan Nguyen <quan@os.amperecomputing.com>
> 
> It is nice that you want to keep this patch seperate to give Dan
> credits, but I still think it should be merged into patch 1, so the
> initial driver is as flawless as it can be. You could give Dan still
> credits by mentioning him in the commit message IMO. Dan, would you be
> fine with this?
> 
>> v6:
>>    + New add in v6, thanks Dan for the patch     [Dan]
>>
>>   drivers/char/ipmi/ssif_bmc.c | 5 ++---
>>   1 file changed, 2 insertions(+), 3 deletions(-)
>>
>> diff --git a/drivers/char/ipmi/ssif_bmc.c b/drivers/char/ipmi/ssif_bmc.c
>> index 62db97773654..91ac2cae756e 100644
>> --- a/drivers/char/ipmi/ssif_bmc.c
>> +++ b/drivers/char/ipmi/ssif_bmc.c
>> @@ -87,9 +87,8 @@ static ssize_t ssif_bmc_write(struct file *file, const char __user *buf, size_t
>>   	if (count > sizeof(struct ssif_msg))
>>   		return -EINVAL;
>>   
>> -	ret = copy_from_user(&msg, buf, count);
>> -	if (ret)
>> -		return ret;
>> +	if (copy_from_user(&msg, buf, count))
>> +		return -EFAULT;
>>   
>>   	if (!msg.len || count < ssif_msg_len(&msg))
>>   		return -EINVAL;
>> -- 
>> 2.35.1
>>