Renato Westphal [Tue, 10 Oct 2017 12:22:41 +0000 (09:22 -0300)]
ldpd: fix heap-use-after-free at exit
This problems happens because, in this port, whenever the child
processes want to log something they send a message to the parent. But
in the shutdown functions the first thing we do is to close the pipes
to the parent process. With that said, add some protections to prevent
the child processes from trying to use a closed pipe and just ignore
their log messages during shutdown. In the future we need to share
the logging configuration with the child processes so they can send
log messages on their own.
While here, remove some unnecessary calls to msgbuf_write() in
ldpe_shutdown().
Vincent JARDIN [Mon, 9 Oct 2017 10:42:11 +0000 (12:42 +0200)]
lib: fix NULL->field_len access
Currenlty, this function is used only by:
- unit test of csv.c (see its main() section)
- ptm_lib.c
In case of ptm, it is safe to return NULL because:
csv_encode_record() -> return NULL
_ptm_lib_encode_header() -> return NULL
the only consumer of the return value is: ptm_lib_init_msg()
that checks the NULL return.
Warning:
Access to field 'field_len' results in a dereference of a null
pointer (loaded from variable 'fld')
Signed-off-by: Vincent Jardin <vincent.jardin@6wind.com>
Vincent JARDIN [Mon, 9 Oct 2017 07:43:26 +0000 (09:43 +0200)]
bgpd: fix aspath parsing
clang provides a notice about it that this p++ is useless,
because ++ would be done after the return.
From code review, I understand that p shall be incremented
for each token that is parsed from the buf. So let's keep
this intent.
Note that this commit is changing the behaviour of the source
code since from now p++ will be returned instead of p.
However, it does not hurt since the only consumer
just free() the aspath if it is parsed as as_token_unknown.
Let's be safe with a proper execution flow from now.
PS:
C reminders:
int f7(void) {
int j = 7;
return ++j; // return 8
}
int f8(void) {
int j = 7;
return j++; // return 7
}
Signed-off-by: Vincent Jardin <vincent.jardin@6wind.com>
Vincent JARDIN [Sun, 8 Oct 2017 21:28:58 +0000 (23:28 +0200)]
ldpd: fix clang warning
size is not used for further parsing. Keep it updated but tell
to the compiler that we know it is not used just in case one
needs to extend the parsing somedays.
Signed-off-by: Vincent Jardin <vincent.jardin@6wind.com>
vivek [Thu, 5 Oct 2017 05:30:53 +0000 (22:30 -0700)]
zebra: Fix MAC counts for EVPN
MAC entries are internally created for purposes such as when a local
neighbor is learnt but the MAC itself is not yet learnt. Such MACs are
not "real", so ensure they are not counted for UI output.
Fix following flaws that resulted in EVPN with L3 multi-tenancy (i.e.,
EVPN dealing with VxLAN routing in the presence of tenant VRFs) not
working properly:
1. EVPN enable ("advertise-all-vni") is a global command, ensure it is
accordingly processed. The config is maintained against the default VRF.
2. There was an incorrect attempt to derive the L3 VRF for L2 interfaces
- the VRF only applies for L3 interfaces, though the code may initialize
to the default value in other cases.
3. Functions to map (port, VLAN) to SVI or vice versa were incorrect -
particularly, zvni_map_svi() since it was looking in the L3 VRF for
"matching" L2 interface which it would never find. Fix.
In addition, since the 'zebra_vrf *' parameter is not relevant in most
places, it has been removed.
Renato Westphal [Fri, 6 Oct 2017 01:31:54 +0000 (22:31 -0300)]
*: use argv[]->text instead of argv[]->arg when appropriate
Incomplete commands like "debug ospf6 route mem" were being ignored. The
changes in ripd and ripngd are intended to make the code easier to read,
no bugs were fixed in these two daemons.
Don Slice [Wed, 4 Oct 2017 13:41:49 +0000 (06:41 -0700)]
zebra: resolve issue with importing table entries correctly
Problem reported when a table entry originated by rdnbrd was moved from one
interface to another on the same switch. Both would be deleted, leaving
no imported entry in the table. Modified zebra_add_import_table_entry to
used rib_add_multipath as well as correct the call to delete a duplicate
entry to include the nexthop associated with the route_entry.
Ticket: CM-18154 Signed-off-by: Don Slice <dslice@cumulusnetworks.com>
Reviewed By: CCR-6731
Testing Done: Manual testing successful, deb given to submitter, bgp-smoke
had no new failures
Donald Sharp [Thu, 28 Sep 2017 01:19:20 +0000 (21:19 -0400)]
*: Convert list_free usage to list_delete
list_free is occassionally being used to delete the
list and accidently not deleting all the nodes.
We keep running across this usage pattern. Let's
remove the temptation and only allow list_delete
to handle list deletion.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Thu, 5 Oct 2017 14:51:01 +0000 (10:51 -0400)]
*: Convert list_delete(struct list *) to ** to allow nulling
Convert the list_delete(struct list *) function to use
struct list **. This is to allow the list pointer to be nulled.
I keep running into uses of this list_delete function where we
forget to set the returned pointer to NULL and attempt to use
it and then experience a crash, usually after the developer
has long since left the building.
Let's make the api explicit in it setting the list pointer
to null.
Cynical Prediction: This code will expose a attempt
to use the NULL'ed list pointer in some obscure bit
of code.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Donald Sharp [Mon, 25 Sep 2017 23:10:40 +0000 (19:10 -0400)]
tools: Fix route flush
This commit does two things:
1) Flush by proto number not string
This is useful because not all systems might have the proto
values installed, or a version of iproute2 that they might
be installed with.
2) Flush missing routes that might have been installed( eigrp
and nhrp )
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Sebastian Lohff [Wed, 4 Oct 2017 19:55:51 +0000 (21:55 +0200)]
debian: Add iproute2 as dependency alternative in Debian package
iproute has been a transitional package that only depends on iproute2
since Debian Jessie or Ubuntu 14.04. To avoid installing this transitional
dummy package on newer installations we add iproute2 as a dependency
alternative to iproute. The iproute dependency can be dropped when
wheezy / 12.04 support is no longer needed.
Signed-off-by: Sebastian Lohff <seba@someserver.de>
Renato Westphal [Tue, 3 Oct 2017 19:11:07 +0000 (16:11 -0300)]
ldpd: detach stdin/stdout/stderr from the child processes
Doing a "ssh user@node 'ldpd -d'" was making the SSH session hang. In
the original OpenBSD's ldpd(8) daemon, the daemon function takes care
of connecting stdin/stdout/stderr to /dev/null. In the FRR port, this
only happens in the frr_run() function, after all children have been
forked. Ideally we could try to rearrange libfrr.c and ldpd.c in a way
that start_child() is called only after the parent connects the standard
I/O streams to /dev/null. But since this issue needs an immediate
fix, let's do this workaround for now. Note: even when running on the
foreground, all log messages from the child processes are sent to the
parent process, which then prints the messages to stdout/stderr and/or
to a log file.
Reported-by: Martin Winter <mwinter@opensourcerouting.org> Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Renato Westphal [Tue, 3 Oct 2017 13:24:45 +0000 (10:24 -0300)]
ldpd: fix wrong indentation in the help menu
The -n option was not aligned with the other ones:
% ldpd --help
[snip]
-A, --vty_addr Set vty's bind address
-P, --vty_port Set vty's port number
--ctl_socket Override ctl socket path
-n, --instance Instance id
Chirag Shah [Mon, 2 Oct 2017 21:07:13 +0000 (14:07 -0700)]
ospfd: Fix maxage_lsa lookup key
maxage_lsa using prefix structure to store in route_table.
The lsa pointer is 4-8 bytes depending on arch, but prefix member field
would be 1 byte. Use ptr field of prefix structure to store lsa pointer.
Also memset to initialize to avoid crash on ARM platform as same LSA is
referenced from multiple times during shutdown of ospf.
Signed-off-by: Chirag Shah <chirag@cumulusnetworks.com>