netdev - Re: [PATCH net 2/2] ice: fix accounting if a VLAN already exists

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20240523194646.GQ883722@kernel.org>
Date: Thu, 23 May 2024 20:46:46 +0100
From: Simon Horman <horms@...nel.org>
To: Jacob Keller <jacob.e.keller@...el.com>
Cc: Jakub Kicinski <kuba@...nel.org>, netdev <netdev@...r.kernel.org>,
	David Miller <davem@...emloft.net>,
	Pucha Himasekhar Reddy <himasekharx.reddy.pucha@...el.com>
Subject: Re: [PATCH net 2/2] ice: fix accounting if a VLAN already exists

On Thu, May 23, 2024 at 10:45:30AM -0700, Jacob Keller wrote:
> The ice_vsi_add_vlan() function is used to add a VLAN filter for the target
> VSI. This function prepares a filter in the switch table for the given VSI.
> If it succeeds, the vsi->num_vlan counter is incremented.
> 
> It is not considered an error to add a VLAN which already exists in the
> switch table, so the function explicitly checks and ignores -EEXIST. The
> vsi->num_vlan counter is still incremented.
> 
> This seems incorrect, as it means we can double-count in the case where the
> same VLAN is added twice by the caller. The actual table will have one less
> filter than the count.
> 
> The ice_vsi_del_vlan() function similarly checks and handles the -ENOENT
> condition for when deleting a filter that doesn't exist. This flow only
> decrements the vsi->num_vlan if it actually deleted a filter.
> 
> The vsi->num_vlan counter is used only in a few places, primarily related
> to tracking the number of non-zero VLANs. If the vsi->num_vlans gets out of
> sync, then ice_vsi_num_non_zero_vlans() will incorrectly report more VLANs
> than are present, and ice_vsi_has_non_zero_vlans() could return true
> potentially in cases where there are only VLAN 0 filters left.
> 
> Fix this by only incrementing the vsi->num_vlan in the case where we
> actually added an entry, and not in the case where the entry already
> existed.
> 
> Fixes: a1ffafb0b4a4 ("ice: Support configuring the device to Double VLAN Mode")
> Signed-off-by: Jacob Keller <jacob.e.keller@...el.com>
> Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@...el.com>

Reviewed-by: Simon Horman <horms@...nel.org>