Re: pf fragment lock ordering

Hello,

diff makes sense to me.

OK sashan

On Sat, Jun 27, 2026 at 09:11:19PM +0200, Alexander Bluhm wrote:
> Hi,
> 
> syzkaller found that pf is logging debug information while holding
> a mutex.  That is not allowed.
> 
> pf fragment mutex -> log(9) -> kernel lock
> 
> https://syzkaller.appspot.com/bug?extid=de885339faa7e0a8cb83
> 
> Easiest fix is to convert pf fragment mutex to rwlock.
> 
> ok?
> 
> bluhm
> 
> Index: net/pf_norm.c
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/pf_norm.c,v
> diff -u -p -r1.238 pf_norm.c
> --- net/pf_norm.c	28 May 2026 06:41:24 -0000	1.238
> +++ net/pf_norm.c	27 Jun 2026 18:59:26 -0000
> @@ -143,9 +143,9 @@ int			 pf_reassemble6(struct mbuf **, st
>  struct pool		 pf_frent_pl, pf_frag_pl, pf_frnode_pl;
>  struct pool		 pf_state_scrub_pl;
>  
> -struct mutex		 pf_frag_mtx;
> +struct rwlock		 pf_frag_lock;
>  
> -#define PF_FRAG_LOCK_INIT()	mtx_init(&pf_frag_mtx, IPL_SOFTNET)
> +#define PF_FRAG_LOCK_INIT()	rw_init(&pf_frag_lock, "pffrag")
>  
>  void
>  pf_normalize_init(void)
> Index: net/pfvar_priv.h
> ===================================================================
> RCS file: /data/mirror/openbsd/cvs/src/sys/net/pfvar_priv.h,v
> diff -u -p -r1.42 pfvar_priv.h
> --- net/pfvar_priv.h	5 Feb 2026 03:26:00 -0000	1.42
> +++ net/pfvar_priv.h	27 Jun 2026 18:58:50 -0000
> @@ -622,7 +622,7 @@ void			 pf_state_unref(struct pf_state *
>  
>  extern struct rwlock	pf_lock;
>  extern struct rwlock	pf_state_lock;
> -extern struct mutex	pf_frag_mtx;
> +extern struct rwlock	pf_frag_lock;
>  extern struct mutex	pf_inp_mtx;
>  
>  #define PF_LOCK()		do {			\
> @@ -668,8 +668,8 @@ extern struct mutex	pf_inp_mtx;
>  			    rw_status(&pf_state_lock), __func__);\
>  	} while (0)
>  
> -#define PF_FRAG_LOCK()		mtx_enter(&pf_frag_mtx)
> -#define PF_FRAG_UNLOCK()	mtx_leave(&pf_frag_mtx)
> +#define PF_FRAG_LOCK()		rw_enter_write(&pf_frag_lock)
> +#define PF_FRAG_UNLOCK()	rw_exit_write(&pf_frag_lock)
>  
>  /* for copies to/from network byte order */
>  void			pf_state_peer_hton(const struct pf_state_peer *,
>