The call to nm_utils_parse_variant_attributes() is useless. The following
_tc_read_common_opts() call does the same thing. This was probably left
in place by accident.
Before:
# nmcli c modify eth666 tc.qdiscs root
Error: failed to modify tc.qdiscs: '(null)' is not a valid kind The valid syntax is: '[root | parent <handle>] [handle <handle>] <qdisc>'.
After:
# nmcli c modify eth666 tc.qdiscs root
Error: failed to modify tc.qdiscs: kind is missing. The valid syntax is: '[root | parent <handle>] [handle <handle>] <kind>'.
==16725==ERROR: AddressSanitizer: global-buffer-overflow on address 0x0000005a159f at pc 0x00000046fc1b bp 0x7fff6038f900 sp 0x7fff6038f8f0
READ of size 1 at 0x0000005a159f thread T0
#0 0x46fc1a in _do_test_unescape_spaces libnm-core/tests/test-general.c:7791
#1 0x46fe5b in test_nm_utils_unescape_spaces libnm-core/tests/test-general.c:7810
#2 0x7f4ac5fe7fc9 in test_case_run gtestutils.c:2318
#3 0x7f4ac5fe7fc9 in g_test_run_suite_internal gtestutils.c:2403
#4 0x7f4ac5fe7e83 in g_test_run_suite_internal gtestutils.c:2415
#5 0x7f4ac5fe7e83 in g_test_run_suite_internal gtestutils.c:2415
#6 0x7f4ac5fe8281 in g_test_run_suite gtestutils.c:2490
#7 0x7f4ac5fe82a4 in g_test_run (/lib64/libglib-2.0.so.0+0x772a4)
#8 0x48240d in main libnm-core/tests/test-general.c:7994
#9 0x7f4ac5dc9412 in __libc_start_main (/lib64/libc.so.6+0x24412)
#10 0x423ffd in _start (/home/bgalvani/work/NetworkManager/libnm-core/tests/test-general+0x423ffd)
0x0000005a159f is located 49 bytes to the right of global variable '*.LC370' defined in 'libnm-core/tests/test-general.c' (0x5a1560) of size 14
'*.LC370' is ascii string 'nick-5, green'
0x0000005a159f is located 1 bytes to the left of global variable '*.LC371' defined in 'libnm-core/tests/test-general.c' (0x5a15a0) of size 1
'*.LC371' is ascii string ''
SUMMARY: AddressSanitizer: global-buffer-overflow libnm-core/tests/test-general.c:7791 in _do_test_unescape_spaces
We already have code that parses exactly this kinds of string:
nm_utils_parse_inaddr_prefix_bin(). Use it.
Also, it doesn't use g_strsplit_set() to separate a string at the first
'/'. Total overkill.
Previously, nm_utils_strsplit_set_full() would always remove empty
tokens. Add a flag NM_UTILS_STRSPLIT_SET_FLAGS_PRESERVE_EMPTY to avoid
that.
This makes nm_utils_strsplit_set_full() return the same result as
g_strsplit_set() and a direct replacement for it -- except for "",
where we return %NULL.
When we delete the runner.name property, the runner object itself gets
deleted if that was the only property, and @runner becomes invalid.
==13818== Invalid read of size 1
==13818== at 0x55EAF4: nm_streq (nm-macros-internal.h:869)
==13818== by 0x55EAF4: _json_team_normalize_defaults (nm-utils.c:5573)
==13818== by 0x566C89: _nm_utils_team_config_set (nm-utils.c:6057)
==13818== by 0x5498A6: _nm_utils_json_append_gvalue (nm-utils-private.h:228)
==13818== by 0x5498A6: set_property (nm-setting-team.c:1622)
==13818== Address 0x182a9330 is 0 bytes inside a block of size 13 free'd
==13818== at 0x4839A0C: free (vg_replace_malloc.c:530)
==13818== by 0x4857868: json_delete_string (value.c:763)
==13818== by 0x4857868: json_delete (value.c:975)
==13818== by 0x4851FA1: UnknownInlinedFun (jansson.h:129)
==13818== by 0x4851FA1: hashtable_do_del (hashtable.c:131)
==13818== by 0x4851FA1: hashtable_del (hashtable.c:289)
==13818== by 0x55DFDD: _json_del_object (nm-utils.c:5384)
==13818== by 0x55EA70: _json_delete_object_on_string_match (nm-utils.c:5532)
==13818== by 0x55EADB: _json_team_normalize_defaults (nm-utils.c:5549)
==13818== by 0x566C89: _nm_utils_team_config_set (nm-utils.c:6057)
==13818== by 0x5498A6: _nm_utils_json_append_gvalue (nm-utils-private.h:228)
==13818== by 0x5498A6: set_property (nm-setting-team.c:1622)
==13818== Block was alloc'd at
==13818== at 0x483880B: malloc (vg_replace_malloc.c:299)
==13818== by 0x4852E8C: lex_scan_string (load.c:389)
==13818== by 0x4852E8C: lex_scan (load.c:620)
==13818== by 0x4853458: parse_object (load.c:738)
==13818== by 0x4853458: parse_value (load.c:862)
==13818== by 0x4853466: parse_object (load.c:739)
==13818== by 0x4853466: parse_value (load.c:862)
==13818== by 0x4853655: parse_json.constprop.7 (load.c:899)
==13818== by 0x48537CF: json_loads (load.c:959)
==13818== by 0x566780: _nm_utils_team_config_set (nm-utils.c:5961)
==13818== by 0x5498A6: _nm_utils_json_append_gvalue (nm-utils-private.h:228)
==13818== by 0x5498A6: set_property (nm-setting-team.c:1622)
Fixes: a5642fd93a ('libnm-core: team: rework defaults management on runner properties')
Traditionally, the MTU in "datagram" transport mode was restricted to
2044. That is no longer the case, relax that.
In fact, choose a very large maximum and don't differenciate between
"connected" mode (they now both use now 65520). This is only the
limitation of the connection profile. Whether setting such large MTUs
actually works must be determined when activating the profile.
Initscripts "ifup-ib" from rdma-core package originally had a limit of 2044.
This was raised to 4092 in rh#1186498. It is suggested to raise it further
in bug rh#1647541.
In general, kernel often does not allow setting large MTUs. And even if it
allows it, it may not work because it also requires the entire network to
be configured accordingly. But that means, it is generally not helpful to
limit the MTU in the connection profile too strictly. Just allow large
MTUs, we need to see at activation time whether the configuration works.
Note also that all other setting types don't validate the range for MTU at
all.
Related: https://bugzilla.redhat.com/show_bug.cgi?id=1186498
Related: https://bugzilla.redhat.com/show_bug.cgi?id=1593334
(rdma-core: raise limit from 2044 to 4092 in ifup-ib)
Related: https://bugzilla.redhat.com/show_bug.cgi?id=1647541
(rdma-core: raise limit beyond 4092 in ifup-ib)
Related: https://bugzilla.redhat.com/show_bug.cgi?id=1532638#c4
(rdma-core: MTU related discussion)
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1534869
(NetworkManager bug about this topic, but with lots of unrelated
discussion. See in particular #c16)
Fixes: https://bugzilla.redhat.com/show_bug.cgi?id=1653494
libnm-core/nm-keyfile.c:679:32: error: 'key_type' may be used uninitialized in this function [-Werror=maybe-uninitialized]
build_list[build_list_len++] = (BuildListData) {
^
Add NMIPRoutingRule API with a few basic rule properties. More
properties will be added later as we want to support them.
Also, add to/from functions for string/GVariant representations.
These will be needed to persist/load/exchange rules.
The to-string format follows the `ip rule add` syntax, with the aim
to be partially compatible. Full compatibility is not possible though,
for various reasons (see code comment).
Add support for IEEE 802.3 organizationally specific TLVs:
- MAC/PHY configuration/status (IEEE 802.1AB-2009 clause F.2)
- power via medium dependent interface (clause F.3)
- maximum frame size (clause F.4)
Previously we exported the contents of VLAN Name TLV in the 'vid'
(uint32) and 'vlan-name' (string) attributes. This is not entirely
correct as the TLV can appear multiple times.
We need a way to export all the VLAN IDs and names for the
neighbor. Add a new 'vlans' attribute which obsoletes the other two
and is an array of dictionaries, where each dictionary contains the
'vid' and 'name' keys.
Support the management address TLV (IEEE 802.1AB-2009 clause
8.5.9). The TLV can appear multiple times and so it is exported on
D-Bus as an array of dictionaries.
The VFs already can be parsed as plain number (to indicate the
ifindex). We should not also support accepting the plain number
as index to be removed.
Fixes: a2f12994b7 ('cli: add support for configuring SR-IOV')
It's usually not necessary, because _nm_utils_unescape_spaces()
gets called after nm_utils_strsplit_set(), which already removes
the non-escaped spaces.
Still, for completeness, this should be here. Also, because with
this the function is useful for individual options (not delimiter
separate list values), to support automatically dropping leading or
trailing whitespace, but also support escaping them.
The setting's verify() function already checks that the s390 options
are not empty and no longer than 200. Asserting for that is a major
annoyance, because callers need to reimplement that check.
The "from" part is like a key for the egress/ingress priority map.
Extend nm_setting_vlan_remove_priority_str_by_value() to accept only the
"from" part when finding and deleting value. This allows for:
$ nmcli connection modify "$PROFILE" -vlan.ingress-priority-map '4:'
$ nmcli connection modify "$PROFILE" -vlan.ingress-priority-map '4:*'
to fuzzy match the value to remove.
The same code is used by nmcli. Obviously, clients also need to
parse string representations.
That begs the question whether this should be public API of libnm.
Maybe, but don't decide that now, just reuse the code internally via
"shared/nm-libnm-core-utils.h".
We have code in "shared/nm-utils" which are general purpose
helpers, independent of "libnm", "libnm-core", "clients" and "src".
We have shared code like "shared/nm-ethtool-utils.h" and
"shared/nm-meta-setting.h", which is statically linked, shared
code that contains libnm related helpers. But these helpers already
have a specific use (e.g. they are related to ethtool or NMSetting
metadata).
Add a general purpose helper that:
- depends (and extends) libnm-core
- contains unrelated helpers
- can be shared (meaning it will be statically linked).
- this code can be used by any library user of "libnm.so"
(nmcli, nm-applet) and by "libnm-core" itself. Thus, "src/"
and "libnm/" may also use this code indirectly, via "libnm-core/".
- avoid the memory allocations by not using g_strsplit().
- add a helper function priority_map_parse_str(). This will
be used later, to avoid allocating a NMVlanQosMapping
result, when we don't need it on the heap.
- for the priority mappings, the "from" part is the key and must
be unique. As such, it would make sense to say
$ nmcli connection modify "$PROFILE" -vlan.ingress-priority-map '1:*'
or
$ nmcli connection modify "$PROFILE" -vlan.ingress-priority-map '1:'
to delete any mapping for that priority, regardless of the "to" part.
Add an option to leave the "to" part unspecified. This will be used
later.