rtrlib/rtr: Ensure shadow tables are only freed when initialized #204

mroethke · 2018-12-07T13:46:16Z

This patch ensures the cleanup code can check whether the shadow tables
have been initialized and only frees them if they are.

Fix #202

codecov-io · 2018-12-07T13:48:09Z

Codecov Report

Merging #204 into master will decrease coverage by 0.26%.
The diff coverage is 7.69%.

@@            Coverage Diff            @@
##           master    #204      +/-   ##
=========================================
- Coverage   69.07%   68.8%   -0.27%     
=========================================
  Files          15      15              
  Lines        2231    2247      +16     
=========================================
+ Hits         1541    1546       +5     
- Misses        690     701      +11

NetDEF-CI · 2018-12-07T13:59:49Z

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/RPKI-RTRLIBPR-57/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Warnings Generated during build:

Ubuntu 14.04 amd64: Successful with additional warnings

Debian Package lintian failed for Ubuntu 14.04 amd64:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-57/artifact/UBUNTU1404AMD64/ErrorLog/log_lintian.txt)

E: librtr0 changes: bad-distribution-in-changes-file stable

Ubuntu 16.04 arm8: Successful with additional warnings

Debian Package lintian failed for Ubuntu 16.04 arm8:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-57/artifact/UBUNTU1604ARM8/ErrorLog/log_lintian.txt)

E: librtr0 changes: bad-distribution-in-changes-file stable

Ubuntu 16.04 arm7: Successful with additional warnings

Debian Package lintian failed for Ubuntu 16.04 arm7:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-57/artifact/UBUNTU1604ARM7/ErrorLog/log_lintian.txt)

E: librtr0 changes: bad-distribution-in-changes-file stable

Ubuntu 18.04 amd64: Successful with additional warnings

Debian Package lintian failed for Ubuntu 18.04 amd64:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-57/artifact/U1804AMD64/ErrorLog/log_lintian.txt)

E: librtr0 changes: bad-distribution-in-changes-file stable
W: rtr-tools-dbg: priority-extra-is-replaced-by-priority-optional
W: librtr-dbg: priority-extra-is-replaced-by-priority-optional

Ubuntu 16.04 amd64: Successful with additional warnings

Debian Package lintian failed for Ubuntu 16.04 amd64:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-57/artifact/UBUNTU1604AMD64/ErrorLog/log_lintian.txt)

E: librtr0 changes: bad-distribution-in-changes-file stable

Ubuntu 16.04 i386: Successful with additional warnings

Debian Package lintian failed for Ubuntu 16.04 i386:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-57/artifact/UBUNTU1604I386/ErrorLog/log_lintian.txt)

E: librtr0 changes: bad-distribution-in-changes-file stable

nbars · 2018-12-07T14:46:13Z

pfx_shadow_table and spki_shadow_table are still handeled as they where allocated on the stack

/home/nils/git/rtrlib/rtrlib/rtr/packets.c:1197:55: warning: incompatible pointer types passing 'struct pfx_table **' to parameter of type 'struct pfx_table *'; remove & [-Wincompatible-pointer-types]
                pfx_table_swap(rtr_socket->pfx_table, &pfx_shadow_table);
                                                      ^~~~~~~~~~~~~~~~~
/home/nils/git/rtrlib/./rtrlib/pfx/pfx_private.h:36:60: note: passing argument to parameter 'b' here
void pfx_table_swap(struct pfx_table *a, struct pfx_table *b);
                                                           ^
/home/nils/git/rtrlib/rtrlib/rtr/packets.c:1198:57: warning: incompatible pointer types passing 'struct spki_table **' to parameter of type 'struct spki_table *'; remove & [-Wincompatible-pointer-types]
                spki_table_swap(rtr_socket->spki_table, &spki_shadow_table);
                                                        ^~~~~~~~~~~~~~~~~~
/home/nils/git/rtrlib/./rtrlib/spki/spkitable_private.h:144:63: note: passing argument to parameter 'b' here
void spki_table_swap(struct spki_table *a, struct spki_table *b);
                                                              ^
/home/nils/git/rtrlib/rtrlib/rtr/packets.c:1202:66: warning: incompatible pointer types passing 'struct pfx_table **' to parameter of type 'struct pfx_table *'; remove & [-Wincompatible-pointer-types]
                    pfx_table_notify_diff(rtr_socket->pfx_table, &pfx_shadow_table, rtr_socket);
                                                                 ^~~~~~~~~~~~~~~~~
/home/nils/git/rtrlib/./rtrlib/pfx/pfx_private.h:54:75: note: passing argument to parameter 'old_table' here
void pfx_table_notify_diff(struct pfx_table *new_table, struct pfx_table *old_table, const struct rtr_socket *socket);
                                                                          ^
