Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:6 as permitted sender) client-ip=2620:137:e000::3:6;
Message-ID: <1abd6bcb-6f6c-10a7-9b6f-e5e038233af8@intel.com>
Date:   Wed, 13 Dec 2023 10:49:10 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.3.1
Subject: Re: [PATCH v2 iwl-next] ice: Fix some null pointer dereference issues
 in ice_ptp.c
Content-Language: en-US
To:     Kunwu Chan <chentao@kylinos.cn>, <jesse.brandeburg@intel.com>,
        <anthony.l.nguyen@intel.com>, <davem@davemloft.net>,
        <edumazet@google.com>, <kuba@kernel.org>, <pabeni@redhat.com>,
        <richardcochran@gmail.com>, <jacob.e.keller@intel.com>
CC:     <karol.kolacinski@intel.com>, <michal.michalik@intel.com>,
        <intel-wired-lan@lists.osuosl.org>, <netdev@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, Kunwu Chan <kunwu.chan@hotmail.com>
References: <20231212024015.11595-1-chentao@kylinos.cn>
From:   Przemek Kitszel <przemyslaw.kitszel@intel.com>
In-Reply-To: <20231212024015.11595-1-chentao@kylinos.cn>
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?QzRHYldiaVBwaDg0OE1VTVNGL3JvNUtjREZPN2FVZlE4S1FUdVp0dHNCT3FL?=
 =?utf-8?B?S1IzOVFKcU1IdEJtbmU0RmhFZlN1eWpOUXFXaEJWL1hPYXowdzFqblpwS2RI?=
 =?utf-8?B?a1JLeWVCVG1OQ2hDMk0wa3dCSXhUSW1IaFFYOThQL3F0N2FLNWRGc2NOQ240?=
 =?utf-8?B?RHJoY3NCT2NEWjJteXduRDM4c1l1aXoxdWoxRzhKSWJvZDNqVUxUbjFYTC9p?=
 =?utf-8?B?aVZHTzNjckV1VmxDQWQ4QzE3UlNSeWtFUUlSc1FYV1MwenpSenFnQzMzQzJT?=
 =?utf-8?B?UHpCbEhRN3lJeDFqTk12T1BzZklnNHFlMkVWR3FjMXdPZnFlWnI0Umk5bUZz?=
 =?utf-8?B?MnNWcGxkVGlRRnNxNGdKWUhGN01US2ZjQURzOEJHc2FldDZBWm93OCtmMDEv?=
 =?utf-8?B?b3l1VFNZejgxN2ttekRTUGd6MU1tTkEreUZYZGJoQXpEMXNhN3hKNXRCeTVS?=
 =?utf-8?B?d3JYb2gxUHBMUUVYYlI4WHhBeUtSeDd4QSsyWlIzdVBXN0J0UzcybTNYUWd1?=
 =?utf-8?B?bVN3RStRS3dhc1lCa1ZhdHZnUWh1OXMzbkZWcGtDVXFhTFFrcjdhQy9lTUtl?=
 =?utf-8?B?UC9WSlpVajQ5Qzk2VTlmekVyd1V6WXBRaDE0ZnBFMUl6aEVqVm11T25oNnhG?=
 =?utf-8?B?U1l4ZVNybkNtOWtQa3BjWTd2VHVkeUtLY0pPbDBQR3JrRHFPTGFuYkFWbitJ?=
 =?utf-8?B?c3hIZ0cveU5QMWwzVWllVDlOUFhLd3Z3Q05HU3JMeWdUd0pZS2VUenJWbWRJ?=
 =?utf-8?B?alZmT2JGRmx3ak5yTWFwazJQY1lFNE5WQzdURXVxcjIwRWRsamRzYUFCdjFh?=
 =?utf-8?B?YVVnZUdjUUhCOVB1Vm9wT2duQTVxbEJWdDNOTWp4TGtyeVFRamRMOGZFY2Fu?=
 =?utf-8?B?VTNxU2g0b2RQcUsydHhCYVcwbWNjTmJxazBnSnRNZnNnUzZQTEpnNjRwaElE?=
 =?utf-8?B?dXRhTHNrRmVHQ1lvbEI1aWRPREtsRTcxNnNDZUhxaFQ0WUk3TURaa1VacDg0?=
 =?utf-8?B?VEk0emdqU0I5bmdjbExDS1JLUjBtNHBmOW9rdFBZdW1MOUpLbUprempLaTha?=
 =?utf-8?B?QzViZGxGdFJZelluZVN1WVBOcElpcnlpZDF1NFE4N1Y4d2xJV2xJNFB1d0VY?=
 =?utf-8?B?c0RVZmxmVnMya3gvTTZFVWY0d3VoUGZRTkZxcFJVS1hMdnozaXhseGhzMEMv?=
 =?utf-8?B?am5JODFKdUdzVjRaaC9aa0Q0MkdUQjZlME84OWZRRk9hRjBmMnQ4RWgwM3pl?=
 =?utf-8?B?VlhiaU54YmJTQ2VyVlo2RlFqVTg4OG1FWFFKVzVOd3d0N1JxUGU1N0IvWFd3?=
 =?utf-8?B?REcvbEtmdEEzTVVqNnpnQjFCMFpnZnl3VXdZaHlrMHhxZFphaUJkZjNkOVhp?=
 =?utf-8?B?ZVZldVJnd09VYmt2RWdORlg2ZWVTK1lJU3FRY0dnT0lkRCtEblJ4NTR1TzBD?=
 =?utf-8?B?S3FKblJxVUZibFhaZkx4UjNlckJCYUNCSk9EUXJXZnQzRWx5VHY3N1NFU0lC?=
 =?utf-8?B?Vm9nd0ZYSURtaTFPOXdjSzZLbTJVSFN2VjZzS2dHZlkyNERKOTlGU003V2tH?=
 =?utf-8?B?cFZLWG1Rbzd4YUdDSVp4bUtySjRBTjRiNnNQMlVaNmZML0xtZ0J3TjVSWTFJ?=
 =?utf-8?B?bHBVSisyd1Q3Q2t4TEQyZjlmT1pqUWhXb0JMSlMxQUt3WUhCenlkUFJGSWVt?=
 =?utf-8?B?ZWtScWkyMzFDdFhUOVFmRkl5WXk0S0hDVTZvYVdlcEJWa2hoNFBpdStJMC96?=
 =?utf-8?B?MDJ4NHhNblljY0U0NkdBUDVVeEtrOUNYZ3VvRmNuMXFCRmovWlF5UU5oeXZr?=
 =?utf-8?B?RkNWOUtzSzJpQ0ZISGg1K1JVT2xUL1p6Zy9IQVJsTURmZWVYZ2JqREt0UDB4?=
 =?utf-8?B?RktHMThtVGhiT1AxMXZuaUFYSGN2OHJFQkQwalF0WHJwSm13VElnSk5sSWNR?=
 =?utf-8?B?eG9BN2VUMldEcEVCMlBiSlUrclhwTUZDelFIUHgweHdGUmppU3M4Tk1oZVZo?=
 =?utf-8?B?dkVHWG9ZVE4wTERzVWR2cm5uUzZCYVBVanU1cGdPb1B4YkV2QW9oRjhzRTdH?=
 =?utf-8?B?Y1NQb29kOGRrU0dMRkF3dkZNWmRQMWlKb0tSdUVYL2tpWUVnYzJDcExPbW5y?=
 =?utf-8?B?SGZPM2cxVGNWVDJORzZUVjQ2aVY4ekJiVVRjR0d2R2ZjSi9kV0NWYllRaGU3?=
 =?utf-8?B?ZlE9PQ==?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 810c45cc-d015-4132-ff16-08dbfbc0c82c
