On Fri, Jun 13, 2025 at 12:42:18AM +0200, Pablo Neira Ayuso wrote:
> On Thu, Jun 12, 2025 at 08:30:24PM +0200, Phil Sutter wrote:
> > All routines modified in this patch conditionally return early depending
> > on event value (and other criteria, i.e., chain/flowtable updates).
> > These checks were defeated by an upfront modification of that variable
> > for use in nfnl_msg_put(). Restore functionality by avoiding the
> > modification.
>
> Thanks for fixing this.
Took me more than a moment to notice! I guess 'var = func(var)' is
convenient, but also bad practice. :)
> > This change is particularly important for user space to distinguish
> > between a chain/flowtable update removing a hook and full deletion.
> >
> > Fixes: 28339b21a365 ("netfilter: nf_tables: do not send complete notification of deletions")
> > Signed-off-by: Phil Sutter <phil@xxxxxx>
> > ---
> > Channeling this through -next despite it being a fix since unpatched
> > nft monitor chokes on the shortened delete flowtable notifications.
>
> I am afraid this patch will end up in -stable, breaking userspace, how
> bad is the choking? Maybe 28339b21a365 needs to be reverted, then fix
> userspace to prepare for it and re-add it in nf-next?
Oh right, the Fixes: tag will probably cause that. User space segfaults
dereferencing a NULL-ptr. Happens in netlink_delinearize_{obj,flowtable}
which are called during cache population, ergo all users affected.
> Not sure what path to follow with this.
If dropping the Fixes: tag was sufficient, there remains a risk that
someone else notices the bug and fixes it. If we do treat the revert of
28339b21a365 as a "fix", can we legally tag it as fixing itself? :D
If so, I'd do that and reintroduce the feature in bug-free form.
Thanks, Phil
