On Saturday, 30 November 2024 00:46:32 GMT+1 Linus Lüssing wrote:
[...]
> @@ -2233,25 +2233,16 @@ int batadv_bla_claim_dump(struct sk_buff *msg, struct netlink_callback *cb)
>  {
>  	struct batadv_hard_iface *primary_if = NULL;
>  	int portid = NETLINK_CB(cb->skb).portid;
> -	struct net *net = sock_net(cb->skb->sk);
>  	struct net_device *soft_iface;
>  	struct batadv_hashtable *hash;
>  	struct batadv_priv *bat_priv;
>  	int bucket = cb->args[0];
>  	int idx = cb->args[1];
> -	int ifindex;
>  	int ret = 0;
>  
> -	ifindex = batadv_netlink_get_ifindex(cb->nlh,
> -					     BATADV_ATTR_MESH_IFINDEX);
> -	if (!ifindex)
> -		return -EINVAL;
> -
> -	soft_iface = dev_get_by_index(net, ifindex);
> -	if (!soft_iface || !batadv_softif_is_valid(soft_iface)) {
> -		ret = -ENODEV;
> -		goto out;
> -	}
> +	soft_iface = batadv_netlink_get_softif(cb);
> +	if (IS_ERR(soft_iface))
> +		return PTR_ERR(soft_iface);



To use these helpers, you need to "#include <linux/err.h>" in various files. 
There are also other changes needed. Here an overview:

    diff --git a/net/batman-adv/bridge_loop_avoidance.c b/net/batman-adv/bridge_loop_avoidance.c
    index c38a1ac9..fa8a8246 100644
    --- a/net/batman-adv/bridge_loop_avoidance.c
    +++ b/net/batman-adv/bridge_loop_avoidance.c
    @@ -12,6 +12,7 @@
     #include <linux/compiler.h>
     #include <linux/container_of.h>
     #include <linux/crc16.h>
    +#include <linux/err.h>
     #include <linux/errno.h>
     #include <linux/etherdevice.h>
     #include <linux/gfp.h>
    @@ -38,7 +39,6 @@
     #include <net/arp.h>
     #include <net/genetlink.h>
     #include <net/netlink.h>
    -#include <net/sock.h>
     #include <uapi/linux/batadv_packet.h>
     #include <uapi/linux/batman_adv.h>
     
    @@ -47,7 +47,6 @@
     #include "log.h"
     #include "netlink.h"
     #include "originator.h"
    -#include "soft-interface.h"
     #include "translation-table.h"
     
     static const u8 batadv_announce_mac[4] = {0x43, 0x05, 0x43, 0x05};
    diff --git a/net/batman-adv/distributed-arp-table.c b/net/batman-adv/distributed-arp-table.c
    index 67bb3b17..ff660758 100644
    --- a/net/batman-adv/distributed-arp-table.c
    +++ b/net/batman-adv/distributed-arp-table.c
    @@ -11,6 +11,7 @@
     #include <linux/bitops.h>
     #include <linux/byteorder/generic.h>
     #include <linux/container_of.h>
    +#include <linux/err.h>
     #include <linux/errno.h>
     #include <linux/etherdevice.h>
     #include <linux/gfp.h>
    @@ -36,7 +37,6 @@
     #include <net/arp.h>
     #include <net/genetlink.h>
     #include <net/netlink.h>
    -#include <net/sock.h>
     #include <uapi/linux/batman_adv.h>
     
     #include "bridge_loop_avoidance.h"
    @@ -46,7 +46,6 @@
     #include "netlink.h"
     #include "originator.h"
     #include "send.h"
    -#include "soft-interface.h"
     #include "translation-table.h"
     #include "tvlv.h"
     
    diff --git a/net/batman-adv/gateway_client.c b/net/batman-adv/gateway_client.c
    index dc57b54b..8afdc711 100644
    --- a/net/batman-adv/gateway_client.c
    +++ b/net/batman-adv/gateway_client.c
    @@ -10,6 +10,7 @@
     #include <linux/atomic.h>
     #include <linux/byteorder/generic.h>
     #include <linux/container_of.h>
    +#include <linux/err.h>
     #include <linux/errno.h>
     #include <linux/etherdevice.h>
     #include <linux/gfp.h>
    @@ -31,7 +32,6 @@
     #include <linux/sprintf.h>
     #include <linux/stddef.h>
     #include <linux/udp.h>
    -#include <net/sock.h>
     #include <uapi/linux/batadv_packet.h>
     #include <uapi/linux/batman_adv.h>
     
    @@ -40,7 +40,6 @@
     #include "netlink.h"
     #include "originator.h"
     #include "routing.h"
    -#include "soft-interface.h"
     #include "translation-table.h"
     
     /* These are the offsets of the "hw type" and "hw address length" in the dhcp
    diff --git a/net/batman-adv/multicast.c b/net/batman-adv/multicast.c
    index 17b302db..5b13010d 100644
    --- a/net/batman-adv/multicast.c
    +++ b/net/batman-adv/multicast.c
    @@ -12,6 +12,7 @@
     #include <linux/bug.h>
     #include <linux/byteorder/generic.h>
     #include <linux/container_of.h>
    +#include <linux/err.h>
     #include <linux/errno.h>
     #include <linux/etherdevice.h>
     #include <linux/gfp.h>
    @@ -46,7 +47,6 @@
     #include <net/ip.h>
     #include <net/ipv6.h>
     #include <net/netlink.h>
    -#include <net/sock.h>
     #include <uapi/linux/batadv_packet.h>
     #include <uapi/linux/batman_adv.h>
     
    @@ -56,7 +56,6 @@
     #include "log.h"
     #include "netlink.h"
     #include "send.h"
    -#include "soft-interface.h"
     #include "translation-table.h"
     #include "tvlv.h"
     
    diff --git a/net/batman-adv/originator.c b/net/batman-adv/originator.c
    index 145eb110..972003d3 100644
    --- a/net/batman-adv/originator.c
    +++ b/net/batman-adv/originator.c
    @@ -9,6 +9,7 @@
     
     #include <linux/atomic.h>
     #include <linux/container_of.h>
    +#include <linux/err.h>
     #include <linux/errno.h>
     #include <linux/etherdevice.h>
     #include <linux/gfp.h>
    @@ -26,9 +27,7 @@
     #include <linux/spinlock.h>
     #include <linux/stddef.h>
     #include <linux/workqueue.h>
    -#include <net/sock.h>
     #include <uapi/linux/batadv_packet.h>
    -#include <uapi/linux/batman_adv.h>
     
     #include "bat_algo.h"
     #include "distributed-arp-table.h"
    @@ -41,7 +40,6 @@
     #include "netlink.h"
     #include "network-coding.h"
     #include "routing.h"
    -#include "soft-interface.h"
     #include "translation-table.h"
     
     /* hash class keys */
    diff --git a/net/batman-adv/translation-table.c b/net/batman-adv/translation-table.c
    index d464569e..a7680858 100644
    --- a/net/batman-adv/translation-table.c
    +++ b/net/batman-adv/translation-table.c
    @@ -15,6 +15,7 @@
     #include <linux/compiler.h>
     #include <linux/container_of.h>
     #include <linux/crc32c.h>
    +#include <linux/err.h>
     #include <linux/errno.h>
     #include <linux/etherdevice.h>
     #include <linux/gfp.h>
    @@ -39,7 +40,6 @@
     #include <linux/workqueue.h>
     #include <net/genetlink.h>
     #include <net/netlink.h>
    -#include <net/sock.h>
     #include <uapi/linux/batadv_packet.h>
     #include <uapi/linux/batman_adv.h>


[...]
> @@ -1150,23 +1137,17 @@ static int batadv_netlink_set_vlan(struct sk_buff *skb, struct genl_info *info)
>  }
>  
>  /**
> - * batadv_get_softif_from_info() - Retrieve soft interface from genl attributes
> + * batadv_netlink_get_softif_from_info() - Retrieve soft interface from ifindex
>   * @net: the applicable net namespace
> - * @info: receiver information
> + * @ifindex: index of the soft interface
>   *
>   * Return: Pointer to soft interface (with increased refcnt) on success, error
>   *  pointer on error
>   */
>  static struct net_device *
> -batadv_get_softif_from_info(struct net *net, struct genl_info *info)
> +batadv_netlink_get_softif_from_ifindex(struct net *net, int ifindex)

