Prior to this fix, restarting the client just failed b/c the code tried to
"refresh" the existing LSA being added, except that code checked for meta-data
to exist, which was deleted when the client disconnected previously (or had
never connected and the LSA state was picked up from the network).
Signed-off-by: Christian Hopps <chopps@labn.net>
Reachable router information is used by OSPF opaque clients in order
to determine if the router advertising the opaque LSA data is
reachable (i.e., 2-way conectivity check).
Signed-off-by: Christian Hopps <chopps@labn.net>
Add a 'json' parameter to the 'show_opaque_info' callback definition,
and update all instances of that callback to not display plain-text
data when the user requested JSON data.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
When browsing or parsing OSPF LSA TLVs, we need to use the LSA length which is
part of the LSA header. This length, encoded in 16 bits, must be first
converted to host byte order with ntohs() function. However, Coverity Scan
considers that ntohs() function return TAINTED data. Thus, when the length is
used to control for() loop, Coverity Scan marks this part of the code as defect
with "Untrusted Loop Bound" due to the usage of Tainted variable. Similar
problems occur when browsing sub-TLV where length is extracted with ntohs().
To overcome this limitation, a size attribute has been added to the ospf_lsa
structure. The size is set when lsa->data buffer is allocated. In addition,
when an OSPF packet is received, the size of the payload is controlled before
contains is processed. For OSPF LSA, this allow a secure buffer allocation.
Thus, new size attribute contains the exact buffer allocation allowing a
strict control during TLV browsing.
This patch adds extra control to bound for() loop during TLV browsing to
avoid potential problem as suggested by Coverity Scan. Controls are based
on new size attribute of the ospf_lsa structure to avoid any ambiguity.
Signed-off-by: Olivier Dugeon <olivier.dugeon@orange.com>
Change thread_cancel to take a ** to an event, NULL-check
before dereferencing, and NULL the caller's pointer. Update
many callers to use the new signature.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
Remove mid-string line breaks, cf. workflow doc:
.. [#tool_style_conflicts] For example, lines over 80 characters are allowed
for text strings to make it possible to search the code for them: please
see `Linux kernel style (breaking long lines and strings)
<https://www.kernel.org/doc/html/v4.10/process/coding-style.html#breaking-long-lines-and-strings>`_
and `Issue #1794 <https://github.com/FRRouting/frr/issues/1794>`_.
Scripted commit, idempotent to running:
```
python3 tools/stringmangle.py --unwrap `git ls-files | egrep '\.[ch]$'`
```
Signed-off-by: David Lamparter <equinox@diac24.net>
We test nbr->oi in a couple of places for null, but
in the majority of places of the nbr->oi data is being
used we just access it. Touch up code to trust this
assertion and make the code more consistent in others.
Found in Coverity.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
In all but one instance we were following this pattern
with ospf_lsa_new:
ospf_lsa_new()
ospf_lsa_data_new()
so let's create a ospf_lsa_new_and_data to abstract
this bit of fun and cleanup all the places where
it assumes these function calls can fail.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
There is no need to check for failure of a ALLOC call
as that any failure to do so will result in a assert
happening. So we can safely remove all of this code.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
The following types are nonstandard:
- u_char
- u_short
- u_int
- u_long
- u_int8_t
- u_int16_t
- u_int32_t
Replace them with the C99 standard types:
- uint8_t
- unsigned short
- unsigned int
- unsigned long
- uint8_t
- uint16_t
- uint32_t
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
Inform the .clang-format file about LSDB_LOOP and
put the proper indentation for this loop into the
code.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
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>
This reverts commit c14777c6bf.
clang 5 is not widely available enough for people to indent with. This
is particularly problematic when rebasing/adjusting branches.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
The FSF's address changed, and we had a mixture of comment styles for
the GPL file header. (The style with * at the beginning won out with
580 to 141 in existing files.)
Note: I've intentionally left intact other "variations" of the copyright
header, e.g. whether it says "Zebra", "Quagga", "FRR", or nothing.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
Pass pointer to pointer instead of assigning by return value. See
previous commit message.
To ensure that the behavior stays functionally correct, any assignments
with the result of a thread_add* function have been transformed to set
the pointer to null before passing it. These can be removed wherever the
pointer is known to already be null.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
The way thread.c is written, a caller who wishes to be able to cancel a
thread or avoid scheduling it twice must keep a reference to the thread.
Typically this is done with a long lived pointer whose value is checked
for null in order to know if the thread is currently scheduled. The
check-and-schedule idiom is so common that several wrapper macros in
thread.h existed solely to provide it.
This patch removes those macros and adds a new parameter to all
thread_add_* functions which is a pointer to the struct thread * to
store the result of a scheduling call. If the value passed is non-null,
the thread will only be scheduled if the value is null. This helps with
consistency.
A Coccinelle spatch has been used to transform code of the form:
if (t == NULL)
t = thread_add_* (...)
to the form
thread_add_* (..., &t)
The THREAD_ON macros have also been transformed to the underlying
thread.c calls.
Signed-off-by: Quentin Young <qlyoung@cumulusnetworks.com>
