Logging pointer values is useful to identify the object in the logging message.
But plain pointer values also can be used to defeat ASLR and should not be logged.
Instead, print NM_HASH_OBFUSCATE_PTR() value, which is a 64 bit number based on
the pointer value and some random seed. A minor problem is that there is still the
chance of duplicates, albeit small.
We no longer add these. If you use Emacs, configure it yourself.
Also, due to our "smart-tab" usage the editor anyway does a subpar
job handling our tabs. However, on the upside every user can choose
whatever tab-width he/she prefers. If "smart-tabs" are used properly
(like we do), every tab-width will work.
No manual changes, just ran commands:
F=($(git grep -l -e '-\*-'))
sed '1 { /\/\* *-\*- *[mM]ode.*\*\/$/d }' -i "${F[@]}"
sed '1,4 { /^\(#\|--\|dnl\) *-\*- [mM]ode/d }' -i "${F[@]}"
Check remaining lines with:
git grep -e '-\*-'
The ultimate purpose of this is to cleanup our files and eventually use
SPDX license identifiers. For that, first get rid of the boilerplate lines.
From the files under "shared/nm-utils" we build an internal library
that provides glib-based helper utilities.
Move the files of that basic library to a new subdirectory
"shared/nm-glib-aux" and rename the helper library "libnm-core-base.la"
to "libnm-glib-aux.la".
Reasons:
- the name "utils" is overused in our code-base. Everything's an
"utils". Give this thing a more distinct name.
- there were additional files under "shared/nm-utils", which are not
part of this internal library "libnm-utils-base.la". All the files
that are part of this library should be together in the same
directory, but files that are not, should not be there.
- the new name should better convey what this library is and what is isn't:
it's a set of utilities and helper functions that extend glib with
funcitonality that we commonly need.
There are still some files left under "shared/nm-utils". They have less
a unifying propose to be in their own directory, so I leave them there
for now. But at least they are separate from "shared/nm-glib-aux",
which has a very clear purpose.
(cherry picked from commit 80db06f768)
NetworkManager is single-threaded and uses a mainloop.
However, sometimes we may need multiple threads. For example, we will
need to write sysctl values asynchronously, using the glib thread-pool.
For that to work, we also need to switch the network-namespace of the
thread-pool thread. We want to use NMPNetns for that. Hence it's better
to have NMPNetns thread-safe, instead of coming up with a duplicate
implementation. But NMPNetns may want to log, so we also need nm-logging
thread-safe.
In general, code under "shared/nm-utils" and nm-logging should be usable
from multiple threads. It's simpler to make this code thread-safe than
re-implementing it. Also, it's a bad limitation to be unable to log
from other threads. If there is an error, the best we can often do is to
log about it.
Make nm-logging thread-safe. Actually, we only need to be able to log
from multiple threads. We don't need to setup or configure logging from
multiple threads. This restriction allows us to access logging from the
main-thread without any thread-synchronization (because all changes in
the logging setup are also done from the main-thread).
So, while logging from other threads requires a mutex, logging from the
main-thread is lock-free.
Instead of having two functions nm_logging_set_syslog_identifier()
and nm_logging_set_prefix(), merge them.
They must both be called at earliest point and together. No point
in giving them the appearance that they could be called any time.
Previously, _nm_logging_clear_platform_logging_cache was an extern variable,
and NMLinuxPlatform would set it to a function pointer at certain points.
That's unnecessary complex, also when trying to make nm-logging thread-safe,
it's just more global variables that need to be considered. Don't do it
that way, but just link in a regular function.
In core ("src/"), we use "nm-logging.h" for all logging. This dispatches
for logging to syslog, glog or systemd-journald.
If we want to log from a shared component under "shared/", we need to
use a common logging function. Add "nm-utils/nm-logging-fwd.h" for
forward declaring the used logging mechaism.
The shared library will still need to link with "src/nm-logging.c"
or an alternative implementation, depending on whether it is used
inside core or not.
Correct the spelling across the *entire* tree, including translations,
comments, etc. It's easier that way.
Even the places where it's not exposed to the user, such as tests, so
that we learn how is it spelled correctly.
Using '#ifdef' is generally error prone. It's better to always define
a define and check for it explicitly. This way, the compiler can issue
a warning if the define does not exist.
Also, note how meson would always define NM_MORE_LOGGING, possibly to
"0". That means, for meson, we unintentionally always enabled more
logging because the define was always present.
Fix that.
"debug" was documentation in `man NetworkManager.conf` as a valid
logging backend. However, it was completely ignored by
nm_logging_syslog_openlog().
In fact, it makes not sense. Passing debug = TRUE to
nm_logging_syslog_openlog(), means that all messages will be
printed to stderr in addition to syslog/journal. However, when
NetworkManager is daemonizing, stderr is closed.
Whether NetworkManager is daemonizing depends entirely on command
line options --no-daemon and --debug. Hence, the logging backend "debug"
from the configuration file either conflicts or is redundant.
Also, adjust logging backend description in `man NetworkManager.conf`.
Also, log a warning about invalid/unsupported logging backend.
(cherry picked from commit 2ccf6168dc)
Coverity claims:
1. NetworkManager-1.9.2/src/devices/team/nm-device-team.c:303:
unsigned_compare: This greater-than-or-equal-to-zero comparison of an
unsigned value is always true. "0U <= LOGL_DEBUG".
Previously, the daemon would just use syslog with LOG_PERROR when run with
--debug option, even when actually configured to log into the journal.
Let's respect the configuration, but preserve the logging to stderr.
For nm-iface-helper we want to use a different syslog-identifier then
"NetworkManager".
Since we build "nm-logging.c" as part of libNetworkManagerBase.la,
it would be cumbersome to compile the logging part multiple times
with different -DG_LOG_DOMAIN settings.
Instead, allow configuring at runtime.
A new object NMPacRunnerManager has been added to manage and interact
PacRunner. It invokes both DBus methods on PacRunner DBus interface.
It stores the returned object path from CreateProxyConfiguration()
to feed as parameter to DestroyProxyCofiguration() when network goes down.
The function name is no longer visible in the default
logging output. It is anyway only used together with
journal logging to set "CODE_FUNC".
Drop it. It allows to remove the strings from the binary,
which decreases the object size of a default build of NetworkManager
from 2437400 to 2412824 bytes (-24k, -1%).
The logging domain VPN_PLUGIN controlls logging of the VPN plugins.
Especially at verbose levels <debug> and <trace>, the plugins might
reveal sensitive information in the logging.
Thus, this level should not be enabled by a
$ nmcli logging general level DEBUG domains ALL
It should only be enabled when requested explicitly.
$ nmcli logging general level DEBUG domains ALL,VPN_PLUGIN:DEBUG
Previously, the special level VPN_PLUGIN was entirely excluded from
ALL and DEFAULT domains and it was entirely disabled by default. That
is however to strict, as it completely silences the VPN plugins by
defult. Now, enable them by default up to level INFO.
VPN plugins should take care that they don't reveal sensitive
information at levels <info> (LOG_NOTICE) and higher (less verbose).
For more verbose levels they may print passwords, but that should
still be avoided as far as possible.
This logging domain will be used to enable debugging of the VPN plugins.
However, the plugins might expose sensitive data in this mode, so exclude
the new domain from "LOGD_ALL".
With this check, the function is safe to use even with invalid
logging levels. But it can still be inlined as for most cases
@level is a (enum) constant and the check can be evaluated at
compile time.
Oddly enough, on a default build with
./autogen.sh && make && strip ./src/NetworkManager
this patch decreases the size of the binary by 8k.
Basically every logging statement is wrapped by a nm_logging_enabled()
to evaluate the function call of the logging lazy.
Make the function a candidate for inlining, it safes some space. On
a default build it goes for me from 2580584 to 2560104 bytes (20k).
$ ./autogen.sh && make && strip ./src/NetworkManager
"nm-logging.h" is a basic core module that should have few other
dependencies. Instead of letting nm-logging.c directly call a function
from "nm-linux-platform.c", let platform register a handler as needed.
This way, you can build a core library containing nm-logging but no
nm-platform.
- "gsystem-local-alloc.h" and <gio/gio.h> are already included via
"nm-default.h". No need to include them separately.
- include "nm-macros-internal.h" via "nm-default.h" and drop all
explict includes.
- in the modified files, ensure that we always include "config.h"
and "nm-default.h" first. As second, include the header file
for the current source file (if applicable). Then follow external
includes and finally internal nm includes.
- include nm headers inside source code files with quotes
- internal header files don't need to include default headers.
They can savely assume that "nm-default.h" is already included
and with it glib, nm-glib.h, nm-macros-internal.h, etc.
Previsously, _LOGT() could be disabled at compile time. Thus it
was different then the other macros _LOGD(), _LOGI(), etc.
OTOH, _LOGt() was the macro that always was compiled in.
Swap the name of the macros. Now the upper-case macros are always
enabled, while the lower-case macro _LOGt() is enabled depending
on compile configuration.
Without this, the user cannot configure only certain logging domains
without touching them all.
E.g.
# nmcli general logging level DEBUG domains PLATFORM
will disable all non-PLATFORM domains.
Well, the user can do:
# nmcli general logging level INFO domains PLATFORM:DEBUG
# nmcli general logging level DEBUG domains ALL:INFO,PLATFORM
but in this case all non-PLATFORM domains are reset explicitly.
Now the user can:
# nmcli general logging level KEEP domains PLATFORM:DEBUG
# nmcli general logging level DEBUG domains ALL:KEEP,PLATFORM
which will only change the platform domain.
We already have the macros _LOGD(), _LOGI(), etc. to provide context sensitive
logging (such as printing the object pointer as prefix).
In some implementations, we would like to have a second set of logging
macros, that shall be used differently. For example, use the default
_LOGD() for messages that are explicitly issued by one objects, and use
_LOG2D() in a static context when no object is around.
The "_LOG2" prefix is not great from a naming point of view. However, it is
meant to be a second (alternative) set of logging macros with the same
usage pattern as the _LOGD() macros.
The logging macros _LOGD(), etc. are specific to each
file as they format the message according to their context.
Still, they were cumbersome to define and their implementation
was repeated over and over (slightly different at times).
Move the declaration of these macros to "nm-logging.h".
The source file now only needs to define _NMLOG(), and either
_NMLOG_ENABLED() or _NMLOG_DOMAIN.
This reduces code duplication and encourages a common implementation
and usage of these macros.
The only way to disable logging for a domain entirely is to
omit the domain from the "domains" list. For example:
"level=INFO, domains=PLATFORM,..."
Now add an explicit level "OFF" to facilitate configuration like:
"level=INFO, domains=ALL,WIFI_SCAN:OFF"
It also supports
"level=OFF, domains=PLATFORM:INFO"
but this is for the most part equivalent to
"level=INFO, domains=PLATFORM"
Rather than randomly including one or more of <glib.h>,
<glib-object.h>, and <gio/gio.h> everywhere (and forgetting to include
"nm-glib-compat.h" most of the time), rename nm-glib-compat.h to
nm-glib.h, include <gio/gio.h> from there, and then change all .c
files in NM to include "nm-glib.h" rather than including the glib
headers directly.
(Public headers files still have to include the real glib headers,
since nm-glib.h isn't installed...)
Also, remove glib includes from header files that are already
including a base object header file (which must itself already include
the glib headers).
Remove nm_logging_syslog_closelog(). The reasons are:
- closelog() is optional according to the manual.
- we called nm_logging_syslog_closelog() at the end of the
main() function. But we have destructors running afterwards,
so we were closing the log before logging the last line.
Apparently that had no bad consequences either, so why was
closelog() even useful?
Also, it's hard to determine when we log the last line and
only closelog() afterwards.
- closelog() does not revert what openlog() did, this is ugly.
We already have "nm-utils*.h" and "NetworkManagerUtils.h" headers. Rename
"include/nm-utils-internal.h" to "nm-macros-internal.h". I think that
name is better, because this file is header-only, internal, and
repository-wide.
Also, it will never contain non-header-only declarations because
there is no backing object file under "include/".
It will only contain macros and inline functions.