I think you wanted to document batadv_netlink_get_softif_from_ifindex here and 
not batadv_netlink_get_softif_from_info

[...]
> @@ -1184,28 +1165,61 @@ batadv_get_softif_from_info(struct net *net, struct genl_info *info)
>  }
>  
>  /**
> - * batadv_get_hardif_from_info() - Retrieve hardif from genl attributes
> - * @bat_priv: the bat priv with all the soft interface information
> + * batadv_get_softif_from_info() - Retrieve soft interface from genl attributes
>   * @net: the applicable net namespace
>   * @info: receiver information
>   *
> + * Return: Pointer to soft interface (with increased refcnt) on success, error
> + *  pointer on error
> + */
> +static struct net_device *
> +batadv_netlink_get_softif_from_info(struct net *net, struct genl_info *info)

I think you wanted to document batadv_netlink_get_softif_from_info here and 
not batadv_get_softif_from_info

[...]
> +/**
> + * batadv_netlink_get_hardif_from_info() - Retrieve hard interface from ifindex
> + * @bat_priv: the bat priv with all the soft interface information
> + * @net: the applicable net namespace
> + * @ifindex: index of the hard interface
> + *
>   * Return: Pointer to hard interface (with increased refcnt) on success, error
>   *  pointer on error
>   */
>  static struct batadv_hard_iface *
> -batadv_get_hardif_from_info(struct batadv_priv *bat_priv, struct net *net,
> -			    struct genl_info *info)
> +batadv_netlink_get_hardif_from_ifindex(struct batadv_priv *bat_priv,
> +				       struct net *net, int ifindex)
>  {

I think you wanted to document batadv_netlink_get_hardif_from_ifindex here and 
not batadv_netlink_get_hardif_from_info

[...]
> @@ -1229,6 +1243,51 @@ batadv_get_hardif_from_info(struct batadv_priv *bat_priv, struct net *net,
>  	return ERR_PTR(-EINVAL);
>  }
>  
> +/**
> + * batadv_get_hardif_from_info() - Retrieve hardif from genl attributes
> + * @bat_priv: the bat priv with all the soft interface information
> + * @net: the applicable net namespace
> + * @info: receiver information
> + *
> + * Return: Pointer to hard interface (with increased refcnt) on success, error
> + *  pointer on error
> + */
> +static struct batadv_hard_iface *
> +batadv_netlink_get_hardif_from_info(struct batadv_priv *bat_priv,
> +				    struct net *net, struct genl_info *info)

I think you wanted to document batadv_netlink_get_hardif_from_info here and 
not batadv_get_hardif_from_info

[...]
> diff --git a/net/batman-adv/originator.c b/net/batman-adv/originator.c
> index 8f6dd2c6ee41..9ee9655d9bea 100644
> --- a/net/batman-adv/originator.c
> +++ b/net/batman-adv/originator.c
> @@ -755,24 +755,15 @@ batadv_neigh_node_get_or_create(struct batadv_orig_node *orig_node,
>   */
>  int batadv_hardif_neigh_dump(struct sk_buff *msg, struct netlink_callback *cb)
>  {
> -	struct net *net = sock_net(cb->skb->sk);
>  	struct net_device *soft_iface;
> -	struct net_device *hard_iface = NULL;
> -	struct batadv_hard_iface *hardif = BATADV_IF_DEFAULT;
> +	struct batadv_hard_iface *hard_iface;

hard_iface is no longer initialized but you will use it after a goto (see 
below).

>  	struct batadv_priv *bat_priv;
>  	struct batadv_hard_iface *primary_if = NULL;
>  	int ret;
> -	int ifindex, hard_ifindex;
>  
> -	ifindex = batadv_netlink_get_ifindex(cb->nlh, BATADV_ATTR_MESH_IFINDEX);
> -	if (!ifindex)
> -		return -EINVAL;
> -
> -	soft_iface = dev_get_by_index(net, ifindex);
> -	if (!soft_iface || !batadv_softif_is_valid(soft_iface)) {
> -		ret = -ENODEV;
> -		goto out;
> -	}
> +	soft_iface = batadv_netlink_get_softif(cb);
> +	if (IS_ERR(soft_iface))
> +		return PTR_ERR(soft_iface);
>  
>  	bat_priv = netdev_priv(soft_iface);
>  
> @@ -782,22 +773,14 @@ int batadv_hardif_neigh_dump(struct sk_buff *msg, struct netlink_callback *cb)
>  		goto out;
>  	}

The goto here seems to be rather problematic... see below for more details

>  
> -	hard_ifindex = batadv_netlink_get_ifindex(cb->nlh,
> -						  BATADV_ATTR_HARD_IFINDEX);
> -	if (hard_ifindex) {
> -		hard_iface = dev_get_by_index(net, hard_ifindex);
> -		if (hard_iface)
> -			hardif = batadv_hardif_get_by_netdev(hard_iface);
> -
> -		if (!hardif) {
> -			ret = -ENODEV;
> +	hard_iface = batadv_netlink_get_hardif(bat_priv, cb);
> +	if (IS_ERR(hard_iface)) {
> +		if (PTR_ERR(hard_iface) != -ENONET) {
> +			ret = PTR_ERR(hard_iface);
>  			goto out;
>  		}
>  
> -		if (hardif->soft_iface != soft_iface) {
> -			ret = -ENOENT;
> -			goto out;
> -		}
> +		hard_iface = BATADV_IF_DEFAULT;
>  	}
>  
>  	if (!bat_priv->algo_ops->neigh.dump) {
> @@ -805,13 +788,12 @@ int batadv_hardif_neigh_dump(struct sk_buff *msg, struct netlink_callback *cb)
>  		goto out;
>  	}
>  
> -	bat_priv->algo_ops->neigh.dump(msg, cb, bat_priv, hardif);
> +	bat_priv->algo_ops->neigh.dump(msg, cb, bat_priv, hard_iface);
>  
>  	ret = msg->len;
>  
>   out:
> -	batadv_hardif_put(hardif);
> -	dev_put(hard_iface);
> +	batadv_hardif_put(hard_iface);
>  	batadv_hardif_put(primary_if);
>  	dev_put(soft_iface);


You will now give batadv_hardif_put an unitialized value for hard_iface when 
following goto is used:


[...]
	primary_if = batadv_primary_if_get_selected(bat_priv);
	if (!primary_if || primary_if->if_status != BATADV_IF_ACTIVE) {
		ret = -ENOENT;
		goto out;
	}

	hard_iface = batadv_netlink_get_hardif(bat_priv, cb);
[...]

This is also the reason why a non-descriptive, single return goto labels 
should be avoided [1].

Kind regards,
	Sven

[1] https://www.kernel.org/doc/html/v6.12/process/coding-style.html#centralized-exiting-of-functions