All taken from batman-adv master (b08dc3d9452d28944e7db09c2005c633b63f55e9)

1. multicast.c (dereference of NULL orig_node on at least the first round): ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

orig_node = NULL before the first entry in hlist_for_each_entry_rcu. NULL dereferenced in atomic_inc_not_zero(...) to get the refcount

Was introduced with 45dab5f5509023946793fc9d1d0566c12ca8bb52 ("batman-adv: fix more code style")

Code section: ============

static struct batadv_orig_node * batadv_mcast_forw_unsnoop_node_get(struct batadv_priv *bat_priv) { struct batadv_orig_node *tmp_orig_node, *orig_node = NULL;

rcu_read_lock(); hlist_for_each_entry_rcu(tmp_orig_node, &bat_priv->mcast.want_all_unsnoopables_list, mcast_want_all_unsnoopables_node) { if (!atomic_inc_not_zero(&orig_node->refcount)) continue;

orig_node = tmp_orig_node; break; } rcu_read_unlock();

return orig_node; }

Solution: ========

Change atomic_inc_not_zero from orig_node to tmp_orig_node

2. multicast.c (dereference of NULL orig_node on at least the first round): ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

orig_node = NULL before the first entry in hlist_for_each_entry_rcu. NULL dereferenced in atomic_inc_not_zero(...) to get the refcount

Was introduced with 45dab5f5509023946793fc9d1d0566c12ca8bb52 ("batman-adv: fix more code style")

Code section: ============

static struct batadv_orig_node * batadv_mcast_forw_ipv6_node_get(struct batadv_priv *bat_priv) { struct batadv_orig_node *tmp_orig_node, *orig_node = NULL;

rcu_read_lock(); hlist_for_each_entry_rcu(tmp_orig_node, &bat_priv->mcast.want_all_ipv6_list, mcast_want_all_ipv6_node) { if (!atomic_inc_not_zero(&orig_node->refcount)) continue;

orig_node = tmp_orig_node; break; } rcu_read_unlock();

return orig_node; }

Solution: ========

Change atomic_inc_not_zero from orig_node to tmp_orig_node

3. multicast.c (dereference of NULL orig_node on at least the first round): ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

orig_node = NULL before the first entry in hlist_for_each_entry_rcu. NULL dereferenced in atomic_inc_not_zero(...) to get the refcount

Was introduced with 45dab5f5509023946793fc9d1d0566c12ca8bb52 ("batman-adv: fix more code style")

Code section: ============

static struct batadv_orig_node * batadv_mcast_forw_ipv4_node_get(struct batadv_priv *bat_priv) { struct batadv_orig_node *tmp_orig_node, *orig_node = NULL;

rcu_read_lock(); hlist_for_each_entry_rcu(tmp_orig_node, &bat_priv->mcast.want_all_ipv4_list, mcast_want_all_ipv4_node) { if (!atomic_inc_not_zero(&orig_node->refcount)) continue;

orig_node = tmp_orig_node; break; } rcu_read_unlock();

return orig_node; }

Solution: ========

Change atomic_inc_not_zero from orig_node to tmp_orig_node

4. originator.c (Indirect hard_iface NULL dereference): ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

hard_iface is NULL and goto out is made. batadv_hardif_free_ref doesn't check for NULL before dereferencing it to get to refcount.

Introduced in f13f960797fd1969b3c0470cc97435ddfb6aecb4 ("batman-adv: add debugfs support to view multiif tables").

Code section: ============

int batadv_orig_hardif_seq_print_text(struct seq_file *seq, void *offset) { struct net_device *net_dev = (struct net_device *)seq->private; struct batadv_hard_iface *hard_iface; struct batadv_priv *bat_priv;

hard_iface = batadv_hardif_get_by_netdev(net_dev);

if (!hard_iface || !hard_iface->soft_iface) { seq_puts(seq, "Interface not known to B.A.T.M.A.N.\n"); goto out; }

[...]

out: batadv_hardif_free_ref(hard_iface); return 0; }

Solution: ========

if (hard_iface) batadv_hardif_free_ref(hard_iface);

Related code section: ====================

static inline void batadv_hardif_free_ref_now(struct batadv_hard_iface *hard_iface) { if (atomic_dec_and_test(&hard_iface->refcount)) batadv_hardif_free_rcu(&hard_iface->rcu); }

5. BONUS: fragmentation.c (Impossible code section marked likely): ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

frag_entry_curr is iterator used for list chain->head. This loop is only stopped when the list ends and frag_entry_curr is NULL. The next section following the loop says that it is likely that frag_entry_curr is NOT NULL. This is a contradiction.

Introduced in 9b3eab61754d74a93c9840c296013fe3b4a1b606 ("batman-adv: Receive fragmented packets and merge")

Code section: ============

static bool batadv_frag_insert_packet(struct batadv_orig_node *orig_node, struct sk_buff *skb, struct hlist_head *chain_out) { [...]

/* Find the position for the new fragment. */ hlist_for_each_entry(frag_entry_curr, &chain->head, list) { /* Drop packet if fragment already exists. */ if (frag_entry_curr->no == frag_entry_new->no) goto err_unlock;

/* Order fragments from highest to lowest. */ if (frag_entry_curr->no < frag_entry_new->no) { hlist_add_before(&frag_entry_new->list, &frag_entry_curr->list); chain->size += skb->len - hdr_size; chain->timestamp = jiffies; ret = true; goto out; } }

/* Reached the end of the list, so insert after 'frag_entry_curr'. */ if (likely(frag_entry_curr)) { hlist_add_after(&frag_entry_curr->list, &frag_entry_new->list); chain->size += skb->len - hdr_size; chain->timestamp = jiffies; ret = true; }

[...] }

Solution: ========

Rewrite this part to track the last seen batadv_frag_list_entry and add it after that. The empty list should already handled earlier in the scope starting with if (batadv_frag_init_chain(chain, seqno)) {

Related code section: ====================

#define hlist_for_each_entry(pos, head, member) \ for (pos = hlist_entry_safe((head)->first, typeof(*(pos)), member);\ pos; \ pos = hlist_entry_safe((pos)->member.next, typeof(*(pos)), member))