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


Groups > linux.kernel > #1562071

Re: [RFC/PATCH 2/3] netfilter: ctnetlink: Fix regression in CTA_STATUS processing

From Doug Anderson <dianders@chromium.org>
Newsgroups linux.kernel
Subject Re: [RFC/PATCH 2/3] netfilter: ctnetlink: Fix regression in CTA_STATUS processing
Date 2017-01-18 20:10 +0100
Message-ID <t13Mm-7ZT-1@gated-at.bofh.it> (permalink)
References <t0ulz-2IQ-3@gated-at.bofh.it> <t0ulA-2IQ-13@gated-at.bofh.it>
Organization linux.* mail to news gateway

Show all headers | View raw


Hi,

On Mon, Jan 16, 2017 at 9:14 PM, Kevin Cernekee <cernekee@chromium.org> wrote:
> The libnetfilter_conntrack userland library always sets IPS_CONFIRMED
> when building a CTA_STATUS attribute.  If this toggles the bit from
> 0->1, Linux 4.4+ will reject it and this will cause any NFQA_EXP
> attribute in the packet to be ignored.  This breaks conntrackd's
> userland helpers because they operate on unconfirmed connections.

I prefer the wording that you changed it to between this patch and the
next one on the Chromium OS review system:

https://chromium-review.googlesource.com/c/428493/1..2//COMMIT_MSG

It makes it more obvious that the function has always considered some
of these things to be "errors" but that the errors simply affect
things so much in the past (before commit b7bd1809e078 ("netfilter:
nfnetlink_queue: get rid of nfnetlink_queue_ct.c")).

> Instead of returning -EBUSY if the user program asks to modify an
> unchangeable bit, simply ignore the change.
>
> Also, fix the logic so that user programs are allowed to clear
> the bits that they are allowed to change.

This is a general bugfix to the code.  If folks are using netfilter
then it seems like this ought to be backported even before 4.4.


> Signed-off-by: Kevin Cernekee <cernekee@chromium.org>
> ---
>  include/uapi/linux/netfilter/nf_conntrack_common.h |  4 ++++
>  net/netfilter/nf_conntrack_netlink.c               | 10 ++++------
>  2 files changed, 8 insertions(+), 6 deletions(-)

This bugfix is fairly easy to understand.  Assuming you agree that
it's OK to silently ignore these errors (and behave more like things
worked before kernel 4.4), then it's a clear win.

Unless someone has a better suggestion, I'd say:

Reviewed-by: Douglas Anderson <dianders@chromium.org>

Back to linux.kernel | Previous | NextPrevious in thread | Next in thread | Find similar | Unroll thread


Thread

[RFC/PATCH 0/3] Fix ctnetlink regressions Kevin Cernekee <cernekee@chromium.org> - 2017-01-17 06:20 +0100
  [RFC/PATCH 2/3] netfilter: ctnetlink: Fix regression in CTA_STATUS processing Kevin Cernekee <cernekee@chromium.org> - 2017-01-17 06:20 +0100
    Re: [RFC/PATCH 2/3] netfilter: ctnetlink: Fix regression in  CTA_STATUS processing Doug Anderson <dianders@chromium.org> - 2017-01-18 20:10 +0100
  [RFC/PATCH 1/3] netfilter: ctnetlink: Fix regression in CTA_TIMEOUT processing Kevin Cernekee <cernekee@chromium.org> - 2017-01-17 06:20 +0100
    Re: [RFC/PATCH 1/3] netfilter: ctnetlink: Fix regression in  CTA_TIMEOUT processing Doug Anderson <dianders@chromium.org> - 2017-01-18 20:00 +0100
  Re: [RFC/PATCH 0/3] Fix ctnetlink regressions Pablo Neira Ayuso <pablo@netfilter.org> - 2017-01-25 01:40 +0100

csiph-web