Bug#888291: /usr/sbin/NetworkManager: segfault when adding tc filter rule

2018-01-24 Thread Andreas Henriksson
Package: network-manager
Version: 1.10.2-3
Severity: important
File: /usr/sbin/NetworkManager

Dear Maintainer,

NetworkManager consistently segfaults when adding a tc filter rule for
me. Backtrace and the example tc rules I'm trying to test out below.


-->88<->88<--

$ sudo coredumpctl dump
   PID: 526 (NetworkManager)
   UID: 0 (root)
   GID: 0 (root)
Signal: 11 (SEGV)
 Timestamp: Wed 2018-01-24 18:46:02 CET (4s ago)
  Command Line: /usr/sbin/NetworkManager --no-daemon
Executable: /usr/sbin/NetworkManager
 Control Group: /system.slice/NetworkManager.service
  Unit: NetworkManager.service
 Slice: system.slice
   Boot ID: 4f7a28442cbe452d9b2e04a5dea264b6
Machine ID: 0ca30f1374c64ae99923a08d640084f1
  Hostname: nyera
   Storage: 
/var/lib/systemd/coredump/core.NetworkManager.0.4f7a28442cbe452d9b2e04a5dea264b6.526.151681596200.lz4
   Message: Process 526 (NetworkManager) of user 0 dumped core.

Stack trace of thread 526:
#0  0x7f808eef0866 __GI___strlen_sse2 (libc.so.6)
#1  0x55f5d436d466 nm_hash_update_str (NetworkManager)
#2  0x55f5d437aa3e nmp_object_hash_update (NetworkManager)
#3  0x55f5d42f93b6 _dict_idx_objs_hash (NetworkManager)
#4  0x7f8090814644 g_hash_table_lookup (libglib-2.0.so.0)
#5  0x55f5d42f9d59 nm_dedup_multi_index_obj_intern 
(NetworkManager)
#6  0x55f5d42f9ee3 _add (NetworkManager)
#7  0x55f5d42fa297 nm_dedup_multi_index_add_full 
(NetworkManager)
#8  0x55f5d437acf1 _idxcache_update (NetworkManager)
#9  0x55f5d437d079 nmp_cache_update_netlink (NetworkManager)
#10 0x55f5d435bdbd event_valid_msg (NetworkManager)
#11 0x55f5d435d30d event_handler_read_netlink 
(NetworkManager)
#12 0x55f5d435e241 delayed_action_handle_READ_NETLINK 
(NetworkManager)
#13 0x55f5d435e471 event_handler (NetworkManager)
#14 0x7f8090825dd5 g_main_context_dispatch 
(libglib-2.0.so.0)
#15 0x7f80908261a0 n/a (libglib-2.0.so.0)
#16 0x7f80908264b2 g_main_loop_run (libglib-2.0.so.0)
#17 0x55f5d42be789 main (NetworkManager)
#18 0x7f808ee7bf2a __libc_start_main (libc.so.6)
#19 0x55f5d42bedea _start (NetworkManager)

Stack trace of thread 545:
#0  0x7f808ef45e6b __GI___poll (libc.so.6)
#1  0x7f8090826119 n/a (libglib-2.0.so.0)
#2  0x7f809082622c g_main_context_iteration 
(libglib-2.0.so.0)
#3  0x7f8090826271 n/a (libglib-2.0.so.0)
#4  0x7f809084d5f5 n/a (libglib-2.0.so.0)
#5  0x7f808f21851a start_thread (libpthread.so.0)
#6  0x7f808ef503ef __clone (libc.so.6)

Stack trace of thread 547:
#0  0x7f808ef45e6b __GI___poll (libc.so.6)
#1  0x7f8090826119 n/a (libglib-2.0.so.0)
#2  0x7f80908264b2 g_main_loop_run (libglib-2.0.so.0)
#3  0x7f8091013ad6 n/a (libgio-2.0.so.0)
#4  0x7f809084d5f5 n/a (libglib-2.0.so.0)
#5  0x7f808f21851a start_thread (libpthread.so.0)
#6  0x7f808ef503ef __clone (libc.so.6)
Refusing to dump core to tty (use shell redirection or specify --output).

-->88<->88<--

export FOOPORT=53000
export FOOIP=1.2.3.4
export INTERFACE=eth0 # change to your actual interface name.

tc qdisc add dev $INTERFACE root handle 1: htb default 10
tc class add dev $INTERFACE parent 1: classid 1:1 htb rate 1000mbit ceil 
1000mbit

# default leaf class
tc class add dev $INTERFACE parent 1:1 classid 1:10 htb rate 1000mbit ceil 
1000mbit

# leaf class to be used for SF traffic limited to 5mbit
tc class add dev $INTERFACE parent 1:1 classid 1:20 htb rate 5mbit ceil 5mbit

# Put traffic going to Aurix on SF port number in 1:20 leaf class
tc filter add dev $INTERFACE protocol ip parent 1:0 prio 0 u32 \
match ip dport ${FOOPORT} 0x \
match ip dst ${FOOIP}/32 \
flowid 1:20


-->88<->88<--

The segfault happens when running the last command (tc filter ...)
which means my wifi connection goes down and all my network traffic
is dead.


Regards,
Andreas Henriksson

-- System Information:
Debian Release: buster/sid
  APT prefers unstable-debug
  APT policy: (500, 'unstable-debug'), (500, 'testing-debug'), (500, 
'unstable'), (1, 'experimental-debug'), (1, 'experimental')
Architecture: amd64 (x86_64)

Kernel: Linux 4.14.0-3-amd64 (SMP w/4 CPU cores)
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), 
LANGUAGE

Bug#888291: /usr/sbin/NetworkManager: segfault when adding tc filter rule

