Attachment 254881 Details for Bug 282478 – ipfilter: Avoid stopping with a lock held

[patch] ipfilter: Avoid stopping with a lock held

0001-ipfilter-Avoid-stopping-with-a-lock-held.patch (text/plain), 3.14 KB, created by Jose Luis Duran on 2024-11-02 18:17:10 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jose Luis Duran

Created: 2024-11-02 18:17:10 UTC

Size: 3.14 KB

patch

obsolete

>From a56f46b8558a74d3ad09e4d7241f2e97bcb47f07 Mon Sep 17 00:00:00 2001
>From: Jose Luis Duran <jlduran@FreeBSD.org>
>Date: Sat, 2 Nov 2024 17:58:59 +0000
>Subject: [PATCH] ipfilter: Avoid stopping with a lock held
>
>Avoid calling _callout_stop_safe with a non-sleepable lock held by:
>
>  1. Using callout_init_rw() to initialize ipf_slow_ch, and remove the
>     locking in ipf_timer_func().
>  2. Using callout_stop() instead of callout_drain().
>
>The global ipf lock is not dropped while executing ipf_timer_func().
>Then callout_stop() can cancel a pending callout without sleeping,
>avoiding the problem that triggered the following WITNESS warning when
>stopping the service:
>
>    # service ipfilter stop
>    calling _callout_stop_safe with the following non-sleepable locks held:
>    shared rw ipf filter load/unload mutex (ipf filter load/unload mutex) r = 0 (0xffff0000417c7530) locked @ /usr/src/sys/netpfil/ipfilter/netinet/fil.c:7926
>    stack backtrace:
>    #0 0xffff00000052d394 at witness_debugger+0x60
>    #1 0xffff00000052e620 at witness_warn+0x404
>    #2 0xffff0000004d4ffc at _callout_stop_safe+0x8c
>    #3 0xffff0000f7236674 at ipfdetach+0x3c
>    #4 0xffff0000f723fa4c at ipf_ipf_ioctl+0x788
>    #5 0xffff0000f72367e0 at ipfioctl+0x144
>    #6 0xffff00000034abd8 at devfs_ioctl+0x100
>    #7 0xffff0000005c66a0 at vn_ioctl+0xbc
>    #8 0xffff00000034b2cc at devfs_ioctl_f+0x24
>    #9 0xffff0000005331ec at kern_ioctl+0x2e0
>    #10 0xffff000000532eb4 at sys_ioctl+0x140
>    #11 0xffff000000880480 at do_el0_sync+0x604
>    #12 0xffff0000008579ac at handle_el0_sync+0x4c
>
>PR:		282478
>Suggested by:	markj
>---
> sys/netpfil/ipfilter/netinet/ip_fil_freebsd.c | 8 +++-----
> 1 file changed, 3 insertions(+), 5 deletions(-)
>
>diff --git a/sys/netpfil/ipfilter/netinet/ip_fil_freebsd.c b/sys/netpfil/ipfilter/netinet/ip_fil_freebsd.c
>index bcde0d2c7323..0575ae52c8bd 100644
>--- a/sys/netpfil/ipfilter/netinet/ip_fil_freebsd.c
>+++ b/sys/netpfil/ipfilter/netinet/ip_fil_freebsd.c
>@@ -172,7 +172,6 @@ ipf_timer_func(void *arg)
> 	SPL_INT(s);
> 
> 	SPL_NET(s);
>-	READ_ENTER(&softc->ipf_global);
> 
> 	if (softc->ipf_running > 0)
> 		ipf_slowtimer(softc);
>@@ -181,12 +180,11 @@ ipf_timer_func(void *arg)
> #if 0
> 		softc->ipf_slow_ch = timeout(ipf_timer_func, softc, hz/2);
> #endif
>-		callout_init(&softc->ipf_slow_ch, 1);
>+		callout_init_rw(&softc->ipf_slow_ch, &softc->ipf_global.ipf_lk, 0);
> 		callout_reset(&softc->ipf_slow_ch,
> 			(hz / IPF_HZ_DIVIDE) * IPF_HZ_MULT,
> 			ipf_timer_func, softc);
> 	}
>-	RWLOCK_EXIT(&softc->ipf_global);
> 	SPL_X(s);
> }
> 
>@@ -221,7 +219,7 @@ ipfattach(ipf_main_softc_t *softc)
> 	softc->ipf_slow_ch = timeout(ipf_timer_func, softc,
> 				     (hz / IPF_HZ_DIVIDE) * IPF_HZ_MULT);
> #endif
>-	callout_init(&softc->ipf_slow_ch, 1);
>+	callout_init_rw(&softc->ipf_slow_ch, &softc->ipf_global.ipf_lk, 0);
> 	callout_reset(&softc->ipf_slow_ch, (hz / IPF_HZ_DIVIDE) * IPF_HZ_MULT,
> 		ipf_timer_func, softc);
> 	return (0);
>@@ -249,7 +247,7 @@ ipfdetach(ipf_main_softc_t *softc)
> 		untimeout(ipf_timer_func, softc, softc->ipf_slow_ch);
> 	bzero(&softc->ipf_slow, sizeof(softc->ipf_slow));
> #endif
>-	callout_drain(&softc->ipf_slow_ch);
>+	callout_stop(&softc->ipf_slow_ch);
> 
> 	ipf_fini_all(softc);
> 
>-- 
>Jose Luis Duran
>

Actions: View | Diff

Attachments on bug 282478: 254881 | 254890 | 254917 | 254918