Thread (3 messages) flat view 3 messages, 2 authors, 2012-07-12

Re: [PATCH 2/2] bonding: debugfs and network namespaces are incompatible

From: Eric W. Biederman <hidden>
Date: 2012-07-12 00:18:07

Jay Vosburgh [off-list ref] writes:
Eric W. Biederman [off-list ref] wrote:
quoted
I haven't run across any of those network devices, but if they create a
debugfs entry that embeds the device name it will be a problem.
	A quick grep suggests that cxgb4, skge, sky2, stmmac, ipoib and
half a dozen of the wireless drivers all create files in debugfs.  I did
not check exhaustively, but at least some of them include the device
name.
Yep.  It looks like imperfect habits are common.
quoted
Last I looked any custom user space interface from network devices was
rare and bonding using debugfs is the first instance of using debugfs
from networking devices I have seen.

I think the problem will be a little less severe for physical network
devices as they all start in the initial network namespace and so start
with distinct names.

With bonding I can do "ip link add type bond" in any network namespace
and get another bond0.  So name conflicts are very much expeted with all
virtual networking devices.
	Fair enough, although it is trivial to rename any network device
such that a conflict would occur.
Actually for userspace and administrative reasons frequently it isn't
trivial to rename devices.
quoted
But if you know of any other networking devices using debugsfs that
code should probably get the same treatment as the bonding debugfs code.
	Is there no alternative than simply disabling debugfs whenever
network namespaces are enabled?  The information bonding displays via
debugfs is useful, and having it unavailable on all distro kernels seems
a bit harsh.

I took a good hard look at debugfs while writing this reply and debufs
scares me.  It is the kind of code that just about wants to me to throw
in the towel seeing no hope of a good solid kernel. 

I can definitely open a /sys/kernel/debug/bonding/bond0/rlb_hash_table
and delete the bond and then read the file.  On a bad day that will oops
the kernel, as there is nothing holding a reference to the network
device.  I think only the BOND_MODE_ALB check makes keeps the kernel
from oopsing in my quick tests.

The fact that debugfs is enabled in distro kernels is actually apalling
to me.  debugfs makes it easy to oops the kernel.

There are lots of alternatives to debugfs on where to put information
and the bonding driver already uses most of them.
	Why is the logic already in the driver not sufficient?  If the
attempt to create the debugfs directory with the interface name fails,
then it merely prints a warning and continues without the debugfs for
that interface.
All I know for certain is the existing logic will eventually cause
someone doing something reasonable to send me a bug report.

I can see where you are coming from in that the bonding driver debugfs
code really was built to gracefully fail and ignore problems of instead
of just hapharzardly and sloppily ignore problems.  At the same time
I can oops the kernel if I try with your debugfs in the bonding driver.

But it causes the code to fail and issue a warning.  So if I don't
disable the code now, I expect I will get a bug report, and who
knows how many sill files in bonding will have in debugfs by then.
what silly things bonding may be doing in debugfs by then.

Eric
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help