Add a new flag to match deprecated addresses. An address is deprecated
when its preferred lifetime has expired but its valid lifetime has
not.
Address deprecation is one of the criteria for source address
selection in IPv6. For IPv4 the deprecation doesn't have any
real effect.
Note that this commit changes the behavior of
nm_ip_config_get_first_address(WITH_ADDRSTATE_NORMAL), since now
deprecated addresses are not returned. However this should not impact
existing callers since they either:
- request a IPv6 (WITH_ADDRTYPE_LINKLOCAL | WITH_ADDRSTATE_NORMAL)
address; IPv6 link-local addresses are supposed to have infinite
lifetimes;
or
- request a IPv6 (WITH_ADDRTYPE_NORMAL | WITH_ADDRSTATE__ANY)
address.
The warning is wrong, because we already assert for the string length a few
lines earlier.
Error: STRING_OVERFLOW (CWE-120): [#def595]
NetworkManager-1.31.90/src/libnm-platform/nm-platform-utils.c:1896: fixed_size_dest: You might overrun the 16-character fixed-size string "ifname_buf_last_try" by copying "ifname" without checking the length.
# 1894| if (nm_streq(ifname, ifname_buf_last_try))
# 1895| return -1;
# 1896|-> strcpy(ifname_buf_last_try, ifname);
# 1897|
# 1898| fd_dir = open(sysdir, O_DIRECTORY | O_CLOEXEC);
(cherry picked from commit c87433ebd2)
Coverity thinks there is a problem here:
Error: TAINTED_SCALAR (CWE-20): [#def233]
NetworkManager-1.31.5/src/libnm-platform/nm-netlink.c:1437: tainted_argument: Calling function "recvmsg" taints argument "msg".
NetworkManager-1.31.5/src/libnm-platform/nm-netlink.c:1458: tainted_data: Passing tainted expression "msg.msg_controllen" to "g_realloc", which uses it as an allocation size.
NetworkManager-1.31.5/src/libnm-platform/nm-netlink.c:1458: remediation: Ensure that tainted values are properly sanitized, by checking that their values are within a permissible range.
# 1456|
# 1457| msg.msg_controllen *= 2;
# 1458|-> msg.msg_control = g_realloc(msg.msg_control, msg.msg_controllen);
# 1459| goto retry;
# 1460| }
but the problem is not the tainted data. The problem is how should
we handle MSG_CTRUNC? If we reach MSG_CTRUNC we already lost a message.
Retrying to receive the next message is not going to fix that and is
wrong.
Also, there really is no reason why any truncation should happen. The only
ancillary data that should be present is the sender information, and for
that our buffer is supposed to be large enough.
So, simply ignore truncation. It shouldn't happen, if it happened we
cannot recover from it (aside failing an assertion), and all we really
care are the retrieved credentials. If truncation happened, we might
not have retrieved the credentials, but then that is for the caller
to handle (by rejecting the message as untrusted).
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/872
For one, "src_n_map" must always be greater than zero at this point.
lgtm.com warns about that, and the point of this patch is to avoid
that warning.
Still, the check really isn't needed, also because nm_memdup() explicitly
handles buffers sizes of zero.
Introducing ethtool PAUSE support with:
* ethtool.pause-autoneg on/off
* ethtool.pause-rx on/off
* ethtool.pause-tx on/off
Limitations:
* When `ethtool.pause-autoneg` is set to true, the `ethtool.pause-rx`
and `ethtool.pause-tx` will be ignored. We don't have warning for
this yet.
Unit test case included.
Signed-off-by: Gris Ge <fge@redhat.com>
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/829
- consistently check for success/failure of _ethtool_call_handle()
with "< 0" / ">= 0".
- drop unnecessary memset(). In the past, I argued to add this because
there were obscure cases with valgrind where this made a difference.
As it's not clear when/how that is necessary, drop it again.
Also, we want to prefer explicit struct initialization over memset(),
so if memset() would be necessary, those places would be problematic
as well.
- inline unnecessary helper functions. They had only one caller and
only make the code more verbose.
- use _ethtool_call_once() instead of _ethtool_call_handle() at places
where we use the handle only once. The handle and _ethtool_call_handle()
are useful to cache and reuse the file descriptor and the interface
name. If we only make one call with the handle, we can use
_ethtool_call_once() instead.
https://gitlab.freedesktop.org/NetworkManager/NetworkManager/-/merge_requests/830
Previously, we used nm_udev_utils_property_as_boolean(), which was
taken from g_udev_device_get_property_as_boolean(). That function
accepts "1" and "true" (with ASCII case insensitive).
When we parse a flag, there is no need to reject "no", "yes" or
"on"/"off" as invalid (and thus return FALSE). We have a boolean
parse method _nm_utils_ascii_str_to_bool(), which parses everything
that nm_udev_utils_property_as_boolean() accepts, and more.
Be liberal in what we accept, so use our general parse function.
Having two functions like link_set_x() and link_set_nox() it is not a
good idea. This patch is introducing nm_platform_link_change_flags().
This allow flag modification directly, so the developer does not need to
define the virtual functions all the time everywhere.
Signed-off-by: Fernando Fernandez Mancera <ffmancera@riseup.net>
When adding an IPv4 address, kernel automatically adds a local route.
This is done by fib_add_ifaddr(). Note that if the address is
IFA_F_SECONDARY, then the "src" is the primary address. That means, with
nmcli connection add con-name t type ethernet ifname t autoconnect no \
ipv4.method manual ipv6.method disabled \
ipv4.addresses '192.168.77.10/24, 192.168.77.11/24'
we get two routes:
"local 192.168.77.10 dev t table local proto kernel scope host src 192.168.77.10"
"local 192.168.77.11 dev t table local proto kernel scope host src 192.168.77.10"
Our code would only generate instead:
"local 192.168.77.10 dev t table local proto kernel scope host src 192.168.77.10"
"local 192.168.77.11 dev t table local proto kernel scope host src 192.168.77.11"
Afterwards, this artificial route will be leaked:
#!/bin/bash
set -vx
nmcli connection delete t || :
ip link delete t || :
ip link add name t type veth peer t-veth
nmcli connection add con-name t type ethernet ifname t autoconnect no ipv4.method manual ipv4.addresses '192.168.77.10/24, 192.168.77.11/24' ipv6.method disabled
nmcli connection up t
ip route show table all dev t | grep --color '^\|192.168.77.11'
sleep 1
nmcli device modify t -ipv4.addresses 192.168.77.11/24
ip route show table all dev t | grep --color '^\|192.168.77.11'
ip route show table all dev t | grep -q 192.168.77.11 && echo "the local route 192.168.77.11 is still there, because NM adds a local route with wrong pref-src"
It will also be leaked because in the example above ipv4.route-table is
unset, so we are not in full route sync mode and the local table is not
synced.
This was introduced by commit 3e5fc04df3 ('core: add dependent local
routes configured by kernel'), but it's unclear to me why we really need
this. Drop it again and effectively revert commit 3e5fc04df3 ('core:
add dependent local routes configured by kernel').
I think this "solution" is still bad. We need to improve our route sync
approach with L3Cfg rework. For now, it's probably good enough.
https://bugzilla.redhat.com/show_bug.cgi?id=1907661
We have a cache for sysctl values, so that we can log changes and
previous values.
When resetting the log level, we prune that cache, which is done by
_nm_logging_clear_platform_logging_cache(). That function is called
by nm_logging_setup(), which is guaranteed to only happen on the main
thread.
NMPlatform in general is not thread safe (meaning, that the same NMPlatform
instance cannot be used by multiple threads at the same time). There is however
a reasonable aim that you could use different NMPlatform instances on their
own threads.
That currently doesn't work, mainly due to nm-logging which always must
be done from the main thread -- unless we would set NM_THREAD_SAFE_ON_MAIN_THREAD
in all of NMPlatform (which would be too expensive for something we
don't actually need). That means also the sysctl getter must only be
called on the main thread an all was good already.
Still, we could have NMPlatform usable from multiple thread by setting
NM_THREAD_SAFE_ON_MAIN_THREAD. As we are almost there to have the code
thread-safe, make accessing the sysctl value cache thread-safe (even if
we currently don't actually access it from multiple thread).
Avoid dependencies but explicitly link the static library where it is
used.
This also fixes that we linked libnm-log-core into
libnm-settings-plugin-ifcfg-rh.so, which duplicated the symbols
while it should used them from NetworkManager.
"nm-test-utils.h" is a header-only, helper library for our unit tests.
It was somewhat unmotivated in "shared/nm-utils", because all tests use
it, but it was not part of a "module".
Move it to "src/libnm-glib-aux/". It fits there very well. They both
have (only) a dependency on glib.
