If flock(local_lock_fd,...) fails, in function local_server(), the file
descriptor to the lock file is closed but local_lock_fd is not reset to
-1. This leads to server() calling end_local_server(), which closes the
file descriptor again.
Fix this double-close issue by setting local_lock_fd to -1 after closing
it.
This issue was found by using Facebook's Infer static analyzer.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
This completely inactivate the .desktop file incase the user session is
managed by systemd as restorecond also provide a service file
Signed-off-by: Laurent Bigonville <bigon@bigon.be>
The user systemd service file could be installed in an other location than the
system ones. In debian for example, the system files are installed
/lib/systemd/system and the user ones in /usr/lib/systemd/user.
Suggested-by: Laurent Bigonville <bigon@bigon.be>
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
Acked-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Make user restorecond systemd service consistent with restorecond_user.conf file
used by `restorecond -u`
Signed-off-by: Petr Lautrbach <plautrba@redhat.com>
When restorecond starts, it installs a SIGTERM handler in order to exit
cleanly (by removing its PID file). When restorecond --user starts,
there is no PID file, and g_main_loop_run() does not stop when master_fd
is closed. This leads to an unkillable service, which is an issue.
Fix this by overriding the handler for SIGTERM in restorecond --user.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
When running restorecond in user sessions using D-Bus activation,
restorecond's process is spawned in the CGroup of the D-Bus daemon:
$ systemctl --user status
[...]
CGroup: /user.slice/user-1000.slice/user@1000.service
├─init.scope
│ ├─1206 /usr/lib/systemd/systemd --user
│ └─1208 (sd-pam)
└─dbus.service
├─1628 /usr/bin/dbus-daemon --session --address=systemd:
└─4570 /usr/sbin/restorecond -u
In order to separate it, introduce a systemd unit for
restorecond-started-as-user.
After this patch:
CGroup: /user.slice/user-1000.slice/user@1000.service
├─restorecond-user.service
│ └─2871 /usr/sbin/restorecond -u
├─init.scope
│ ├─481 /usr/lib/systemd/systemd --user
│ └─485 (sd-pam)
└─dbus.service
└─2868 /usr/bin/dbus-daemon --session --address=systemd:
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=955940 states:
dbus-glib is a deprecated D-Bus library with some significant design
flaws, and is essentially unmaintained.
restorecond uses dbus-glib in order to spawn as a D-Bus service on the
session bus of users. This makes restorecond stays so long as the user
session exists.
Migrate from dbus-glib to GDbus API for the implementation of this
feature.
Moreover restorecond currently uses a D-Bus signal to trigger starting
the service. This is quite inappropriate, as stated for example in
https://dbus.freedesktop.org/doc/dbus-tutorial.html#members
Methods are operations that can be invoked on an object, with
optional input (aka arguments or "in parameters") and output (aka
return values or "out parameters"). Signals are broadcasts from the
object to any interested observers of the object; signals may
contain a data payload.
Implementing a method is more appropriate. It appears that all D-Bus
users can implement method Ping from interface org.freedesktop.DBus.Peer
(https://dbus.freedesktop.org/doc/dbus-specification.html#standard-interfaces-peer)
and that calling this method is enough to trigger the launch of the
service. This can be tested in a shell by running:
gdbus call --session --dest=org.selinux.Restorecond \
--object-path=/ --method=org.freedesktop.DBus.Peer.Ping
As this method is automatically provided, there is no need to implement
its handling in the service.
Fixed: https://github.com/SELinuxProject/selinux/issues/217
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
When starting restorecond without any option the following redundant
console log is outputed:
/dev/log 100.0%
/var/volatile/run/syslogd.pid 100.0%
...
This is caused by two global variables of same name r_opts. When
executes r_opts = opts in restore_init(), it originally intends
to assign the address of struct r_opts in "restorecond.c" to the
pointer *r_opts in "restore.c".
However, the address is assigned to the struct r_opts and covers
the value of low eight bytes in it. That causes unexpected value
of member varibale 'nochange' and 'verbose' in struct r_opts, thus
affects value of 'restorecon_flags' and executes unexpected operations
when restorecon the files such as the redundant console log output or
file label nochange.
Cause restorecond/restore.c is copied from policycoreutils/setfiles,
which share the same pattern. It also has potential risk to generate
same problems, So fix it in case.
Signed-off-by: Baichuan Kong <kongbaichuan@huawei.com>
Use codespell (https://github.com/codespell-project/codespell) in order
to find many common misspellings that are present in English texts.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
For some reasons, restorecond was explicitly linking against libpcre but
the code is not using any of its symbols
Closes: https://github.com/SELinuxProject/selinux/issues/137
Signed-off-by: Laurent Bigonville <bigon@bigon.be>
On most distributions, /var/run is a symbolic link to /run so using
/var/run or /run lead to the same result. Nevertheless systemd started
to warn about using /var/run in a service file, logging entries such as:
/usr/lib/systemd/system/restorecond.service:8: PIDFile= references
path below legacy directory /var/run/, updating
/var/run/restorecond.pid → /run/restorecond.pid; please update the
unit file accordingly.
Switch to /run in order to follow this advice.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
Since the default value of watch_file is set unconditionally *after* the
command-line arguments have been parsed, the -f option is (and has
always been) effectively ignored. Fix this by setting it before the
parsing.
Fixes: 48681bb49c ("policycoreutils: restorecond: make restorecond dbuss-able")
Signed-off-by: Ondrej Mosnacek <omosnace@redhat.com>
write_pid_file() leaks a file descriptor to /var/run/restorecond.pid if
it fails to write the PID to it. Close the file before returning.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
musl doesn't implement GLOB_BRACE and GLOB_TILDE, so simply don't use
them there. This affects restorecond -u but braces are not used in the
example configs. GLOB_TILDE is on the roadmap[1] for musl 1.1.21 so
restorecond -u should be fine soon.
[1]: https://wiki.musl-libc.org/roadmap.html
Signed-off-by: Jason Zaman <jason@perfinion.com>
When compiling restorecond with -Wunused, gcc 4.8.4 (from Ubuntu 14.04)
reports the following warnings:
restorecond.c: In function ‘main’:
restorecond.c:208:9: error: ignoring return value of ‘daemon’,
declared with attribute warn_unused_result [-Werror=unused-result]
daemon(0, 0);
^
restorecond.c: In function ‘write_pid_file’:
restorecond.c:106:2: error: ignoring return value of ‘write’,
declared with attribute warn_unused_result [-Werror=unused-result]
(void)write(pidfd, val, (unsigned int)len);
^
If any of these calls returns an error, it is currently silently
discarded. Add a message in order to warn about such an error.
Signed-off-by: Nicolas Iooss <nicolas.iooss@m4x.org>
There were several places in the makefiles where LDLIBS or CFLAGS were
supposed to include options to build. They were missing the override
keyword so would be skipped if these vars were set on the make cmdline.
Add the override directive to fix this.
Signed-off-by: Jason Zaman <jason@perfinion.com>
