Received-SPF: softfail (google.com: domain of transitioning linux-kernel-owner@vger.kernel.org does not designate 23.128.96.19 as permitted sender) client-ip=23.128.96.19;
From:   "Keller, Jacob E" <jacob.e.keller@intel.com>
To:     Xiaomeng Tong <xiam0nd.tong@gmail.com>,
        "Brandeburg, Jesse" <jesse.brandeburg@intel.com>
CC:     "Nguyen, Anthony L" <anthony.l.nguyen@intel.com>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "kuba@kernel.org" <kuba@kernel.org>,
        "pabeni@redhat.com" <pabeni@redhat.com>,
        "Raj, Victor" <victor.raj@intel.com>,
        "intel-wired-lan@lists.osuosl.org" <intel-wired-lan@lists.osuosl.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "stable@vger.kernel.org" <stable@vger.kernel.org>
Subject: RE: [PATCH] ice: ice_sched: fix an incorrect NULL check on list
 iterator
Thread-Topic: [PATCH] ice: ice_sched: fix an incorrect NULL check on list
 iterator
Thread-Index: AQHYQaYPxsKO3xAdd0iiv3AHWrwYUazVF6wA
Date:   Mon, 28 Mar 2022 17:59:35 +0000
Message-ID: <CO1PR11MB5089C7298CC46861FF41D3E3D61D9@CO1PR11MB5089.namprd11.prod.outlook.com>
References: <20220327064344.7573-1-xiam0nd.tong@gmail.com>
In-Reply-To: <20220327064344.7573-1-xiam0nd.tong@gmail.com>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?fv1bXTSk2hQi/GoMr8txe1G2EA04df0Y92XMHoOhIrD0h5WdeDgzoLby3dl4?=
 =?us-ascii?Q?k/mMI8+E1YhwV+KUyGYzBCaPfPFe0epc1+eY2QVnatmkjsbp9IzWbE0dIgv4?=
 =?us-ascii?Q?DiJr2A+j+pvtADAE6Kno1zx0M8VHtfQTuYhvRTFwnldlnhI447YTYP9HUaLP?=
 =?us-ascii?Q?mh8GweWA7Xu6XHbleBdK1qWl5h3LS0s7U0zXiv+8iRyyLamAbKtvjPy8DBLU?=
 =?us-ascii?Q?GBT4frGfBaugzIrEKaYqHphVkW3hFJ/hFWwZ7k2omc4F1Ia0ZmVORx8tUV7Q?=
 =?us-ascii?Q?ff9nQBBAd3keuJhKOcrIjvBr7VncEji4zFEGjEMzqxjsEVMqNS8cBijjMvuK?=
 =?us-ascii?Q?sUBpLWw119O9uuLZ4J+CSY+6w3QnOvr3dECAHFuN0F1yEmQVD5wd2ip40c7b?=
 =?us-ascii?Q?WJwO+nEVcNokNDYNHV8ipYt1TUYzZ0ayf/LfsBLNGQdcODSgD79o2mky8V7z?=
 =?us-ascii?Q?Oa2kvlexn6C8LodVNqexxw/YoyxvHiYzg1zlZOCbaQdNRffS7PMnTRK1kvJV?=
 =?us-ascii?Q?pP0eFUlBdz6H5FRo4TjsH4dXghVw2j6Aivf6/wcv9yp8jXqudP2NwoVw5UOR?=
 =?us-ascii?Q?I/H86Se4SQVCq/xrQT7eJKoKcA/dert7QqbBSgg81RYvBnUC/4W3MS90DdAc?=
 =?us-ascii?Q?BrtzkrDKf/Y0Z1UyK6V6cWDxR1QJbq2XTUXb9W77oZBri3+4iZ1RbKjm/fyt?=
 =?us-ascii?Q?f0G4F8vtWyN2n8F7+4J7wi9hIOuw/+1crz3Do53JnAUiSKMDaAyK5te/tWRh?=
 =?us-ascii?Q?JfuDsDP8uMRnsrFi2WyB1OcPKq9F6G/pbYOnM/pjOU9cozrdN2ixF3a9nU8Y?=
 =?us-ascii?Q?B0r7dP9DazelaRAdFVPqBGvq4D46sZa1edFHaTOHKqbLPgG/rO/o8ySZ/mfd?=
 =?us-ascii?Q?m+rRRIH02IADKFAvZKAbv6KAzVm7u5rLU6p59ZDm2kA6zlYSpBEZsNtkgH1S?=
 =?us-ascii?Q?EgGxm2D5JVh/0qZ+ILwp/1H5cupv5V97Cv6nMzmjjLya8cRI+x0dxXbSli+x?=
 =?us-ascii?Q?aZs6TCZLf+XJG6tTIsiI2vTRE+mFqL2mmWHE+dZPq4H6tXTx18hhSzEcpQpV?=
 =?us-ascii?Q?TLIUpidiRFQ+79/fPRe4Gsprtyqfjfy3ZHyhIh5n14XEK20oGlEgniAOwKJc?=
 =?us-ascii?Q?ZMldcFmTlRfR6TFNWyKHgRI3FJXs+PQfKXvos4vbQZm98rHZvR6yyIG1PnXt?=
 =?us-ascii?Q?ylRd2bpJXFEHL4/WUkQXK71ENe+L5aj02YWKEb/1gZnlMWBilY2a9txdPvZx?=
 =?us-ascii?Q?QlY97+67QoKKy1TRs2KEW0Bb7q1vwEgOe1RnOZkmezBeAKB5SKrvIkYLZoME?=
 =?us-ascii?Q?0S2QWD5NQdvi1xKAJnhib3YlWFZ6u0Rs2vCbKErf/lbVtkP71y9wFuu8Ic7h?=
 =?us-ascii?Q?oveZWZojxvXPcWUrmIQg2N7fVfHurUjizFaMnEfR/A0BvPOFKe8G04l8nRda?=
 =?us-ascii?Q?VQIa55tRwsfRHdWvhQqEeK1O+m2ThSLPTvjo71F5wkHqNGfW7iHuWnROzT26?=
 =?us-ascii?Q?eNwBjhHFMdRwuuE1nIRt5xnBagYANDIV5WCDQGHYCiN0cCrtl7fRxnZn3OtA?=
 =?us-ascii?Q?PFSj9oQMIyK9X2ZuxUEyl8ykJoipuadtdXzvtyOpX91ioKFO/DUSBdsjWqzp?=
 =?us-ascii?Q?ziUbuuSlq+c7+DFue3YN5PM5wSvi4NEyQXSKwxShslu128R8xddqWdtX9hx/?=
 =?us-ascii?Q?IW+MA2iDWP5G+UILvS+p5c4U09vzivFEbuxRafAZHuMWsL1dJyrbhPoclmW4?=
 =?us-ascii?Q?LD5tbZdhlQ=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: CO1PR11MB5089.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: e68153e5-f1e1-45ab-e1db-08da10e4b881
