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-...