Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > muc.lists.netbsd.source-changes > #156234

Re: CVS commit: src/sys/net/npf

Show all headers | View raw

> Date: Tue, 15 Sep 2026 17:51:05 -0400
> From: Christos Zoulas <christos@zoulas.com>
> 
> > On Sep 15, 2026, at 5:18 PM, Taylor R Campbell <riastradh@netbsd.org> wrote:
> > 
> >> Date: Tue, 15 Sep 2026 17:13:27 +0000
> >> From: Taylor R Campbell <riastradh@NetBSD.org>
> >> 
> >>> @@ -87,7 +87,7 @@
> >>> 	npf_ext_route_t *meta;
> >>> 	const char *ifname;
> >>> 
> >>> -	meta = kmem_zalloc(sizeof(*meta), KM_SLEEP);
> >>> +	meta = kmem_intr_zalloc(sizeof(*meta), KM_SLEEP);
> >>> 	ifname = nvlist_get_string(params, "route-interface");
> >>> 
> >>> 	if (!ifname)
> >> 
> >> This cannot be correct.  Please revert this change and file a PR for
> >> the issue.
> > 
> > I took a closer look and I don't see how the ctor/dtor functions can
> > be called from intr or softintr context, so when you file a PR for
> > whatever issue you were trying to fix by this change, please make sure
> > to include the stack trace from the crash that presumably led you
> > here.
> 
> [...]
> kern_assert() at netbsd:kern_assert+0x4b
> kmem_free() at netbsd:kmem_free+0x89
> npf_rproc_release() at netbsd:npf_rproc_release+0x6c
> npfk_packet_handler() at netbsd:npfk_packet_handler+0x32b
> pfil_run_hooks() at netbsd:pfil_run_hooks+0x116
> ip_output() at netbsd:ip_output+0x4bd
> ip_forward() at netbsd:ip_forward+0x156
> ipintr() at netbsd:ipintr+0xeee
> softint_dispatch() at netbsd:softint_dispatch+0x112
> [...]

OK, I see the possible _static_ path now, hadn't looked hard enough
for the dtor callers.

However, I suspect the _dynamic_ path is still wrong -- while the same
subroutines npf_rproc_acquire/release are used in configuration and in
the packet-processing path, I suspect it should not be possible for
the packet-processing path to bring the reference count down to zero.

So I think this is evidence of a much more serious reference-counting
bug in the packet-processing path -- unless you were changing the
configuration just before the panic happened?

I also suspect there is no reason the packet-processing path should
even be touching the reference counts: once the configuration is in
place, the objects in question should be stable once configured, and
deconfiguring them should wait until all packet-processing paths have
observed the deconfiguration before freeing the objects.

So, please revert this change, but you can change kmem_free to
kmem_intr_free in the npf_ext_*.c *_dtor routines if you want a
workaround that is almost certainly wrong but possibly safe for now.

And, please file a PR!

--
Posted automagically by a mail2news gateway at muc.de e.V.
Please direct questions, flames, donations, etc. to news-admin@muc.de

Back to muc.lists.netbsd.source-changes | Previous | Next | Find similar | Unroll thread

Thread

Re: CVS commit: src/sys/net/npf Taylor R Campbell <riastradh@NetBSD.org> - 2026-09-16 10:47 +0000

csiph-web