Received-SPF: pass (google.com: domain of linux-kernel+bounces-154776-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: <0e60057f-b242-4bb5-b3df-2c6d1ff39803@intel.com>
Date: Tue, 23 Apr 2024 11:20:02 +0200
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 1/5] drivers/net/ethernet/intel-ice: ensure the copied buf
 is NULL terminated
Content-Language: en-US
To: 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: <intel-wired-lan@lists.osuosl.org>, <netdev@vger.kernel.org>,
	<linux-kernel@vger.kernel.org>, <linux-scsi@vger.kernel.org>, Saurav Kashyap
	<saurav.kashyap@cavium.com>, <linux-s390@vger.kernel.org>, Jens Axboe
	<axboe@kernel.dk>, "Czapnik, Lukasz" <lukasz.czapnik@intel.com>
References: <20240422-fix-oob-read-v1-0-e02854c30174@gmail.com>
 <20240422-fix-oob-read-v1-1-e02854c30174@gmail.com>
From: Przemek Kitszel <przemyslaw.kitszel@intel.com>
In-Reply-To: <20240422-fix-oob-read-v1-1-e02854c30174@gmail.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?djFHZHJFR05FOE9xZWVmZTJDM2xtNG9UNXY5VHZISE0zMm4xU3hBL2hUR2lh?=
 =?utf-8?B?UGlDRzRWckRxRXg1ZElIZXBJSUE3cGJReGpXYlJORGM1NDZ0L3V1MUw5R0dT?=
 =?utf-8?B?Tmx3K1pCMERJcEVUMnAzQStuanBzYlZKWU11eDlzdjVPMTdWTkFmbis1WDVw?=
 =?utf-8?B?OVlIcDdCMURka1ZoT0l3cVJ0ZHpLMld1eUg0VThFY2RCRmluUDhwdkNWYjlV?=
 =?utf-8?B?bnBHZmNIVi9DbHpvK00yQlJuQWlHclVBem1najhrNFdkRmNad0xWOFRZc1VO?=
 =?utf-8?B?NjdLUlZmYVFmVHdSMkViQ05Fb1JMU09Cdzl0MnovOXI5M1pzb3VzQmIzUllX?=
 =?utf-8?B?enFvMUF5eGJLUGhqTEIwcUo1U0pXZmNPOVQ3eFBEcHhYOVhMN0hoRnF6Q3Vo?=
 =?utf-8?B?eS91UTcrODVhT0FVbGRpNVJOM2VabVMvbmVJZXZFdmc4UUxncUhLajdlcWRH?=
 =?utf-8?B?eVRvV2RNTkNoYmMvaG4ydjNCTkF6VjdPRzh1bGI2UHV0Rmh3YzFiSVNPVU1z?=
 =?utf-8?B?YTVDd3p5OHMzWjFuamswV0xvbGZEVXNoUWtPcW81MEQ2MEJINit1cDhXV0cv?=
 =?utf-8?B?Q0cxUHovODN2ZDlDcm1WbjVmN08xaHVLemlydUdOZ2lLSWkySTRwZGgrQ2dO?=
 =?utf-8?B?MmRlZmlhMmc0Yy8rcWdGRDQyQm1vb1gwZHBHeDZCRGlJWWsyNVFRYk8zazBx?=
 =?utf-8?B?TS9oaHd3UXVUdUxETHNYT1dmQUN3QXFNdGVYSk9SOWhmR2MzREN2Ykhnd1B4?=
 =?utf-8?B?c3RJZEh1UUpvWENzcE9QbkpOSmVHd2VVYVhRL3BDWXhRSlAvZGdWcEh4V2d2?=
 =?utf-8?B?aXRvNDVSR0V4Z3lrZjV3aXNqWVcyTzk2dTM3Ryt6VUNLTUZwcnVXS0dlbE93?=
 =?utf-8?B?OTh2eW10Q1RkZDFTY3V5RU1ub2FSWEVpdmNFeWt2ZXlYYzByenRrQjB2TU9O?=
 =?utf-8?B?TE9uWEh1SzlIQ0V3YW9GZzB0YnJaVElNZzV5b1VrUC9qbVlEcHlHS3ZvaHcz?=
 =?utf-8?B?dXd1Q1hoc0pUdExDK0RBajVRc1BpOUFEUjR4ZmNiSk1pZlEzcVUzZWlnZzVM?=
 =?utf-8?B?R1BjblVkUDJvQ2M3TjRET0NtRWVJUkVyOFcvUzFGeDZqaXptVXZvRlM1L2FR?=
 =?utf-8?B?b1I5QTUxMVZNSFlzTE41MkJsc2djU1FONklSQ0xOUWlDa0pLRXp3Uk1lR2h5?=
 =?utf-8?B?eCsxbzdjZEMxbzdrbVVWdlYzMG10bW10aGROUTVKRzlVeHhWcEFKZW9uc2Jt?=
 =?utf-8?B?M0NqQjV3SGlCUXVubUlyTHc1ek80eWFqZGpqc1dsbFNSbXp3azFqeisxQjBC?=
 =?utf-8?B?UC9VWkpUTGc1c2VnUHVZdTZYc0Z3ZW8vdWJRRVRHZ0VtYmVhbnlPUTdHeStq?=
 =?utf-8?B?cGgyQkRTQmlBaHlIVlp2dGF2SzlNTW1oTTdqR0trZVdmRjRYa0sxdERIQWZW?=
 =?utf-8?B?bnROZ0hGd3orTTN4Q2cxMXl3emdJVW1sbzVBZ3FGM09TdHpCMVIyMTlQM1V2?=
 =?utf-8?B?ZGp1K0R0SlBmWURjZmlLaWt2RjhsbkxVYTZSV2xTM1RVcHIxRmNRNlhGMGVW?=
 =?utf-8?B?eEpuRVNlUGJpQVl2a2ViSDdmTU9tem50WFFtS0g0STBhS0IwbnpkQ2hUQ1Mz?=
 =?utf-8?B?d2dYOGs2aVp0WFBCOTAwVGhXcjI3L2lvTTdaMjdHUHdpOEFpT0J2UlJHSG1G?=
 =?utf-8?B?MUE0Mm4xR0xTMXd5M1NJNE5GZGM2Z05uOUVVR09hU2EzV1VXVnRIRVJQc3Y4?=
 =?utf-8?B?NW5qM0ZuTlY5VndHMHZKMmV1VTJNRjUvSUFHT05JSE5yRFlqT2FFbUl5enB4?=
 =?utf-8?B?QXYrYytLbmEwOExZcEsvUm5MTkMxWlRxRUtyM0I1MUFuOEkyVmZVYWMrLzVr?=
 =?utf-8?B?dmEvajIyQVBSaW5oUXJRcVpVL0sxU2NVbEFsdUdhS0tyeWVoQ1ZVa1BrRUZ3?=
 =?utf-8?B?V2FaSlFvdzBQZEFxUWhRTTRjUS9VT0hQdktGM2h1OTUzcEtZdWR0d1lYK0JE?=
 =?utf-8?B?QWJob252cFhUVjg3b0lGV3oxdXE5WnZPL3IyVG1lT1lDcHhsaEtmY1c4MnBD?=
 =?utf-8?B?aG5FZTY4SUdEaENMQ2ZYKzQxK25yVUUyRS9HSFpaQ2dQS01vdU9UTnlrS0dM?=
 =?utf-8?B?bUlNRFBnQ0pqUmU0VkZFT2NOaWs4RVpCL1E1NnB2Z0c0cExKRWhiSzYzYVd6?=
 =?utf-8?B?MEE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: f2c32e96-b93e-4e3b-0ae5-08dc637697ea
