linux-kernel - RE: [PATCH] ice: ice_sched: fix an incorrect NULL check on list iterator

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <BYAPR11MB339776DBFFC7F86EC7F4DD708DC09@BYAPR11MB3397.namprd11.prod.outlook.com>
Date:   Tue, 3 May 2022 00:31:28 +0000
From:   "Raj, Victor" <victor.raj@...el.com>
To:     "G, GurucharanX" <gurucharanx.g@...el.com>,
        "Keller, Jacob E" <jacob.e.keller@...el.com>,
        Xiaomeng Tong <xiam0nd.tong@...il.com>,
        "Brandeburg, Jesse" <jesse.brandeburg@...el.com>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "intel-wired-lan@...ts.osuosl.org" <intel-wired-lan@...ts.osuosl.org>,
        "kuba@...nel.org" <kuba@...nel.org>,
        "pabeni@...hat.com" <pabeni@...hat.com>,
        "davem@...emloft.net" <davem@...emloft.net>
Subject: RE: [PATCH] ice: ice_sched: fix an incorrect NULL check on list
 iterator

If the control comes down to the loop then surely the VSI is already part of an aggregator (old). The aggregator vsi list should have that VSI information. My understanding is that if control comes here then you always find a valid entry and matching handle here. If that doesn't happen then we need to debug. The fix is kind of masking this problem.

-Victor



-----Original Message-----
From: G, GurucharanX <gurucharanx.g@...el.com> 
Sent: Monday, May 2, 2022 1:18 AM
To: Keller, Jacob E <jacob.e.keller@...el.com>; Xiaomeng Tong <xiam0nd.tong@...il.com>; Brandeburg, Jesse <jesse.brandeburg@...el.com>
Cc: netdev@...r.kernel.org; Raj, Victor <victor.raj@...el.com>; stable@...r.kernel.org; linux-kernel@...r.kernel.org; intel-wired-lan@...ts.osuosl.org; kuba@...nel.org; pabeni@...hat.com; davem@...emloft.net
Subject: RE: [PATCH] ice: ice_sched: fix an incorrect NULL check on list iterator



-----Original Message-----
> From: Xiaomeng Tong <xiam0nd.tong@...il.com>
> Sent: Saturday, March 26, 2022 11:44 PM
> To: Brandeburg, Jesse <jesse.brandeburg@...el.com>
> Cc: Nguyen, Anthony L <anthony.l.nguyen@...el.com>; 
> davem@...emloft.net; kuba@...nel.org; pabeni@...hat.com; Raj, Victor 
> <victor.raj@...el.com>; intel- wired-lan@...ts.osuosl.org; 
> netdev@...r.kernel.org; linux- kernel@...r.kernel.org; Xiaomeng Tong 
> <xiam0nd.tong@...il.com>; stable@...r.kernel.org
> Subject: [PATCH] ice: ice_sched: fix an incorrect NULL check on list 
> iterator
>
> The bugs are here:
> 	if (old_agg_vsi_info)
> 	if (old_agg_vsi_info && !old_agg_vsi_info->tc_bitmap[0]) {
>
> 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).
>
> 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.
>

Yep. This looks correct to me.

> Reviewed-by: Jacob Keller <jacob.e.keller@...el.com>
> 
> Thanks,
> Jake

> Cc: stable@...r.kernel.org
> Fixes: 37c592062b16d ("ice: remove the VSI info from previous agg")
> Signed-off-by: Xiaomeng Tong <xiam0nd.tong@...il.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_sched.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>

Tested-by: Gurucharan <gurucharanx.g@...el.com> (A Contingent worker at Intel)