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

2007-09-25 Thread Eric W. Biederman
Patrick McHardy <[EMAIL PROTECTED]> writes:

> OK lets keep it then. Fixing the race seems overkill to me though.

Me to.

Eric

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


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

2007-09-25 Thread Patrick McHardy

Stephen Hemminger wrote:

On Tue, 25 Sep 2007 06:07:24 +0200
Patrick McHardy <[EMAIL PROTECTED]> 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 [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-09-25 Thread Stephen Hemminger
On Tue, 25 Sep 2007 06:07:24 +0200
Patrick McHardy <[EMAIL PROTECTED]> wrote:

> Stephen Hemminger wrote:
> > On Mon, 24 Sep 2007 18:55:38 +0200
> > Patrick McHardy <[EMAIL PROTECTED]> 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 [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-09-25 Thread Patrick McHardy
Eric W. Biederman wrote:
> Patrick McHardy <[EMAIL PROTECTED]> writes:
> 
>>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?
> 
> 
> Well it is cosmetic in a user space visible way.  Which means I don't
> have a clue which if any user space programs or scripts care if we change
> the behavior. 
>
> I just looked in the git history and brnf_sysctl_call_tables has been
> that way since sysctl support was added to the bridge netfilter code.
> 
> The only comment I can found about the addition is:
> 
> 2003/12/24 19:32:34-08:00 bdschuym
> [BRIDGE]: Add 4 sysctl entries for bridge netfilter behavioral control:
> bridge-nf-call-arptables - pass or don't pass bridged ARP traffic to
> arptables' FORWARD chain.
> bridge-nf-call-iptables - pass or don't pass bridged IPv4 traffic to
> iptables' chains.
> bridge-nf-filter-vlan-tagged - pass or don't pass bridged vlan-tagged
> ARP/IP traffic to arptables/iptables.
> 
> So since forcing the values to 0 or 1 doesn't seem hard to maintain
> I am uncomfortable with removing that check.


OK lets keep it then. Fixing the race seems overkill to me though.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-09-25 Thread Eric W. Biederman
Patrick McHardy <[EMAIL PROTECTED]> writes:

>> 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?

Well it is cosmetic in a user space visible way.  Which means I don't
have a clue which if any user space programs or scripts care if we change
the behavior. 

I just looked in the git history and brnf_sysctl_call_tables has been
that way since sysctl support was added to the bridge netfilter code.

The only comment I can found about the addition is:

2003/12/24 19:32:34-08:00 bdschuym
[BRIDGE]: Add 4 sysctl entries for bridge netfilter behavioral control:
bridge-nf-call-arptables - pass or don't pass bridged ARP traffic to
arptables' FORWARD chain.
bridge-nf-call-iptables - pass or don't pass bridged IPv4 traffic to
iptables' chains.
bridge-nf-filter-vlan-tagged - pass or don't pass bridged vlan-tagged
ARP/IP traffic to arptables/iptables.

So since forcing the values to 0 or 1 doesn't seem hard to maintain
I am uncomfortable with removing that check.

Eric

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


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

2007-09-25 Thread Eric W. Biederman
Patrick McHardy [EMAIL PROTECTED] writes:

 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?

Well it is cosmetic in a user space visible way.  Which means I don't
have a clue which if any user space programs or scripts care if we change
the behavior. 

I just looked in the git history and brnf_sysctl_call_tables has been
that way since sysctl support was added to the bridge netfilter code.

The only comment I can found about the addition is:

2003/12/24 19:32:34-08:00 bdschuym
[BRIDGE]: Add 4 sysctl entries for bridge netfilter behavioral control:
bridge-nf-call-arptables - pass or don't pass bridged ARP traffic to
arptables' FORWARD chain.
bridge-nf-call-iptables - pass or don't pass bridged IPv4 traffic to
iptables' chains.
bridge-nf-filter-vlan-tagged - pass or don't pass bridged vlan-tagged
ARP/IP traffic to arptables/iptables.

So since forcing the values to 0 or 1 doesn't seem hard to maintain
I am uncomfortable with removing that check.

Eric

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


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

2007-09-25 Thread Patrick McHardy
Eric W. Biederman wrote:
 Patrick McHardy [EMAIL PROTECTED] writes:
 
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?
 
 
 Well it is cosmetic in a user space visible way.  Which means I don't
 have a clue which if any user space programs or scripts care if we change
 the behavior. 

 I just looked in the git history and brnf_sysctl_call_tables has been
 that way since sysctl support was added to the bridge netfilter code.
 
 The only comment I can found about the addition is:
 
 2003/12/24 19:32:34-08:00 bdschuym
 [BRIDGE]: Add 4 sysctl entries for bridge netfilter behavioral control:
 bridge-nf-call-arptables - pass or don't pass bridged ARP traffic to
 arptables' FORWARD chain.
 bridge-nf-call-iptables - pass or don't pass bridged IPv4 traffic to
 iptables' chains.
 bridge-nf-filter-vlan-tagged - pass or don't pass bridged vlan-tagged
 ARP/IP traffic to arptables/iptables.
 
 So since forcing the values to 0 or 1 doesn't seem hard to maintain
 I am uncomfortable with removing that check.


OK lets keep it then. Fixing the race seems overkill to me though.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-09-25 Thread Stephen Hemminger
On Tue, 25 Sep 2007 06:07:24 +0200
Patrick McHardy [EMAIL PROTECTED] wrote:

 Stephen Hemminger wrote:
  On Mon, 24 Sep 2007 18:55:38 +0200
  Patrick McHardy [EMAIL PROTECTED] 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 [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-09-25 Thread Patrick McHardy

Stephen Hemminger wrote:

On Tue, 25 Sep 2007 06:07:24 +0200
Patrick McHardy [EMAIL PROTECTED] 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 [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-09-25 Thread Eric W. Biederman
Patrick McHardy [EMAIL PROTECTED] writes:

 OK lets keep it then. Fixing the race seems overkill to me though.

Me to.

Eric

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


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

2007-09-24 Thread Patrick McHardy
Stephen Hemminger wrote:
> On Mon, 24 Sep 2007 18:55:38 +0200
> Patrick McHardy <[EMAIL PROTECTED]> 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 [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-09-24 Thread Stephen Hemminger
On Mon, 24 Sep 2007 18:55:38 +0200
Patrick McHardy <[EMAIL PROTECTED]> wrote:

> Eric W. Biederman wrote:
> > [EMAIL PROTECTED] (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 <[EMAIL PROTECTED]>
> > 
> > 
> > Acked-by: "Eric W. Biederman" <[EMAIL PROTECTED]>
> 
> 
> 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 <[EMAIL PROTECTED]>

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


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

2007-09-24 Thread Patrick McHardy
Eric W. Biederman wrote:
> [EMAIL PROTECTED] (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 <[EMAIL PROTECTED]>
> 
> 
> Acked-by: "Eric W. Biederman" <[EMAIL PROTECTED]>


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?

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


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

2007-09-24 Thread Patrick McHardy
Eric W. Biederman wrote:
 [EMAIL PROTECTED] (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 [EMAIL PROTECTED]
 
 
 Acked-by: Eric W. Biederman [EMAIL PROTECTED]


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?

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


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

2007-09-24 Thread Stephen Hemminger
On Mon, 24 Sep 2007 18:55:38 +0200
Patrick McHardy [EMAIL PROTECTED] wrote:

 Eric W. Biederman wrote:
  [EMAIL PROTECTED] (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 [EMAIL PROTECTED]
  
  
  Acked-by: Eric W. Biederman [EMAIL PROTECTED]
 
 
 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 [EMAIL PROTECTED]

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


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

2007-09-24 Thread Patrick McHardy
Stephen Hemminger wrote:
 On Mon, 24 Sep 2007 18:55:38 +0200
 Patrick McHardy [EMAIL PROTECTED] 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 [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


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

2007-09-20 Thread Eric W. Biederman
[EMAIL PROTECTED] (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 <[EMAIL PROTECTED]>

Acked-by: "Eric W. Biederman" <[EMAIL PROTECTED]>

> ---
>
>This *really* needs to be reviewed by someone who knows what this
>is all about.  I've simply extended the removal of netfilter binary
>sysctl numbers so I could load bridge.ko.  I don't particularly
>care if I get attributed for this fix or any of that.
>
>It Works For Me.

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.

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


[PATCH] Remove broken netfilter binary sysctls from bridging code

2007-09-20 Thread Joseph Fannin
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 <[EMAIL PROTECTED]>

---

   This *really* needs to be reviewed by someone who knows what this
   is all about.  I've simply extended the removal of netfilter binary
   sysctl numbers so I could load bridge.ko.  I don't particularly
   care if I get attributed for this fix or any of that.

   It Works For Me.

diff -ru linux-2.6.23-rc6-mm1.orig/net/bridge/br_netfilter.c 
linux-2.6.23-rc6-mm1/net/bridge/br_netfilter.c
--- linux-2.6.23-rc6-mm1.orig/net/bridge/br_netfilter.c 2007-09-19 
02:40:49.0 -0400
+++ linux-2.6.23-rc6-mm1/net/bridge/br_netfilter.c  2007-09-20 
20:31:41.0 -0400
@@ -904,7 +904,6 @@
 
 static ctl_table brnf_table[] = {
{
-   .ctl_name   = NET_BRIDGE_NF_CALL_ARPTABLES,
.procname   = "bridge-nf-call-arptables",
.data   = _call_arptables,
.maxlen = sizeof(int),
@@ -912,7 +911,6 @@
.proc_handler   = _sysctl_call_tables,
},
{
-   .ctl_name   = NET_BRIDGE_NF_CALL_IPTABLES,
.procname   = "bridge-nf-call-iptables",
.data   = _call_iptables,
.maxlen = sizeof(int),
@@ -920,7 +918,6 @@
.proc_handler   = _sysctl_call_tables,
},
{
-   .ctl_name   = NET_BRIDGE_NF_CALL_IP6TABLES,
.procname   = "bridge-nf-call-ip6tables",
.data   = _call_ip6tables,
.maxlen = sizeof(int),
@@ -928,7 +925,6 @@
.proc_handler   = _sysctl_call_tables,
},
{
-   .ctl_name   = NET_BRIDGE_NF_FILTER_VLAN_TAGGED,
.procname   = "bridge-nf-filter-vlan-tagged",
.data   = _filter_vlan_tagged,
.maxlen = sizeof(int),
@@ -936,7 +932,6 @@
.proc_handler   = _sysctl_call_tables,
},
{
-   .ctl_name   = NET_BRIDGE_NF_FILTER_PPPOE_TAGGED,
.procname   = "bridge-nf-filter-pppoe-tagged",
.data   = _filter_pppoe_tagged,
.maxlen = sizeof(int),

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


[PATCH] Remove broken netfilter binary sysctls from bridging code

2007-09-20 Thread Joseph Fannin
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 [EMAIL PROTECTED]

---

   This *really* needs to be reviewed by someone who knows what this
   is all about.  I've simply extended the removal of netfilter binary
   sysctl numbers so I could load bridge.ko.  I don't particularly
   care if I get attributed for this fix or any of that.

   It Works For Me.

diff -ru linux-2.6.23-rc6-mm1.orig/net/bridge/br_netfilter.c 
linux-2.6.23-rc6-mm1/net/bridge/br_netfilter.c
--- linux-2.6.23-rc6-mm1.orig/net/bridge/br_netfilter.c 2007-09-19 
02:40:49.0 -0400
+++ linux-2.6.23-rc6-mm1/net/bridge/br_netfilter.c  2007-09-20 
20:31:41.0 -0400
@@ -904,7 +904,6 @@
 
 static ctl_table brnf_table[] = {
{
-   .ctl_name   = NET_BRIDGE_NF_CALL_ARPTABLES,
.procname   = bridge-nf-call-arptables,
.data   = brnf_call_arptables,
.maxlen = sizeof(int),
@@ -912,7 +911,6 @@
.proc_handler   = brnf_sysctl_call_tables,
},
{
-   .ctl_name   = NET_BRIDGE_NF_CALL_IPTABLES,
.procname   = bridge-nf-call-iptables,
.data   = brnf_call_iptables,
.maxlen = sizeof(int),
@@ -920,7 +918,6 @@
.proc_handler   = brnf_sysctl_call_tables,
},
{
-   .ctl_name   = NET_BRIDGE_NF_CALL_IP6TABLES,
.procname   = bridge-nf-call-ip6tables,
.data   = brnf_call_ip6tables,
.maxlen = sizeof(int),
@@ -928,7 +925,6 @@
.proc_handler   = brnf_sysctl_call_tables,
},
{
-   .ctl_name   = NET_BRIDGE_NF_FILTER_VLAN_TAGGED,
.procname   = bridge-nf-filter-vlan-tagged,
.data   = brnf_filter_vlan_tagged,
.maxlen = sizeof(int),
@@ -936,7 +932,6 @@
.proc_handler   = brnf_sysctl_call_tables,
},
{
-   .ctl_name   = NET_BRIDGE_NF_FILTER_PPPOE_TAGGED,
.procname   = bridge-nf-filter-pppoe-tagged,
.data   = brnf_filter_pppoe_tagged,
.maxlen = sizeof(int),

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


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

2007-09-20 Thread Eric W. Biederman
[EMAIL PROTECTED] (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 [EMAIL PROTECTED]

Acked-by: Eric W. Biederman [EMAIL PROTECTED]

 ---

This *really* needs to be reviewed by someone who knows what this
is all about.  I've simply extended the removal of netfilter binary
sysctl numbers so I could load bridge.ko.  I don't particularly
care if I get attributed for this fix or any of that.

It Works For Me.

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.

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