Fwd: Re: [PATCH] fix: net/batman-adv: batadv_interface_kill_vid: extra batadv_meshif_vlan_put after destroy

[Sven Eckelmann]

----------  Forwarded Message  ----------
[...]
New issues:
- [High] Removing the final `batadv_meshif_vlan_put()` call in `batadv_interface_kill_vid()` introduces a permanent reference count leak, preventing the VLAN object from ever being freed or unlinked from `meshif_vlan_list`.
[...]
--

commit 0818db7c27664fbdfdce2f408c5630f66e16ed4d
Author: WenTao Liang <vulab@xxxxxxxxxxx>

fix: net/batman-adv: batadv_interface_kill_vid: extra batadv_meshif_vlan_put after destroy

This commit removes a batadv_meshif_vlan_put() call in
batadv_interface_kill_vid() to address a suspected refcount underflow and
potential use-after-free.

> diff --git a/net/batman-adv/mesh-interface.c b/net/batman-adv/mesh-interface.c
> index 44026810b99ce..e39c9c0ffe46e 100644
> --- a/net/batman-adv/mesh-interface.c
> +++ b/net/batman-adv/mesh-interface.c
> @@ -693,9 +693,6 @@ static int batadv_interface_kill_vid(struct net_device *dev, __be16 proto,
>  
>  	batadv_meshif_destroy_vlan(bat_priv, vlan);
>  
> -	/* finally free the vlan object */
> -	batadv_meshif_vlan_put(vlan);
> -

[Severity: High]
Does removing this batadv_meshif_vlan_put() call introduce a permanent
reference count leak?

batadv_meshif_create_vlan() assigns a base refcount of 1 representing the
list's ownership. In batadv_interface_kill_vid(), batadv_meshif_vlan_get()
acquires a temporary reference, incrementing the count.
batadv_meshif_destroy_vlan() then drops the list's base reference.

Doesn't the caller still need to drop its temporary reference with this
second put() to reach a refcount of 0 and trigger
batadv_meshif_vlan_release()?

>  	return 0;
>  }
[...]

Attachment: signature.asc
Description: This is a digitally signed message part.