X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3674.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 13 Dec 2023 09:49:22.4571
 (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: YCifLmbOVtz4dXXr/WNGyhWpCPOf9HI63NrMOG/ksh75lLqr0Ajwb+GUuseH5NiNHSSjzjvmL1PgWEBNbTQFKFlt+Me/+UC89jjtNojH9VM=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: SA1PR11MB8427
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-1.6 required=5.0 tests=DKIMWL_WL_HIGH,DKIM_SIGNED,
	DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,SPF_HELO_NONE,SPF_PASS,
	T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on pete.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (pete.vger.email [0.0.0.0]); Wed, 13 Dec 2023 01:49:51 -0800 (PST)

On 12/12/23 03:40, Kunwu Chan wrote:
> devm_kasprintf() returns a pointer to dynamically allocated memory
> which can be NULL upon failure.
> 
> Fixes: d938a8cca88a ("ice: Auxbus devices & driver for E822 TS")
> Cc: Kunwu Chan <kunwu.chan@hotmail.com>
> Suggested-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>

You found the bug (or some some static analysis tool in that case);
there is no need to add Suggested-by for every person that suggests
something during review - the tag is for "person/s that suggested
making such change in the repo".

Subject line would be better if less generic, eg:
ice: avoid null deref of ptp auxbus name

> Signed-off-by: Kunwu Chan <chentao@kylinos.cn>
> ---
>   drivers/net/ethernet/intel/ice/ice_ptp.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
> index e9e59f4b5580..848e3e063e64 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
> @@ -2743,6 +2743,8 @@ static int ice_ptp_register_auxbus_driver(struct ice_pf *pf)
>   	name = devm_kasprintf(dev, GFP_KERNEL, "ptp_aux_dev_%u_%u_clk%u",
>   			      pf->pdev->bus->number, PCI_SLOT(pf->pdev->devfn),
>   			      ice_get_ptp_src_clock_index(&pf->hw));
> +	if (!name)
> +		return -ENOMEM;
>   
>   	aux_driver->name = name;
>   	aux_driver->shutdown = ice_ptp_auxbus_shutdown;
> @@ -2989,6 +2991,8 @@ static int ice_ptp_create_auxbus_device(struct ice_pf *pf)
>   	name = devm_kasprintf(dev, GFP_KERNEL, "ptp_aux_dev_%u_%u_clk%u",
>   			      pf->pdev->bus->number, PCI_SLOT(pf->pdev->devfn),
>   			      ice_get_ptp_src_clock_index(&pf->hw));
> +	if (!name)
> +		return -ENOMEM;
>   
>   	aux_dev->name = name;
>   	aux_dev->id = id;

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

Regarding iwl-next vs iwl-net: this bug is really unlikely to manifest,
as we take care of both earlier and future mem allocs for ptp auxbus,
and auxiliary_device_init() checks for null name, so no big deal,
so: -next is fine