Re: [PATCH] Remove broken netfilter binary sysctls from bridging code - Kernel

This is a discussion on Re: [PATCH] Remove broken netfilter binary sysctls from bridging code - Kernel ; On Mon, 24 Sep 2007 18:55:38 +0200 Patrick McHardy wrote: > Eric W. Biederman wrote: > > jfannin@gmail.com (Joseph Fannin) writes: > > > > > >>The netfilter sysctls in the bridging code don't set strategy routines: > >> > ...

+ Reply to Thread
Results 1 to 4 of 4

Thread: Re: [PATCH] Remove broken netfilter binary sysctls from bridging code

  1. Re: [PATCH] Remove broken netfilter binary sysctls from bridging code

    On Mon, 24 Sep 2007 18:55:38 +0200
    Patrick McHardy wrote:

    > Eric W. Biederman wrote:
    > > jfannin@gmail.com (Joseph Fannin) writes:
    > >
    > >
    > >>The netfilter sysctls in the bridging code don't set strategy routines:
    > >>
    > >> sysctl table check failed: /net/bridge/bridge-nf-call-arptables .3.10.1 Missing
    > >>strategy
    > >> sysctl table check failed: /net/bridge/bridge-nf-call-iptables .3.10.2 Missing
    > >>strategy
    > >> sysctl table check failed: /net/bridge/bridge-nf-call-ip6tables .3.10.3 Missing
    > >>strategy
    > >> sysctl table check failed: /net/bridge/bridge-nf-filter-vlan-tagged .3.10.4
    > >>Missing strategy
    > >> sysctl table check failed: /net/bridge/bridge-nf-filter-pppoe-tagged .3.10.5
    > >>Missing strategy
    > >>
    > >> These binary sysctls can't work. The binary sysctl numbers of
    > >>other netfilter sysctls with this problem are being removed. These
    > >>need to go as well.
    > >>
    > >>Signed-off-by: Joseph Fannin

    > >
    > >
    > > Acked-by: "Eric W. Biederman"

    >
    >
    > Queued for 2.6.24, thanks.
    >
    > > Hmm. This is an interesting case. The proc method is forcing
    > > the integer to be either 0 or 1 in a racy fashion. But none of the
    > > users appear to depend upon that.
    > >
    > > So this is the least broken set of binary sysctls I have seen caught
    > > by my check.
    > >
    > > A really good fix would be to remove the binary side and then to
    > > modify brnf_sysctl_call_tables to allocate a temporary ctl_table and
    > > integer on the stack and only set ctl->data after we have normalized
    > > the written value. But since in practice nothing cares about
    > > the race a better fix probably isn't worth it.

    >
    >
    > I seem to be missing something, the entire brnf_sysctl_call_tables
    > thing looks purely cosmetic to me, wouldn't it be better to simply
    > remove it?



    I agree, removing seems like a better option. But probably need to go
    through a 3-6mo warning period, since sysctl's are technically an API.



    --
    Stephen Hemminger

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. Re: [PATCH] Remove broken netfilter binary sysctls from bridging code

    Stephen Hemminger wrote:
    > On Mon, 24 Sep 2007 18:55:38 +0200
    > Patrick McHardy wrote:
    >
    >>Eric W. Biederman wrote:
    >>
    >>>A really good fix would be to remove the binary side and then to
    >>>modify brnf_sysctl_call_tables to allocate a temporary ctl_table and
    >>>integer on the stack and only set ctl->data after we have normalized
    >>>the written value. But since in practice nothing cares about
    >>>the race a better fix probably isn't worth it.

    >>
    >>
    >>I seem to be missing something, the entire brnf_sysctl_call_tables
    >>thing looks purely cosmetic to me, wouldn't it be better to simply
    >>remove it?

    >
    >
    > I agree, removing seems like a better option. But probably need to go
    > through a 3-6mo warning period, since sysctl's are technically an API.



    I meant removing brnf_sysctl_call_tables function, not the sysctls
    themselves, all it does is change values != 0 to 1. Or did you
    actually mean that something in userspace might depend on reading
    back the value 1 after writing a value != 0?
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [PATCH] Remove broken netfilter binary sysctls from bridging code

    On Tue, 25 Sep 2007 06:07:24 +0200
    Patrick McHardy wrote:

    > Stephen Hemminger wrote:
    > > On Mon, 24 Sep 2007 18:55:38 +0200
    > > Patrick McHardy wrote:
    > >
    > >>Eric W. Biederman wrote:
    > >>
    > >>>A really good fix would be to remove the binary side and then to
    > >>>modify brnf_sysctl_call_tables to allocate a temporary ctl_table
    > >>>and integer on the stack and only set ctl->data after we have
    > >>>normalized the written value. But since in practice nothing cares
    > >>>about the race a better fix probably isn't worth it.
    > >>
    > >>
    > >>I seem to be missing something, the entire brnf_sysctl_call_tables
    > >>thing looks purely cosmetic to me, wouldn't it be better to simply
    > >>remove it?

    > >
    > >
    > > I agree, removing seems like a better option. But probably need to
    > > go through a 3-6mo warning period, since sysctl's are technically
    > > an API.

    >
    >
    > I meant removing brnf_sysctl_call_tables function, not the sysctls
    > themselves, all it does is change values != 0 to 1. Or did you
    > actually mean that something in userspace might depend on reading
    > back the value 1 after writing a value != 0?


    I was going farther, because don't really see the value of having
    a sysctl for this. It seems better to just not load filters if
    they aren't going to be used. Having another enable/disable hook
    just adds needless complexity.
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [PATCH] Remove broken netfilter binary sysctls from bridging code

    Stephen Hemminger wrote:
    > On Tue, 25 Sep 2007 06:07:24 +0200
    > Patrick McHardy wrote:
    >
    >
    >> I meant removing brnf_sysctl_call_tables function, not the sysctls
    >> themselves, all it does is change values != 0 to 1. Or did you
    >> actually mean that something in userspace might depend on reading
    >> back the value 1 after writing a value != 0?
    >>

    >
    > I was going farther, because don't really see the value of having
    > a sysctl for this. It seems better to just not load filters if
    > they aren't going to be used. Having another enable/disable hook
    > just adds needless complexity.
    >


    These sysctls control whether bridged packets will be handled
    by iptables and friends. The bridge netfilter code always
    handles bridged packets, and iptables might be loaded for
    different reasons. So I don't see how that would work.

    I think it should be specified in the ebtables ruleset, but
    the current netfilter infrastructure doesn't allow to do that
    cleanly.


    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread