Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> Hi,
>
> Cc'ing Florian.
>
> On Wed, Jun 14, 2023 at 06:24:05PM +0200, Jacek Tomasiak wrote:
> > When creating new rules with (e.g. with `conntrack -I -m 123 -u UNSET ...`),
> > the mark from `-m` was overriden by value from `-u`. Additional
> > condition ensures that this happens only in list mode.
> >
> > This behavior was introduced in 1a5828f491c6a1593f30cb5f1551fe9f9cf76a8d
> > ("conntrack: enable kernel-based status filtering with -L -u STATUS") for
> > filtering the output of `-L` option but caused a regression in other cases.
>
> In 1a5828f491c6a:
>
> tmpl->mark.value = status;
> tmpl->filter_status_kernel.val = tmpl->mark.value;
>
> Not sure what the mark has to do this the -L -u STATUS filtering.
>
> > Signed-off-by: Jacek Tomasiak <jtomasiak@xxxxxxxxxx>
> > Signed-off-by: Jacek Tomasiak <jacek.tomasiak@xxxxxxxxx>
> > ---
> > src/conntrack.c | 4 +++-
> > 1 file changed, 3 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/conntrack.c b/src/conntrack.c
> > index bf72739..78d3a07 100644
> > --- a/src/conntrack.c
> > +++ b/src/conntrack.c
> > @@ -3007,7 +3007,9 @@ static void do_parse(struct ct_cmd *ct_cmd, int argc, char *argv[])
> > if (tmpl->filter_status_kernel.mask == 0)
> > tmpl->filter_status_kernel.mask = status;
> >
> > - tmpl->mark.value = status;
> > + // set mark only in list mode to not override value from -m
> > + if (command & CT_LIST)
> > + tmpl->mark.value = status;
This should be
- tmpl->mark.value = status;
- tmpl->filter_status_kernel.val = tmpl->mark.value;
+ tmpl->filter_status_kernel.val = status;
tmpl->filter_status_kernel_set = true;
> The existing code also means that -L -u STATUS cannot be combined with
> -L -m mark, right?
This is a bug.
conntrack -L -u status -m 1 will work, -m 1 -u status won't work.
The clobbering of mark utterly bogus.
