Please do not reply directly to this email. All additional
comments should be made in the comments box of this bug
report.
http://bugzilla.quagga.net/show_bug.cgi?id=437
------- Additional Comments From james.d.carlson@sun.com 2008-01-28 17:50 -------
The isisd flag-handling logic has a bug that can readily cause an
assertion failure. A simple configuration file that triggers the
failure is:
router isis DEAD
interface bge0
ip router isis DEAD
no ip router isis DEAD
ip router isis DEAD
The core dump looks like this:
> $c
libc.so.1`_lwp_kill+7(1, 6)
libc.so.1`raise+0x22(6)
libc.so.1`abort+0xcd(810ec7c, 0, 80ffb08, 8046b1c, feffde18, 8046b1c)
libzebra.so.0.0.0`_zlog_assert_failed+0x9b(808d6dc, 808d6cc, 2e, 8085561)
flags_get_index+0x52(810ec7c)
isis_circuit_configure+0x9c(80e1548, 810ec48)
isis_csm_state_change+0x70(1, 80e1548, 810ec48)
ip_router_isis+0xd5(809f538, 810def0, 1, 8046bac)
libzebra.so.0.0.0`cmd_execute_command_strict+0x22b(810ce80, 810def0, 0)
libzebra.so.0.0.0`config_from_file+0x189(810def0, 80a5af0)
libzebra.so.0.0.0`vty_read_config+0xea(0, 809ed78)
main+0x299(1, 8047174, 804717c)
_start+0x7a(1, 8047320, 0, 804734c, 8047358, 8047372)
> 808d6dc/s
0x808d6dc: (node)->data != 0
>
The problem is fairly obvious. We're pulling this assertion from
linklist.h:
#define listgetdata(X) (assert((X)->data != NULL), (X)->data)
when we attempt to execute this in flags_get_index():
node = listhead (flags->free_idcs);
So why is the head of this list NULL? It's because we did this in
isis_area_create():
area->flags.maxindex = -1;
Then, the first call to flags_get_index() did this:
flags->maxindex++;
index = flags->maxindex;
which means it returned 0. We then proceeded to free that value --
which is the same as NULL -- in flags_free_index():
listnode_add (flags->free_idcs, (void *) index);
That left us with a list that had a NULL node in it, setting us up for
the later failure. The simple fix for this is to use one-based
counting inside isis_flags.c, and allow the caller to use zero-based.
While we're at it, it'd be good if isis_area_create() (in isisd.c)
didn't just reach into the middle of the flag list implementation and
set maxindex to -1, so we should encapsulate that in an
flags_initialize() function.
While I was investigating this problem, I also noticed a bug in
isis_area_destroy(). It invokes isis_circuit_del() without bothering
to call isis_circuit_deconfigure() first. This means that the flag
indices are leaked on a "no router isis" command.
I have a patch for these problems; see:
http://www.opensolaris.org/os/project/rbridges/files/quagga-flags-patch.txt
A broader question is whether listnode_add() should ever allow the second
argument to be NULL. I don't think it should, given that macro definition,
but that's a much bigger (and less obvious) change than repairing the isisd
flag problem.
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
_______________________________________________
Quagga-bugs mailing list
Quagga-bugs@lists.quagga.net
http://lists.quagga.net/mailman/listinfo/quagga-bugs
comments should be made in the comments box of this bug
report.
http://bugzilla.quagga.net/show_bug.cgi?id=437
------- Additional Comments From james.d.carlson@sun.com 2008-01-28 17:50 -------
The isisd flag-handling logic has a bug that can readily cause an
assertion failure. A simple configuration file that triggers the
failure is:
router isis DEAD
interface bge0
ip router isis DEAD
no ip router isis DEAD
ip router isis DEAD
The core dump looks like this:
> $c
libc.so.1`_lwp_kill+7(1, 6)
libc.so.1`raise+0x22(6)
libc.so.1`abort+0xcd(810ec7c, 0, 80ffb08, 8046b1c, feffde18, 8046b1c)
libzebra.so.0.0.0`_zlog_assert_failed+0x9b(808d6dc, 808d6cc, 2e, 8085561)
flags_get_index+0x52(810ec7c)
isis_circuit_configure+0x9c(80e1548, 810ec48)
isis_csm_state_change+0x70(1, 80e1548, 810ec48)
ip_router_isis+0xd5(809f538, 810def0, 1, 8046bac)
libzebra.so.0.0.0`cmd_execute_command_strict+0x22b(810ce80, 810def0, 0)
libzebra.so.0.0.0`config_from_file+0x189(810def0, 80a5af0)
libzebra.so.0.0.0`vty_read_config+0xea(0, 809ed78)
main+0x299(1, 8047174, 804717c)
_start+0x7a(1, 8047320, 0, 804734c, 8047358, 8047372)
> 808d6dc/s
0x808d6dc: (node)->data != 0
>
The problem is fairly obvious. We're pulling this assertion from
linklist.h:
#define listgetdata(X) (assert((X)->data != NULL), (X)->data)
when we attempt to execute this in flags_get_index():
node = listhead (flags->free_idcs);
So why is the head of this list NULL? It's because we did this in
isis_area_create():
area->flags.maxindex = -1;
Then, the first call to flags_get_index() did this:
flags->maxindex++;
index = flags->maxindex;
which means it returned 0. We then proceeded to free that value --
which is the same as NULL -- in flags_free_index():
listnode_add (flags->free_idcs, (void *) index);
That left us with a list that had a NULL node in it, setting us up for
the later failure. The simple fix for this is to use one-based
counting inside isis_flags.c, and allow the caller to use zero-based.
While we're at it, it'd be good if isis_area_create() (in isisd.c)
didn't just reach into the middle of the flag list implementation and
set maxindex to -1, so we should encapsulate that in an
flags_initialize() function.
While I was investigating this problem, I also noticed a bug in
isis_area_destroy(). It invokes isis_circuit_del() without bothering
to call isis_circuit_deconfigure() first. This means that the flag
indices are leaked on a "no router isis" command.
I have a patch for these problems; see:
http://www.opensolaris.org/os/project/rbridges/files/quagga-flags-patch.txt
A broader question is whether listnode_add() should ever allow the second
argument to be NULL. I don't think it should, given that macro definition,
but that's a much bigger (and less obvious) change than repairing the isisd
flag problem.
------- You are receiving this mail because: -------
You are on the CC list for the bug, or are watching someone who is.
_______________________________________________
Quagga-bugs mailing list
Quagga-bugs@lists.quagga.net
http://lists.quagga.net/mailman/listinfo/quagga-bugs