X-MS-Exchange-CrossTenant-originalarrivaltime: 28 Mar 2022 17:59:35.9360
 (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: tUZLuTPpq6wu2fQujGXeGVLGTMdyeakJBqsSfXTtyw0CpmLHi7HoavVs/jfx6oT7I8pOmAJaSLf0v/3K/B3QkEW5xGY9hfnWUP3bUMC5qYY=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB3916
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
	DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,RDNS_NONE,SPF_HELO_NONE,T_SCC_BODY_TEXT_LINE
	autolearn=no 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


> -----Original Message-----
> From: Xiaomeng Tong <xiam0nd.tong@gmail.com>
> Sent: Saturday, March 26, 2022 11:44 PM
> To: Brandeburg, Jesse <jesse.brandeburg@intel.com>
> Cc: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; davem@davemloft.net;
> kuba@kernel.org; pabeni@redhat.com; Raj, Victor <victor.raj@intel.com>; i=
ntel-
> wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Xiaomeng Tong <xiam0nd.tong@gmail.com>;
> stable@vger.kernel.org
> Subject: [PATCH] ice: ice_sched: fix an incorrect NULL check on list iter=
ator
>=20
> The bugs are here:
> 	if (old_agg_vsi_info)
> 	if (old_agg_vsi_info && !old_agg_vsi_info->tc_bitmap[0]) {
>=20
> The list iterator value 'old_agg_vsi_info' will *always* be set
> and non-NULL by list_for_each_entry_safe(), so it is incorrect
> to assume that the iterator value will be NULL if the list is
> empty or no element found (in this case, the check
> 'if (old_agg_vsi_info)' will always be true unexpectly).
>=20
> To fix the bug, use a new variable 'iter' as the list iterator,
> while use the original variable 'old_agg_vsi_info' as a dedicated
> pointer to point to the found element.
>=20

Yep. This looks correct to me.

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>

Thanks,
Jake

> Cc: stable@vger.kernel.org
> Fixes: 37c592062b16d ("ice: remove the VSI info from previous agg")
> Signed-off-by: Xiaomeng Tong <xiam0nd.tong@gmail.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_sched.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>=20
> diff --git a/drivers/net/ethernet/intel/ice/ice_sched.c
> b/drivers/net/ethernet/intel/ice/ice_sched.c
> index 7947223536e3..fba524148a09 100644
> --- a/drivers/net/ethernet/intel/ice/ice_sched.c
> +++ b/drivers/net/ethernet/intel/ice/ice_sched.c
> @@ -2757,6 +2757,7 @@ ice_sched_assoc_vsi_to_agg(struct ice_port_info *pi=
,
> u32 agg_id,
>  			   u16 vsi_handle, unsigned long *tc_bitmap)
>  {
>  	struct ice_sched_agg_vsi_info *agg_vsi_info, *old_agg_vsi_info =3D NULL=
;
> +	struct ice_sched_agg_vsi_info *iter;
>  	struct ice_sched_agg_info *agg_info, *old_agg_info;
>  	struct ice_hw *hw =3D pi->hw;
>  	int status =3D 0;
> @@ -2774,11 +2775,13 @@ ice_sched_assoc_vsi_to_agg(struct ice_port_info
> *pi, u32 agg_id,
>  	if (old_agg_info && old_agg_info !=3D agg_info) {
>  		struct ice_sched_agg_vsi_info *vtmp;
>=20
> -		list_for_each_entry_safe(old_agg_vsi_info, vtmp,
> +		list_for_each_entry_safe(iter, vtmp,
>  					 &old_agg_info->agg_vsi_list,
>  					 list_entry)
> -			if (old_agg_vsi_info->vsi_handle =3D=3D vsi_handle)
> +			if (iter->vsi_handle =3D=3D vsi_handle) {
> +				old_agg_vsi_info =3D iter;
>  				break;
> +			}
>  	}
>=20
>  	/* check if entry already exist */
> --
> 2.17.1