/home/nils/git/rtrlib/rtrlib/rtr/packets.c:1209:68: warning: incompatible pointer types passing 'struct spki_table **' to parameter of type 'struct spki_table *'; remove & [-Wincompatible-pointer-types]
                    spki_table_notify_diff(rtr_socket->spki_table, &spki_shadow_table, rtr_socket);
                                                                   ^~~~~~~~~~~~~~~~~~
/home/nils/git/rtrlib/./rtrlib/spki/spkitable_private.h:137:78: note: passing argument to parameter 'old_table' here
void spki_table_notify_diff(struct spki_table *new_table, struct spki_table *old_table, const struct rtr_socket *socket);

From the Bug report: > If rtr_receive_pdu returns an error code, the execution flow is > redirected to the cleanup label. If ->is_resetting is now true (e.g., > because it was set by a CACHE_RESPONSE PDU processed by > rtr_handle_cache_response_pdu), pfx_table_free_without_notify and > spki_table_free_without_notify are executing multiple operations on this > uninitialized data. This patch ensures the cleanup code can check whether the shadow tables have been initialized and only frees them if they are.

mroethke · 2018-12-07T15:37:08Z

That is embarrassing …

Fixed.

NetDEF-CI · 2018-12-07T15:46:34Z

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/RPKI-RTRLIBPR-60/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Warnings Generated during build:

Ubuntu 18.04 amd64: Successful with additional warnings

Debian Package lintian failed for Ubuntu 18.04 amd64:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-60/artifact/U1804AMD64/ErrorLog/log_lintian.txt)

E: librtr0 changes: bad-distribution-in-changes-file stable
W: librtr-dbg: priority-extra-is-replaced-by-priority-optional
W: rtr-tools-dbg: priority-extra-is-replaced-by-priority-optional

Ubuntu 16.04 arm7: Successful with additional warnings

Debian Package lintian failed for Ubuntu 16.04 arm7:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-60/artifact/UBUNTU1604ARM7/ErrorLog/log_lintian.txt)

E: librtr0 changes: bad-distribution-in-changes-file stable

Ubuntu 14.04 amd64: Successful with additional warnings

Debian Package lintian failed for Ubuntu 14.04 amd64:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-60/artifact/UBUNTU1404AMD64/ErrorLog/log_lintian.txt)

E: librtr0 changes: bad-distribution-in-changes-file stable

Ubuntu 16.04 arm8: Successful with additional warnings

Debian Package lintian failed for Ubuntu 16.04 arm8:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-60/artifact/UBUNTU1604ARM8/ErrorLog/log_lintian.txt)

E: librtr0 changes: bad-distribution-in-changes-file stable

Ubuntu 16.04 amd64: Successful with additional warnings

Debian Package lintian failed for Ubuntu 16.04 amd64:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-60/artifact/UBUNTU1604AMD64/ErrorLog/log_lintian.txt)

E: librtr0 changes: bad-distribution-in-changes-file stable

Ubuntu 16.04 i386: Successful with additional warnings

Debian Package lintian failed for Ubuntu 16.04 i386:
(see full package build log at https://ci1.netdef.org/browse/RPKI-RTRLIBPR-60/artifact/UBUNTU1604I386/ErrorLog/log_lintian.txt)

E: librtr0 changes: bad-distribution-in-changes-file stable

nbars

Looks good 👍

mroethke requested a review from nbars December 7, 2018 13:46

mroethke force-pushed the fix_202 branch from bc59a1e to 67cd1e3 Compare December 7, 2018 15:36

nbars reviewed Dec 7, 2018

View reviewed changes

waehlisch merged commit db2d254 into rtrlib:master Dec 14, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

rtrlib/rtr: Ensure shadow tables are only freed when initialized #204

rtrlib/rtr: Ensure shadow tables are only freed when initialized #204

mroethke commented Dec 7, 2018

codecov-io commented Dec 7, 2018 •

edited

NetDEF-CI commented Dec 7, 2018

nbars commented Dec 7, 2018

mroethke commented Dec 7, 2018

NetDEF-CI commented Dec 7, 2018

nbars left a comment •

edited

rtrlib/rtr: Ensure shadow tables are only freed when initialized #204

rtrlib/rtr: Ensure shadow tables are only freed when initialized #204

Conversation

mroethke commented Dec 7, 2018

codecov-io commented Dec 7, 2018 • edited

Codecov Report

NetDEF-CI commented Dec 7, 2018

Continuous Integration Result: SUCCESSFUL

Warnings Generated during build:

nbars commented Dec 7, 2018

mroethke commented Dec 7, 2018

NetDEF-CI commented Dec 7, 2018

Continuous Integration Result: SUCCESSFUL

Warnings Generated during build:

nbars left a comment • edited

Choose a reason for hiding this comment

codecov-io commented Dec 7, 2018 •

edited

nbars left a comment •

edited