[PATCH] net: hns3: Add error handling for VLAN filter hardware configuration

[PATCH] net: hns3: Add error handling for VLAN filter hardware configuration

[Wentao Liang]

The hclge_rm_vport_vlan_table() calls hclge_set_vlan_filter_hw() but does
not check the return value. This could lead to execution with potentially
invalid data. A proper implementation can be found in
hclge_add_vport_all_vlan_table().

Add error handling after calling hclge_set_vlan_filter_hw(). If
hclge_set_vlan_filter_hw() fails, log an error message via dev_err() and
return.

Fixes: c6075b193462 ("net: hns3: Record VF vlan tables")
Cc: stable@vger.kernel.org # v5.1
Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
---
 .../hisilicon/hns3/hns3pf/hclge_main.c        | 20 +++++++++++++------
 1 file changed, 14 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
index db7845009252..5ab4c7f63766 100644
--- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
+++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
@@ -10141,15 +10141,23 @@ static void hclge_rm_vport_vlan_table(struct hclge_vport *vport, u16 vlan_id,
 {
 	struct hclge_vport_vlan_cfg *vlan, *tmp;
 	struct hclge_dev *hdev = vport->back;
+	int ret;
 
 	list_for_each_entry_safe(vlan, tmp, &vport->vlan_list, node) {
 		if (vlan->vlan_id == vlan_id) {
-			if (is_write_tbl && vlan->hd_tbl_status)
-				hclge_set_vlan_filter_hw(hdev,
-							 htons(ETH_P_8021Q),
-							 vport->vport_id,
-							 vlan_id,
-							 true);
+			if (is_write_tbl && vlan->hd_tbl_status) {
+				ret = hclge_set_vlan_filter_hw(hdev,
+							       htons(ETH_P_8021Q),
+							       vport->vport_id,
+							       vlan_id,
+							       true);
+				if (ret) {
+					dev_err(&hdev->pdev->dev,
+						"restore vport vlan list failed, ret=%d\n",
+						ret);
+					return;
+				}
+			}
 
 			list_del(&vlan->node);
 			kfree(vlan);
-- 
2.42.0.windows.2

Re: [PATCH] net: hns3: Add error handling for VLAN filter hardware configuration

[Jijie Shao]

on 2025/5/17 22:15, Wentao Liang wrote:
> The hclge_rm_vport_vlan_table() calls hclge_set_vlan_filter_hw() but does
> not check the return value. This could lead to execution with potentially
> invalid data. A proper implementation can be found in

Hi:

Are there any real functional problems?
Would you please tell me your test cases? I'm going to try to reproduce the problem.

Thanks,
Jijie Shao

> hclge_add_vport_all_vlan_table().
>
> Add error handling after calling hclge_set_vlan_filter_hw(). If
> hclge_set_vlan_filter_hw() fails, log an error message via dev_err() and
> return.
>
> Fixes: c6075b193462 ("net: hns3: Record VF vlan tables")
> Cc: stable@vger.kernel.org # v5.1
> Signed-off-by: Wentao Liang <vulab@iscas.ac.cn>
> ---
>   .../hisilicon/hns3/hns3pf/hclge_main.c        | 20 +++++++++++++------
>   1 file changed, 14 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> index db7845009252..5ab4c7f63766 100644
> --- a/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> +++ b/drivers/net/ethernet/hisilicon/hns3/hns3pf/hclge_main.c
> @@ -10141,15 +10141,23 @@ static void hclge_rm_vport_vlan_table(struct hclge_vport *vport, u16 vlan_id,
>   {
>   	struct hclge_vport_vlan_cfg *vlan, *tmp;
>   	struct hclge_dev *hdev = vport->back;
> +	int ret;
>   
>   	list_for_each_entry_safe(vlan, tmp, &vport->vlan_list, node) {
>   		if (vlan->vlan_id == vlan_id) {
> -			if (is_write_tbl && vlan->hd_tbl_status)
> -				hclge_set_vlan_filter_hw(hdev,
> -							 htons(ETH_P_8021Q),
> -							 vport->vport_id,
> -							 vlan_id,
> -							 true);
> +			if (is_write_tbl && vlan->hd_tbl_status) {
> +				ret = hclge_set_vlan_filter_hw(hdev,
> +							       htons(ETH_P_8021Q),
> +							       vport->vport_id,
> +							       vlan_id,
> +							       true);
> +				if (ret) {
> +					dev_err(&hdev->pdev->dev,
> +						"restore vport vlan list failed, ret=%d\n",
> +						ret);
> +					return;
> +				}
> +			}
>   
>   			list_del(&vlan->node);
>   			kfree(vlan);

Re: [PATCH] net: hns3: Add error handling for VLAN filter hardwareconfiguration

[Simon Horman]

On Tue, May 20, 2025 at 09:14:28AM +0800, Wentao Liang wrote:
> > Are there any real functional problems?
> > Would you please tell me your test cases? I'm going to try to reproduce the problem.
> 
> Hi,
> 
> I found this problem by static analysis and manual auditing. I believe there should be add a check just like the other call of hclge_set_vlan_filter_hw(). If the check is useless, could you please tell me the detailed reason.

Thanks,

I think it would be useful for the patch description to state
that the problem was found using static analysis.

But let's wait for more feedback from Jijie on if this
check is, at least theoretically, required.

Re: [PATCH] net: hns3: Add error handling for VLAN filter hardwareconfiguration

[Jijie Shao]

on 2025/5/20 9:14, Wentao Liang wrote:
>
> > Are there any real functional problems?
>
> > Would you please tell me your test cases? I'm going to try to reproduce the problem.
>
> Hi,
>
> I found this problem by static analysis and manual auditing. I believe 
> there should be add a check just like the other call of 
> hclge_set_vlan_filter_hw(). If the check is useless, could you please 
> tell me the detailed reason.
>
Hi Wentao,
  
Thanks for your reply.

I checked the code, and found that the purpose of all two places called hclge_rm_vport_vlan_table() now is just to update the vlan list,
never do clear the vlan entry in hardware. So I prefer to remove the parameter 'is_write_tbl' from hclge_rm_vport_vlan_table(),
and remove the hclge_set_vlan_filter_hw() calling in it.

Thanks,
Jijie Shao

> Thanks,
>
> Wentao Liang.
>