Package: bridge-utils
Version: 1.1-1
Severity: important
Tags: patch

Shortly

some libsysfs2 internal stucts are corrupted during execution of brctl
by unneeded and erroneous calls to sysfs_close_class_device()

Long description of my troubles

On my xen Dom0 with 5 bridges and many interfaces every brctl show
invocations fails with a strange error.

# brctl show
bridge name     bridge id               STP enabled     interfaces
xenbr0          8000.feffffffffff       no              vif1.0
                                                        peth0
                                                        vif0.0
xenbr1          8000.feffffffffff       no              vif3.0
                                                        vif2.0
                                                        vif1.1
                                                        peth1
                                                        vif0.1
xenbr2          8000.feffffffffff       no              vif1.2
                                                        peth2
                                                        vif0.2
xenbr3          8000.feffffffffff       no               can't get port info: 
No such device
xenbr4          8000.feffffffffff       no              vif2.2
                                                        peth4
                                                        vif0.4

but xenbr3 was fully up and functional, as stated by the contents of
/sys/class/net/xenbr3 directory.

Investigating with strace i've found that brctl give this error
because it is trying to read xenbr3 interfaces in
/sys/class/net/eth0/brif directory!

.....
lstat64("/sys/class/net/xenbr3/bridge/topology_change_detected", 
{st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
stat64("/sys/class/net/xenbr3/bridge/topology_change_detected", 
{st_mode=S_IFREG|0444, st_size=4096, ...}) = 0
open("/sys/class/net/xenbr3/bridge/topology_change_detected", O_RDONLY) = 4
read(4, "0\n", 4096)                    = 2
close(4)                                = 0
open("/sys/class/net/eth0/brif", O_RDONLY|O_NONBLOCK|O_LARGEFILE|O_DIRECTORY) = 
-1 ENOENT (No such file or directory)
ioctl(3, SIOCDEVPRIVATE, 0xafdec36c)    = -1 ENODEV (No such device)
write(1, "8000.feffffffffff\tno\t\t can\'t get"..., 59) = 59
lstat64("/sys/class/net/xenbr4/bridge", {st_mode=S_IFDIR|0755, st_size=0, ...}) 
= 0
write(1, "xenbr4\t\t", 8)               = 8
.....

after some gdb and documentation reading sessions i've discovered that
all calls to sysfs_close_class_device() in libbridge code are wrong,
because their arguments are pointers returned by sysfs_get_*
functions. This is explicitly forbidden by libsysfs documentation[1]
at 6.1.b and corrupt libsysfs internal structures freeing memory that
still being used.

[1] /usr/share/doc/libsysfs-dev/libsysfs.txt.gz

.....
6.1 Calling Conventions in Libsysfs
-----------------------------------

Libsysfs uses a simple API calling convention. APIs are classified to be
one of "open", "get", "close" types. The convention is as follows:

        a. All "open" APIs have a corresponding "close" API.
        b. References obtained using "get" calls should not be closed
                explicitly.
        c. All "opened" references have to be closed with a call to
                their corresponding "close" call. This takes care of
                freeing structure references obtained with "get" calls.
.....

I have checked all other sysfs_close_* calls and they seem to be
correct.

Attached patch removes the wrong call, so i can operate correctly on
my bridges.

Best Regards

-- System Information:
Debian Release: testing/unstable
  APT prefers unstable
  APT policy: (990, 'unstable'), (1, 'experimental')
Architecture: i386 (i686)
Shell:  /bin/sh linked to /bin/bash
Kernel: Linux 2.6.16-2-686
Locale: LANG=it_IT.UTF-8, LC_CTYPE=it_IT.UTF-8 (charmap=UTF-8)

Versions of packages bridge-utils depends on:
ii  libc6                         2.3.6-15   GNU C Library: Shared libraries
ii  libsysfs2                     2.0.0-7    interface library to sysfs

Versions of packages bridge-utils recommends:
ii  ifupdown                      0.6.7      high level tools to configure netw

-- no debconf information

-- 
---------------------------------------------------------------------
|    Marco Nenciarini    | Debian/GNU Linux Developer - Plug Member |
| [EMAIL PROTECTED] | http://www.prato.linux.it/~mnencia       |
---------------------------------------------------------------------
Key fingerprint = FED9 69C7 9E67 21F5 7D95  5270 6864 730D F095 E5E4

diff -ruN bridge-utils-1.1/libbridge/libbridge_devif.c 
bridge-utils-1.1.my/libbridge/libbridge_devif.c
--- bridge-utils-1.1/libbridge/libbridge_devif.c        2006-02-01 
18:58:00.000000000 +0100
+++ bridge-utils-1.1.my/libbridge/libbridge_devif.c     2006-07-05 
18:33:24.000000000 +0200
@@ -188,7 +188,6 @@
        snprintf(path, SYSFS_PATH_MAX, "%s/bridge", dev->path);
        if (sysfs_path_is_dir(path)) {
                dprintf("path '%s' is not a directory\n", path);
-               sysfs_close_class_device(dev);
                goto fallback;
        }
 
@@ -214,7 +213,6 @@
        info->topology_change = fetch_int(dev, BRIDGEATTR("topology_change"));
        info->topology_change_detected = fetch_int(dev, 
                                                   
BRIDGEATTR("topology_change_detected"));
-       sysfs_close_class_device(dev);
 
        return 0;
 
@@ -287,7 +285,6 @@
 
        snprintf(path, SYSFS_PATH_MAX, "%s/brport", dev->path);
        if (sysfs_path_is_dir(path)) {
-               sysfs_close_class_device(dev);
                goto fallback;
        }
 
@@ -309,7 +306,6 @@
                 &info->forward_delay_timer_value);
        fetch_tv(dev, BRPORT("hold_timer"),
                 &info->hold_timer_value);
-       sysfs_close_class_device(dev);
 
        return 0;
 fallback:
@@ -339,7 +335,6 @@
                        ret = sysfs_write_attribute(attr, buf, strlen(buf));
                        sysfs_close_attribute(attr);
                }
-               sysfs_close_class_device(dev);
        } else
 #endif
        {
@@ -411,7 +406,6 @@
                        ret = sysfs_write_attribute(attr, buf, strlen(buf));
                        sysfs_close_attribute(attr);
                }
-               sysfs_close_class_device(dev);
        } else
 #endif
        {

Attachment: signature.asc
Description: Digital signature

Reply via email to