[PATCH net] neighbour: do not update an existing proxy entry on SIOCSARP

[PATCH net] neighbour: do not update an existing proxy entry on SIOCSARP

[Hengyu Liang]

Commit dc2a27e524ac ("neighbour: Update pneigh_entry in
pneigh_create().") moved the update of flags, protocol and permanent
from neigh_add() into pneigh_create().

However, arp_req_set_public() also calls pneigh_create(), with all
three set to zero. As of now, SIOCSARP on an address that already has a
proxy entry clears the flags of the entry and makes it non-permanent.
The entry is then removed when the carrier goes down, and the host no
longer answers ARP requests for the address.

A similar issue happened before with the protocol. It was fixed by
commit cbada1048847 ("neighbour: restore protocol != 0 check in pneigh
update"). Perhaps flags and permanent need a fix as well.

The issue can be reproduced with:

    ip link add d0 type veth peer name d1
    ip link set d0 up
    ip link set d1 up
    ip neigh add proxy 10.9.9.9 dev d0
    arp -i d0 -Ds 10.9.9.9 d0 pub
    ip link set d1 down
    sleep 1
    ip neigh show proxy

Before commit dc2a27e524ac ("neighbour: Update pneigh_entry in
pneigh_create()."), the result is:

    10.9.9.9 dev d0 proxy

After that commit, the result is empty.

This patch will add an update argument to pneigh_create(). Only
neigh_add() sets it, so SIOCSARP leaves an existing entry alone again.

Fixes: dc2a27e524ac ("neighbour: Update pneigh_entry in pneigh_create().")
Signed-off-by: Hengyu Liang <hengyul@cs.unc.edu>
---
Tested on net: with this patch, the entry stays after the carrier goes
down and the host still answers ARP requests for the address, as before
the commit. "ip neigh add/replace/change proxy" still update an
existing entry, and SIOCSARP still creates an entry when there is none.

 include/net/neighbour.h | 2 +-
 net/core/neighbour.c    | 9 ++++++---
 net/ipv4/arp.c          | 2 +-
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index 79a5a548a129..d6d691fec124 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -394,7 +394,7 @@ struct pneigh_entry *pneigh_lookup(struct neigh_table *tbl,
 				   const void *key, struct net_device *dev);
 int pneigh_create(struct neigh_table *tbl, const void *key,
 		  struct net_device *dev, u32 flags, u8 protocol,
-		  bool permanent);
+		  bool permanent, bool update);
 int pneigh_delete(struct neigh_table *tbl, const void *key,
 		  struct net_device *dev);
 
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index d6e9c7c59a89..ae2db3912454 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -783,7 +783,7 @@ struct pneigh_entry *pneigh_lookup(struct neigh_table *tbl,
 
 int pneigh_create(struct neigh_table *tbl,
 		  const void *pkey, struct net_device *dev,
-		  u32 flags, u8 protocol, bool permanent)
+		  u32 flags, u8 protocol, bool permanent, bool update)
 {
 	struct pneigh_entry *n;
 	unsigned int key_len;
@@ -793,8 +793,11 @@ int pneigh_create(struct neigh_table *tbl,
 	mutex_lock(&tbl->phash_lock);
 
 	n = pneigh_lookup(tbl, pkey, dev);
-	if (n)
+	if (n) {
+		if (!update)
+			goto out;
 		goto update;
+	}
 
 	key_len = tbl->key_len;
 	n = kzalloc(sizeof(*n) + key_len, GFP_KERNEL);
@@ -2155,7 +2158,7 @@ static int neigh_add(struct sk_buff *skb, struct nlmsghdr *nlh,
 		}
 
 		err = pneigh_create(tbl, dst, dev, ndm_flags, protocol,
-				    !!(ndm->ndm_state & NUD_PERMANENT));
+				    !!(ndm->ndm_state & NUD_PERMANENT), true);
 		goto out;
 	}
 
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 1ca4ced280ae..3bc8db6ccd68 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -1099,7 +1099,7 @@ static int arp_req_set_public(struct net *net, struct arpreq *r,
 	if (mask) {
 		__be32 ip = ((struct sockaddr_in *)&r->arp_pa)->sin_addr.s_addr;
 
-		return pneigh_create(tbl, &ip, dev, 0, 0, false);
+		return pneigh_create(tbl, &ip, dev, 0, 0, false, false);
 	}
 
 	return arp_req_set_proxy(net, dev, 1);

base-commit: ead6fa9a0ab194f02a2f36d634b193e9c5f9a1c6
-- 
2.53.0

Re: [PATCH net] neighbour: do not update an existing proxy entry on SIOCSARP

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.