2018-01-24 Thread Andreas Henriksson
Control: tags -1 + upstream fixed-upstream patch
Control: forwarded -1 
https://cgit.freedesktop.org/NetworkManager/NetworkManager/patch/?id=27e8fffdb833748dfeb6648b8768c4ef48822841


On Wed, Jan 24, 2018 at 06:54:34PM +0100, Andreas Henriksson wrote:
> Package: network-manager
> Version: 1.10.2-3
> Severity: important
> File: /usr/sbin/NetworkManager
> 
> Dear Maintainer,
> 
> NetworkManager consistently segfaults when adding a tc filter rule for
> me. Backtrace and the example tc rules I'm trying to test out below.
[...]

I can confirm that the above upstream commit fixes the problem
(and applies cleanly to the debian version). Would be great if
you could include it in your next upload.

Regards,
Andreas Henriksson

diff -Nru network-manager-1.10.2/debian/changelog 
network-manager-1.10.2/debian/changelog
--- network-manager-1.10.2/debian/changelog 2018-01-21 19:50:39.0 
+0100
+++ network-manager-1.10.2/debian/changelog 2018-01-24 22:44:52.0 
+0100
@@ -1,3 +1,10 @@
+network-manager (1.10.2-3+fixtfilter0) UNRELEASED; urgency=medium
+
+  * Add patch fixing tc filter crash from
+
https://cgit.freedesktop.org/NetworkManager/NetworkManager/patch/?id=27e8fffdb833748dfeb6648b8768c4ef48822841
+
+ -- Andreas Henriksson   Wed, 24 Jan 2018 22:44:52 +0100
+
 network-manager (1.10.2-3) unstable; urgency=medium
 
   * secret-agent: construct the dbus proxy for async agent with the correct
diff -Nru 
network-manager-1.10.2/debian/patches/platform-fix-crash-hashing-NMPlatformTfiter-and-NMPlatformQdisc.patch
 
network-manager-1.10.2/debian/patches/platform-fix-crash-hashing-NMPlatformTfiter-and-NMPlatformQdisc.patch
--- 
network-manager-1.10.2/debian/patches/platform-fix-crash-hashing-NMPlatformTfiter-and-NMPlatformQdisc.patch
 1970-01-01 01:00:00.0 +0100
+++ 
network-manager-1.10.2/debian/patches/platform-fix-crash-hashing-NMPlatformTfiter-and-NMPlatformQdisc.patch
 2018-01-24 22:44:52.0 +0100
@@ -0,0 +1,65 @@
+From 27e8fffdb833748dfeb6648b8768c4ef48822841 Mon Sep 17 00:00:00 2001
+From: Thomas Haller 
+Date: Tue, 12 Dec 2017 10:37:59 +0100
+Subject: platform: fix crash hashing NMPlatformTfilter and NMPlatformQdisc
+
+@kind might be NULL. There are 3 forms of the hash-update functions for
+string: str(), str0(), and strarr().
+
+- str0() is when the string might be NULL.
+- str() does not allow the string to be NULL
+- strarr() is like str(), except it adds a G_STATIC_ASSERT()
+  that the argument is a C array.
+
+The reason why a difference between str() and str0() exists, is
+because str0() hashes NULL different from a "" or any other string.
+This has an overhead, because it effectively must hash another bit
+of information that tells whether a string was passed or not.
+
+The reason is, that hashing a tupple of two strings should always
+yield a different hash value, even for "aa",""; "a","a"; "","aa",
+where naive concatentation would yield identical hash values in all
+three cases.
+
+Fixes: e75fc8279becce29e688551930d85e59e1280c89
+---
+ src/platform/nm-platform.c | 8 
+ 1 file changed, 4 insertions(+), 4 deletions(-)
+
+diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c
+index c794970..369effb 100644
+--- a/src/platform/nm-platform.c
 b/src/platform/nm-platform.c
+@@ -5320,7 +5320,7 @@ nm_platform_qdisc_to_string (const NMPlatformQdisc 
*qdisc, char *buf, gsize len)
+ void
+ nm_platform_qdisc_hash_update (const NMPlatformQdisc *obj, NMHashState *h)
+ {
+-  nm_hash_update_str (h, obj->kind);
++  nm_hash_update_str0 (h, obj->kind);
+   nm_hash_update_vals (h,
+obj->ifindex,
+obj->addr_family,
+@@ -5387,17 +5387,17 @@ nm_platform_tfilter_to_string (const NMPlatformTfilter 
*tfilter, char *buf, gsiz
+ void
+ nm_platform_tfilter_hash_update (const NMPlatformTfilter *obj, NMHashState *h)
+ {
+-  nm_hash_update_str (h, obj->kind);
++  nm_hash_update_str0 (h, obj->kind);
+   nm_hash_update_vals (h,
+obj->ifindex,
+obj->addr_family,
+obj->handle,
+obj->parent,
+obj->info);
+-  nm_hash_update_str (h, obj->action.kind);
+   if (obj->action.kind) {
++  nm_hash_update_str (h, obj->action.kind);
+   if (nm_streq (obj->action.kind, NM_PLATFORM_ACTION_KIND_SIMPLE))
+-  nm_hash_update_str (h, obj->action.simple.sdata);
++  nm_hash_update_strarr (h, obj->action.simple.sdata);
+   }
+ }
+ 
+-- 
+cgit v1.1
+
diff -Nru network-manager-1.10.2/debian/patches/series 
network-manager-1.10.2/debian/patches/series
--- network-manager-1.10.2/debian/patches/series2018-01-21 
19:50:39.0 +0100
+++ network-manager-1.10.2/debian/patches/series2018-01-24 
22:44:52.0 +0100
@@ -6,3 +6,4 @@
 libnm-register-empty-NMClient-and-NetworkManager-when-loa.patch