Re: kern/60491: arp creates state for invalid messages

["Manuel Bouyer via gnats" <gnats-admin%NetBSD.org@localhost>]

The following reply was made to PR kern/60491; it has been noted by GNATS.

From: Manuel Bouyer <bouyer%antioche.eu.org@localhost>
To: gnats-bugs%netbsd.org@localhost
Cc: kern-bug-people%netbsd.org@localhost, gnats-admin%netbsd.org@localhost, netbsd-bugs%netbsd.org@localhost,
        campbell+netbsd%mumble.net@localhost
Subject: Re: kern/60491: arp creates state for invalid messages
Date: Wed, 28 Oct 2026 15:59:31 +0100

 On Wed, Oct 21, 2026 at 04:00:52PM +0200, Manuel Bouyer wrote:
 > [ 19315.4531276] --- interrupt ---
 > 
 > The change is:
 > -       KASSERT(op == ARPOP_REPLY || op == ARPOP_REQUEST);
 > +       KASSERTMSG(op == ARPOP_REPLY || op == ARPOP_REQUEST,
 > +           "op not REQUEST or REPLY: %d", op);
 > 
 > Looks like m_makewritable() may be at fault here.
 
 
 Now running with this patch:
 
 ? sys/netinet/if_arp.c.assertmsg
 ? sys/netinet/if_arp.c.assertmsg2
 Index: sys/netinet/if_arp.c
 ===================================================================
 RCS file: /cvsroot/src/sys/netinet/if_arp.c,v
 retrieving revision 1.321
 diff -u -p -u -r1.321 if_arp.c
 --- sys/netinet/if_arp.c	23 Jul 2026 21:49:19 -0000	1.321
 +++ sys/netinet/if_arp.c	28 Oct 2026 13:56:37 -0000
 @@ -742,6 +742,8 @@ arpintr(void *arg __unused)
  				ARP_STATINC(ARP_STAT_RCVBADPROTO);
  				goto free;
  			}
 +			KASSERT(ntohs(mtod(m, struct arphdr *)->ar_op) == ARPOP_REPLY ||
 +			    ntohs(mtod(m, struct arphdr *)->ar_op) == ARPOP_REQUEST);
  			in_arpinput(m);
  			continue;
  		default:
 @@ -793,12 +795,16 @@ in_arpinput(struct mbuf *m)
  	int s;
  	char ipbuf[INET_ADDRSTRLEN];
  	bool find_source, do_dad;
 +	struct mbuf *om = m;
  
 +	KASSERT(ntohs(mtod(m, struct arphdr *)->ar_op) == ARPOP_REPLY ||
 +	    ntohs(mtod(m, struct arphdr *)->ar_op) == ARPOP_REQUEST);
  	if (__predict_false(m_makewritable(&m, 0, m->m_pkthdr.len, M_DONTWAIT)))
  		goto out;
  	ah = mtod(m, struct arphdr *);
  	op = ntohs(ah->ar_op);
 -	KASSERT(op == ARPOP_REPLY || op == ARPOP_REQUEST);
 +	KASSERTMSG(op == ARPOP_REPLY || op == ARPOP_REQUEST,
 +	    "op not REQUEST or REPLY: %d %p %p", op, m, om);
  
  	if (ah->ar_pln != sizeof(struct in_addr))
  		goto out;
 @@ -1895,7 +1901,11 @@ in_revarpinput(struct mbuf *m)
  	switch (op) {
  	case ARPOP_REQUEST:
  	case ARPOP_REPLY:	/* per RFC */
 +		KASSERT(ntohs(mtod(m, struct arphdr *)->ar_op) == ARPOP_REPLY ||
 +		    ntohs(mtod(m, struct arphdr *)->ar_op) == ARPOP_REQUEST);
  		m_put_rcvif(rcvif, &s);
 +		KASSERT(ntohs(mtod(m, struct arphdr *)->ar_op) == ARPOP_REPLY ||
 +		    ntohs(mtod(m, struct arphdr *)->ar_op) == ARPOP_REQUEST);
  		in_arpinput(m);
  		return;
  	case ARPOP_REVREPLY:
 
 I got this panic:
 [ 603891.0584629] panic: kernel diagnostic assertion "op == ARPOP_REPLY || op == ARPOP_REQUEST" failed: file "/dsk/l1/misc/bouyer/HEAD/clean/src/sys/netinet/if_arp.c", line 806 op not REQUEST or REPLY: 65535 0xffff85ca8b4ff7e0 0xffff85ca73cdebc0
 [ 603891.1323430] cpu0: Begin traceback...
 [ 603891.1440383] vpanic() at netbsd:vpanic+0x18d
 [ 603891.1584130] kern_assert() at netbsd:kern_assert+0x61
 [ 603891.1722752] in_arpinput() at netbsd:in_arpinput+0x52c
 [ 603891.1887326] arpintr() at netbsd:arpintr+0x11c
 [ 603891.1995472] softint_dispatch() at netbsd:softint_dispatch+0x124
 
 db{0}> show mbuf /v 0xffff85ca8b4ff7e0
 MBUF(0) 0xffff85ca8b4ff7e0
   data=0xffff85ca8b4ff850, len=0, type=1, flags=0x102<PKTHDR,BCAST>
   owner=0x0, next=0xffff85ca73cdebc0, nextpkt=0x0
   leadingspace=0, trailingspace=400, readonly=0
   pktlen=46, rcvif=0xffff8f0001f66060, csum_flags=0, csum_data=0x0, segsz=0
 ETHER: DST = 00:01:08:00:06:04
 ETHER: SRC = 00:01:f4:cf:e2:d4
 ETHER: TYPE = 0x4b00(unknown)
 offset 0014: 84 e3 67 01
 offset 0018: 00 00 00 00
 offset 0022: 00 00 84 e3
 offset 0026: 67 9e 00 00
 offset 0030: 00 00 00 00
 offset 0034: 00 00 00 00
 offset 0038: 00 00 00 00
 offset 0042: 00 00 00 00
 db{0}> show mbuf /v 0xffff85ca73cdebc0
 MBUF(0) 0xffff85ca73cdebc0
   data=0xffff85ca73ac680e, len=46, type=1, flags=0x9000101<EXT,BCAST,EXT_CLUSTER
 ,EXT_RW>
   owner=0x0, next=0x0, nextpkt=0x0
   leadingspace=14, trailingspace=1988, readonly=0
   ext_refcnt=1, ext_buf=0xffff85ca73ac6800, ext_size=2048, ext_free=0x0, ext_arg
 =0x0
 ETHER: DST = 00:01:08:00:06:04
 ETHER: SRC = 00:01:f4:cf:e2:d4
 ETHER: TYPE = 0x4b00(unknown)
 offset 0014: 84 e3 67 01
 offset 0018: 00 00 00 00
 offset 0022: 00 00 84 e3
 offset 0026: 67 9e 00 00
 offset 0030: 00 00 00 00
 offset 0034: 00 00 00 00
 offset 0038: 00 00 00 00
 offset 0042: 00 00 00 00
 
 So it looks like that after m_makewritable(), we end up with a mbuf with no
 data (len = 0), but which is chained with our old mbuf. But the data here don't
 make much sense; these ethernet addresses are unknown on my network and
 the ether type doesn't make sense either.
 But if we look at  0xffff85ca73ac6800 (the start of the mbuf cluster area):
 db{0}> x/x 0xffff85ca73ac6800
 ffff85ca73ac6800:       ffffffff
 db{0}> 
 ffff85ca73ac6804:       cff4ffff
 db{0}> 
 ffff85ca73ac6808:       4bd4e2
 db{0}> 
 ffff85ca73ac680c:       1000608
 db{0}> 
 ffff85ca73ac6810:       4060008
 db{0}> 
 ffff85ca73ac6814:       cff40100
 db{0}> 
 ffff85ca73ac6818:       4bd4e2
 db{0}> 
 ffff85ca73ac681c:       167e384
 db{0}> 
 ffff85ca73ac6820:       0
 db{0}> 
 ffff85ca73ac6824:       e3840000
 
 
 This gives:
 DST: ff:ff:ff:ff:ff:ff
 SRC: f4:cf:e2:d4:4b:00 (this is a known address on my network)
 ETYPE: 0x0806 (this is ETHERTYPE_ARP)
 ar_hrd: 0001 (ARPHRD_ETHER)
 
 This makes more sense if we assume fields after ETYPE have been overwritten
 (we reached the area pointed to by mbuf->data)
 
 I didn't look closely enough at m_makewritable() to explain how we could end
 up in this situation.
 Anyway I don't think we can use mtod() on a mbuf whose len is 0, and it doens't
 looks like m_makewritable() garantees that the first mbuf will contain the
 whole arp header.
 
 -- 
 Manuel Bouyer <bouyer%antioche.eu.org@localhost>
      NetBSD: 26 ans d'experience feront toujours la difference
 --