Received-SPF: pass (google.com: domain of linux-kernel+bounces-154978-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <f4c18446-e0f9-4f32-8460-2e885a673ce6@intel.com>
Date: Tue, 23 Apr 2024 13:25:07 +0200
User-Agent: Mozilla Thunderbird
Subject: Re: [Intel-wired-lan] [PATCH 0/5] Ensure the copied buf is NULL
 terminated
Content-Language: en-US
To: Marcin Szycik <marcin.szycik@linux.intel.com>, Bui Quang Minh
	<minhquangbui99@gmail.com>, Jesse Brandeburg <jesse.brandeburg@intel.com>,
	Tony Nguyen <anthony.l.nguyen@intel.com>, "David S. Miller"
	<davem@davemloft.net>, Eric Dumazet <edumazet@google.com>, Jakub Kicinski
	<kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>, Paul M Stillwell Jr
	<paul.m.stillwell.jr@intel.com>, Rasesh Mody <rmody@marvell.com>, "Sudarsana
 Kalluru" <skalluru@marvell.com>, <GR-Linux-NIC-Dev@marvell.com>, "Krishna
 Gudipati" <kgudipat@brocade.com>, Anil Gurumurthy
	<anil.gurumurthy@qlogic.com>, Sudarsana Kalluru
	<sudarsana.kalluru@qlogic.com>, "James E.J. Bottomley"
	<James.Bottomley@HansenPartnership.com>, "Martin K. Petersen"
	<martin.petersen@oracle.com>, Fabian Frederick <fabf@skynet.be>, "Saurav
 Kashyap" <skashyap@marvell.com>, Javed Hasan <jhasan@marvell.com>,
	<GR-QLogic-Storage-Upstream@marvell.com>, Nilesh Javali
	<nilesh.javali@cavium.com>, Arun Easi <arun.easi@cavium.com>, "Manish
 Rangankar" <manish.rangankar@cavium.com>, Vineeth Vijayan
	<vneethv@linux.ibm.com>, Peter Oberparleiter <oberpar@linux.ibm.com>, "Heiko
 Carstens" <hca@linux.ibm.com>, Vasily Gorbik <gor@linux.ibm.com>, "Alexander
 Gordeev" <agordeev@linux.ibm.com>, Christian Borntraeger
	<borntraeger@linux.ibm.com>, Sven Schnelle <svens@linux.ibm.com>
CC: Jens Axboe <axboe@kernel.dk>, <linux-s390@vger.kernel.org>,
	<linux-scsi@vger.kernel.org>, <netdev@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <intel-wired-lan@lists.osuosl.org>, "Saurav
 Kashyap" <saurav.kashyap@cavium.com>
References: <20240422-fix-oob-read-v1-0-e02854c30174@gmail.com>
 <eb54c7bb-db63-4361-b42f-dc02e2c37fbf@linux.intel.com>
From: Przemek Kitszel <przemyslaw.kitszel@intel.com>
In-Reply-To: <eb54c7bb-db63-4361-b42f-dc02e2c37fbf@linux.intel.com>
Content-Type: text/plain; charset="UTF-8"; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?M1l0WUMyM2diU0J4RURzVC9rVDNXN0ZETVFNNnZTL3FVK3hBTmQ3QW9SK2kx?=
 =?utf-8?B?eUl3VHVqZ3pmMVVBUElUTEV6VVBaY2EvemJTUlNTb3dDMFVGaVI3bFBYRjN2?=
 =?utf-8?B?ZTN3azV6VEV3U01UbGhBSW1teUhZUnpnam13M3QvT1ZuRUtNcEV4RlVEc2VU?=
 =?utf-8?B?NEpNekx0ZERJbnl5aGIwYUlaQkRQM2RwSkdXb3FlNnJWR1A3ak9HQWlPSHV3?=
 =?utf-8?B?YjA1aTJaLzljR2JXVGtZRHZ5eEQ1NVRhVFROMVQ0ZXA5NDdHVmVDOUhza2JV?=
 =?utf-8?B?V2RMYmIzaHVoelU4ZXNHTEdvL0N5M2hZaExQek9qcU1nVTBHRHo3TkdQdmdU?=
 =?utf-8?B?QjQ5S2o4ZEZRRzQ1aCtvUE9xc3Bza2pqTHFVVW9scStzblFMWUl6QVJqaS85?=
 =?utf-8?B?eUdjMmorZW1WTDNKOGZHMXpzQkcyOXlyek5jV3gycE5OeVJZa0ZnMVpyYlNO?=
 =?utf-8?B?NUJUeERFdWtJRmtwWU5VT1BhQXJHKzZneDZiVW9MdVRHK3FObUtaQnl5WGw2?=
 =?utf-8?B?QUJrblNBNU5VYkliYnZEcTZOQm80aFVjYnlsc1YyVkpaU0lkMy9lMTdieGVH?=
 =?utf-8?B?SGhKclpRYkJCdjd6dU4zSDlGMzVQWWdtS1dwOXAzSmh3RlhvS2lrbFZrdWNn?=
 =?utf-8?B?Z012SDBZZjVVOU5EbkEydmc5TVdxMHd0T0IzQ0tPdzEvUXFRYThUUUJxaTFu?=
 =?utf-8?B?eFBrMkNmLytMVGRxZHdWTmlUUmRMeU9Bc3ZRNUVCM2JvbXc1UWpKeUMxdVFw?=
 =?utf-8?B?OUpPeTdqL0VkbTg0NVNHL0pHcUxwZGQ2Z0Q2VDJCb2NlWU42MUJVUTRnelhI?=
 =?utf-8?B?aEk3RldUMGx3R0pRMUlTOEMvbm05Z25CRWc3ZjFNR256ODAvVTg5WExjMndz?=
 =?utf-8?B?RmFrUmJuaVJ2MTFmc3BXT0tlSzd2ZElVT09mWmFrcTFvK3dSd0ZiZG5ON2l3?=
 =?utf-8?B?czIrQURGOEhFZGREcko5ZFg1M0ovQUdrQy9mQkhCbDBKc2w2TkprS0RDSyt3?=
 =?utf-8?B?NHA2WkozNGJsdFdpUlVZSzA2a3dWMjFrcnRjS3FFeG5jSnlqaDVqTHc5N242?=
 =?utf-8?B?dGRPdS9UYVJ2RW1MMFJtNVhGUkIwZE5PQXUxRDZZNSs5dGlPUFBpOEdDTWMr?=
 =?utf-8?B?d1pXMW5wbFpuUy81NW82VXp6dmVuekFvTFdjOWVqQktrcWE0YVBCOXlrTzIz?=
 =?utf-8?B?RHdOMjJUZWtwYlFLVFZtemw5M3ZCTXhnRlgxdjRvTDFXOVRqNEhISUFoWkFL?=
 =?utf-8?B?cU9IRU11aHVnams1M2JJVWVJYUZmTDRrV2s0ZlJueEtEbWQrNzAvYU1TT3Vo?=
 =?utf-8?B?c29RODI4VC9SeFFURE44NVdTZjdERWkvcDBPdGxxSm4xbDdmQTQwdlZhU1hw?=
 =?utf-8?B?RFVMdEt4OW5ocTI1eVY5V0Q1UUJlOWNwZEZxWS9VZ2xPcUNjSDhSRHEzNzRL?=
 =?utf-8?B?dFJsbUpydzUvSTg1T0NNZUh0di9iYVJFWjRWdFYwK3liVjZLK2dVY2IyeUVD?=
 =?utf-8?B?b01FRzNjaUJvNnBhY3FVRC9jb0RCb21JdmMwc3EwNjlMeEZVQnZEc1drSXM3?=
 =?utf-8?B?MW8rVEVEaEU1Qm9VRUFFUGZPdGF6b1pFSkJOZXdYWVM5WWVQUDVvQ0k5RWdI?=
 =?utf-8?B?VWFVZGRPWWtLaExJQXBteW54SWV6WW13dk8vN0hOU2RBUUlLMHlqTlJucTFL?=
 =?utf-8?B?dVMzR2NWUWRQb2xmbUhkd2dHTGFBNVNjaVM4Tk81RjNRcnVoelVMZVZxeXA5?=
 =?utf-8?B?N2x5dGxyenkvbk9PMXF0THR3S1RMMVMxOGNRMUJzejJlSytuYTNsS2dKT2tk?=
 =?utf-8?B?RmQzZ0w4YlZhSnFsaHgzSUZLQUNsTTBqeitlRFFNYlh2K3RNSjk5dW1sdVFL?=
 =?utf-8?B?Y0EySVFkSDJzMXBsU0JXZ3doWExDbW9hbU5lZkxHbWJKd0ZybnYybk1zeGVj?=
 =?utf-8?B?VXhSb2FNbE5GZmYvQlBPWUUvVTRIUXBScldlTkJrSEtjRHQ2MlNJMEtlV2Vq?=
 =?utf-8?B?VHhieDB2aDJkWlc3dm5nVzJhVDBSelcwaDF0RFI2Y0xSUUN2dVRkak9DbFFw?=
 =?utf-8?B?ZzlzZzVjUGRLWVFFUzlCMTFYVlNUR3J1RS9HaERmRThpb3lhODdhSENCYk1V?=
 =?utf-8?B?UjBJUDRPbHNvbVdLSUZxcnF0TTZnR3JJQkNtUVU5a24zVlB6bjhjNGlPMDY4?=
 =?utf-8?B?dlE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 377a3a23-b695-443e-c583-08dc63880ffc
X-MS-Exchange-CrossTenant-AuthSource: CH0PR11MB8086.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Apr 2024 11:25:21.6358
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: K6ihMREXiL86w0nSn2YjyVjH2ZwgEdagAgkjcYWNr0SKPR7S2ebHjd3/EVGCGBvt+xg4kyUYvTuk6ARnN0U3T3m1xneBgA4e0otgc9+vrH4=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA2PR11MB4922
X-OriginatorOrg: intel.com

On 4/23/24 13:10, Marcin Szycik wrote:
> 
> 
> On 22.04.2024 18:41, Bui Quang Minh wrote:
>> Hi everyone,
>>
>> I found that some drivers contains an out-of-bound read pattern like this
>>
>> 	kern_buf = memdup_user(user_buf, count);
>> 	...
>> 	sscanf(kern_buf, ...);
>>
>> The sscanf can be replaced by some other string-related functions. This
>> pattern can lead to out-of-bound read of kern_buf in string-related
>> functions.
>>
>> This series fix the above issue by replacing memdup_user with
>> memdup_user_nul or allocating count + 1 buffer then writing the NULL
>> terminator to end of buffer after userspace copying.
>>
>> Thanks,
>> Quang Minh.
>>
>> Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com>
>> ---
>> Bui Quang Minh (5):
>>        drivers/net/ethernet/intel-ice: ensure the copied buf is NULL terminated
>>        drivers/net/brocade-bnad: ensure the copied buf is NULL terminated
>>        drivers/scsi/bfa/bfad: ensure the copied buf is NULL terminated
>>        drivers/scsi/qedf: ensure the copied buf is NULL terminated
>>        drivers/s390/cio: ensure the copied buf is NULL terminated
> 
> Typically you don't include path to module in title, instead:
> ice: ensure the copied buf is NULL terminated
> bna: ensure the copied buf is NULL terminated
> etc.

good point,
if you would respin, then the character name is NUL, not NULL.

> 
>>
>>   drivers/net/ethernet/brocade/bna/bnad_debugfs.c | 4 ++--
>>   drivers/net/ethernet/intel/ice/ice_debugfs.c    | 8 ++++----
>>   drivers/s390/cio/cio_inject.c                   | 3 ++-
>>   drivers/scsi/bfa/bfad_debugfs.c                 | 4 ++--
>>   drivers/scsi/qedf/qedf_debugfs.c                | 2 +-
>>   5 files changed, 11 insertions(+), 10 deletions(-)
>> ---
>> base-commit: ed30a4a51bb196781c8058073ea720133a65596f
>> change-id: 20240422-fix-oob-read-19ae7f8f3711
>>
>> Best regards,
> 
> Thanks,
> Marcin