X-MS-Exchange-CrossTenant-AuthSource: CH0PR11MB8086.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Apr 2024 09:20:18.8191
 (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: 6iD925zWFZL+b0YuWHAPPQMZn+roNsCUEun16cC0F263DmeAEEOngwzbTx+7hbnwnUh7sLGDqa1Qxkc+Z/YTEZt2u/YYu3AER8fh8Ik/EbI=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: LV3PR11MB8602
X-OriginatorOrg: intel.com

On 4/22/24 18:41, Bui Quang Minh wrote:
> Currently, we allocate a count-sized kernel buffer and copy count bytes
> from userspace to that buffer. Later, we use sscanf on this buffer but we
> don't ensure that the string is terminated inside the buffer, this can lead
> to OOB read when using sscanf. Fix this issue by using memdup_user_nul
> instead of memdup_user.
> 
> Fixes: 96a9a9341cda ("ice: configure FW logging")
> Fixes: 73671c3162c8 ("ice: enable FW logging")
> Signed-off-by: Bui Quang Minh <minhquangbui99@gmail.com>
> ---
>   drivers/net/ethernet/intel/ice/ice_debugfs.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_debugfs.c b/drivers/net/ethernet/intel/ice/ice_debugfs.c
> index d252d98218d0..9fc0fd95a13d 100644
> --- a/drivers/net/ethernet/intel/ice/ice_debugfs.c
> +++ b/drivers/net/ethernet/intel/ice/ice_debugfs.c
> @@ -171,7 +171,7 @@ ice_debugfs_module_write(struct file *filp, const char __user *buf,
>   	if (*ppos != 0 || count > 8)
>   		return -EINVAL;
>   
> -	cmd_buf = memdup_user(buf, count);
> +	cmd_buf = memdup_user_nul(buf, count);
>   	if (IS_ERR(cmd_buf))
>   		return PTR_ERR(cmd_buf);
>   
> @@ -257,7 +257,7 @@ ice_debugfs_nr_messages_write(struct file *filp, const char __user *buf,
>   	if (*ppos != 0 || count > 4)
>   		return -EINVAL;
>   
> -	cmd_buf = memdup_user(buf, count);
> +	cmd_buf = memdup_user_nul(buf, count);
>   	if (IS_ERR(cmd_buf))
>   		return PTR_ERR(cmd_buf);
>   
> @@ -332,7 +332,7 @@ ice_debugfs_enable_write(struct file *filp, const char __user *buf,
>   	if (*ppos != 0 || count > 2)
>   		return -EINVAL;
>   
> -	cmd_buf = memdup_user(buf, count);
> +	cmd_buf = memdup_user_nul(buf, count);
>   	if (IS_ERR(cmd_buf))
>   		return PTR_ERR(cmd_buf);
>   
> @@ -428,7 +428,7 @@ ice_debugfs_log_size_write(struct file *filp, const char __user *buf,
>   	if (*ppos != 0 || count > 5)
>   		return -EINVAL;
>   
> -	cmd_buf = memdup_user(buf, count);
> +	cmd_buf = memdup_user_nul(buf, count);
>   	if (IS_ERR(cmd_buf))
>   		return PTR_ERR(cmd_buf);
>   
> 

Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>