Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider.
Critical: 0 · High: 0 · Medium: 1 · Low: 0
If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).
- [Medium] The Fixes: tag in commit "ice: release the VF reference in the
switchdev trust path" points at the wrong commit.
--
ice: release the VF reference in the switchdev trust path
In ice_set_vf_trust(), the VF reference taken by ice_get_vf_by_id() was
not dropped when the request was rejected because the PF is in switchdev
mode. This patch sends that path through the existing out_put_vf label
so ice_put_vf() is called.
> ice: release the VF reference in the switchdev trust path
>
> ice_set_vf_trust() takes a reference on the VF with ice_get_vf_by_id()
> and then rejects the request when the PF is in switchdev mode without
> dropping that reference.
>
> Return through the existing out_put_vf label so the VF reference is
> released on this path too.
>
> Fixes: 1281b7459657 ("ice: Forbid trusted VFs in switchdev mode")
[Severity: Medium]
Is this the right commit for the Fixes: tag?
When 1281b7459657 added the switchdev check, it was at the very top of
ice_set_vf_trust(), before any VF lookup. VF refcounting (3d5985a185e6)
did not exist yet, so the bare return did not leak anything.
The leak seems to come from 83b49e7f63da ("ice: check if VF exists before
mode check"). That commit moved the lookup above the switchdev check and
did not change the early return:
+ vf = ice_get_vf_by_id(pf, vf_id);
+ if (!vf)
+ return -EINVAL;
+
if (ice_is_eswitch_mode_switchdev(pf)) {
dev_info(ice_pf_to_dev(pf), "Trusted VF is forbidden in
switchdev mode\n");
return -EOPNOTSUPP;
}
- vf = ice_get_vf_by_id(pf, vf_id);
- if (!vf)
- return -EINVAL;
-
With Cc: stable, the current tag would aim at trees that have
1281b7459657 but not 83b49e7f63da. Those trees do not have the leak.
There, ice_get_vf_by_id() still comes after the switchdev check, so a
fuzzed backport would add a goto out_put_vf that runs before vf is
assigned.
Should this be the following instead?
Fixes: 83b49e7f63da ("ice: check if VF exists before mode check")
The phrase "takes a reference on the VF with ice_get_vf_by_id() and then
rejects the request" also reads as if the check has always leaked. That
has only been true since 83b49e7f63da.
> Cc: [email protected]
> Assisted-by: DeepSeek-V4.1-Flash and OpenAI Codex
> Signed-off-by: Wentao Liang <[email protected]>
> diff --git a/drivers/net/ethernet/intel/ice/ice_sriov.c
> b/drivers/net/ethernet/intel/ice/ice_sriov.c
> index c49f48b5ae38c..90bf1becbc3da 100644
> --- a/drivers/net/ethernet/intel/ice/ice_sriov.c
> +++ b/drivers/net/ethernet/intel/ice/ice_sriov.c
> @@ -1400,7 +1400,8 @@ int ice_set_vf_trust(struct net_device *netdev, int
> vf_id, bool trusted)
>
> if (ice_is_eswitch_mode_switchdev(pf)) {
> dev_info(ice_pf_to_dev(pf), "Trusted VF is forbidden in
> switchdev mode\n");
> - return -EOPNOTSUPP;
> + ret = -EOPNOTSUPP;
> + goto out_put_vf;
> }
>
> ret = ice_check_vf_ready_for_cfg(vf);
--
Sashiko AI review ·
https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261103100530.1309907-1-vulab%40iscas.ac.cn
