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

["Taylor R Campbell via gnats" <gnats-admin%NetBSD.org@localhost>]

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

From: Taylor R Campbell <riastradh%NetBSD.org@localhost>
To: gnats-bugs%NetBSD.org@localhost, netbsd-bugs%NetBSD.org@localhost
Cc: 
Subject: Re: kern/60491: arp creates state for invalid messages
Date: Fri, 16 Oct 2026 15:16:30 +0000

 This is a multi-part message in MIME format.
 --=_Jk88w0qeZbZBxabcXkD5GbHSqAbVNpLw
 
 Fixxy bug, buggy fix...
 
 --=_Jk88w0qeZbZBxabcXkD5GbHSqAbVNpLw
 Content-Type: message/rfc822
 Content-Disposition: inline
 
 Date: Fri, 16 Oct 2026 15:57:15 +0200
 From: Thomas Klausner <wiz%netbsd.org@localhost>
 To: Taylor R Campbell <riastradh%netbsd.org@localhost>
 Cc: current-users%netbsd.org@localhost
 Subject: Re: CVS commit: src/sys/netinet
 Message-ID: <atIsrv9rvTBqnr6s%exadelic.gatalith.at@localhost>
 In-Reply-To: <20260723214919.6E9F5F99A%cvs.NetBSD.org@localhost>
 References: <20260723214919.6E9F5F99A%cvs.NetBSD.org@localhost>
 MIME-Version: 1.0
 Content-Type: text/plain; charset=us-ascii
 Content-Disposition: inline
 
 On Thu, Jul 23, 2026 at 09:49:19PM +0100, Taylor R Campbell wrote:
 > Module Name:	src
 > Committed By:	riastradh
 > Date:		Thu Jul 23 21:49:19 UTC 2026
 > 
 > Modified Files:
 > 	src/sys/netinet: if_arp.c
 > 
 > Log Message:
 > arp: Validate op on input, before creating any state.
 > 
 > PR kern/60491: arp creates state for invalid messages
 
 I just had a panic triggered by this message:
 kernel diagnostic assertion "op == ARPOP_REPLY || op == ARPOP_REQUEST" failed: file "/usr/src/sys/netinet/if_arp.c", line 801
 
 I have never seen this one before.
 
 The only unusual thing I had going was a qemu with NetBSD downloading
 pkgsrc (via a tap bridge).
 
 A core dump is available.
  Thomas
 
 
 > 
 > To generate a diff of this commit:
 > cvs rdiff -u -r1.320 -r1.321 src/sys/netinet/if_arp.c
 > 
 > Please note that diffs are not public domain; they are subject to the
 > copyright notices on the relevant files.
 > 
 
 > Modified files:
 > 
 > Index: src/sys/netinet/if_arp.c
 > diff -u src/sys/netinet/if_arp.c:1.320 src/sys/netinet/if_arp.c:1.321
 > --- src/sys/netinet/if_arp.c:1.320	Fri May 29 02:45:14 2026
 > +++ src/sys/netinet/if_arp.c	Thu Jul 23 21:49:19 2026
 > @@ -1,4 +1,4 @@
 > -/*	$NetBSD: if_arp.c,v 1.320 2026/05/29 02:45:14 ozaki-r Exp $	*/
 > +/*	$NetBSD: if_arp.c,v 1.321 2026/07/23 21:49:19 riastradh Exp $	*/
 >  
 >  /*
 >   * Copyright (c) 1998, 2000, 2008 The NetBSD Foundation, Inc.
 > @@ -68,7 +68,7 @@
 >   */
 >  
 >  #include <sys/cdefs.h>
 > -__KERNEL_RCSID(0, "$NetBSD: if_arp.c,v 1.320 2026/05/29 02:45:14 ozaki-r Exp $");
 > +__KERNEL_RCSID(0, "$NetBSD: if_arp.c,v 1.321 2026/07/23 21:49:19 riastradh Exp $");
 >  
 >  #ifdef _KERNEL_OPT
 >  #include "opt_ddb.h"
 > @@ -734,6 +734,14 @@ arpintr(void *arg __unused)
 >  		switch (ntohs(ar->ar_pro)) {
 >  		case ETHERTYPE_IP:
 >  		case ETHERTYPE_IPTRAILERS:
 > +			switch (ntohs(ar->ar_op)) {
 > +			case ARPOP_REQUEST:
 > +			case ARPOP_REPLY:
 > +				break;
 > +			default:
 > +				ARP_STATINC(ARP_STAT_RCVBADPROTO);
 > +				goto free;
 > +			}
 >  			in_arpinput(m);
 >  			continue;
 >  		default:
 > @@ -790,6 +798,7 @@ in_arpinput(struct mbuf *m)
 >  		goto out;
 >  	ah = mtod(m, struct arphdr *);
 >  	op = ntohs(ah->ar_op);
 > +	KASSERT(op == ARPOP_REPLY || op == ARPOP_REQUEST);
 >  
 >  	if (ah->ar_pln != sizeof(struct in_addr))
 >  		goto out;
 > @@ -1112,8 +1121,8 @@ reply:
 >  		la = NULL;
 >  	}
 >  	if (op != ARPOP_REQUEST) {
 > -		if (op == ARPOP_REPLY)
 > -			ARP_STATINC(ARP_STAT_RCVREPLY);
 > +		KASSERT(op == ARPOP_REPLY);
 > +		ARP_STATINC(ARP_STAT_RCVREPLY);
 >  		goto out;
 >  	}
 >  	ARP_STATINC(ARP_STAT_RCVREQUEST);
 > 
 
 
 --=_Jk88w0qeZbZBxabcXkD5GbHSqAbVNpLw--