Stephen Hemminger wrote:
On Mon, 24 Sep 2007 18:55:38 +0200
Patrick McHardy [off-list ref] wrote:
quoted
Eric W. Biederman wrote:
quoted
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?
On Tue, 25 Sep 2007 06:07:24 +0200
Patrick McHardy [off-list ref] wrote:
Stephen Hemminger wrote:
quoted
On Mon, 24 Sep 2007 18:55:38 +0200
Patrick McHardy [off-list ref] wrote:
quoted
Eric W. Biederman wrote:
quoted
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.
Stephen Hemminger wrote:
On Tue, 25 Sep 2007 06:07:24 +0200
Patrick McHardy [off-list ref] wrote:
quoted
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.