Help with debugging of -D_FORTIFY_SOURCE=3

[marxin]

I'm in the middle of the debugging session where I'm trying to use -D_FORTIFY_SOURCE=3 with the upcoming GCC compiler. I noticed that using the flags for systemd and then using /lib64/libudev.so.xyz library triggers errors in your project test-suite:

[   16s] >>> LD_LIBRARY_PATH=/home/abuild/rpmbuild/BUILD/umockdev-0.17.8/x86_64-suse-linux GI_TYPELIB_PATH=/home/abuild/rpmbuild/BUILD/umockdev-0.17.8/x86_64-suse-linux G_DEBUG=fatal-warnings,fatal-criticals,gc-friendly PATH=/home/abuild/rpmbuild/BUILD/umockdev-0.17.8/x86_64-suse-linux:/usr/local/bin:/usr/bin:/bin TOP_SRCDIR=/home/abuild/rpmbuild/BUILD/umockdev-0.17.8 /home/abuild/rpmbuild/BUILD/umockdev-0.17.8/src/umockdev-wrapper /home/abuild/rpmbuild/BUILD/umockdev-0.17.8/x86_64-suse-linux/test-umockdev-record
[   16s] ――――――――――――――――――――――――――――――――――――― ✀  ―――――――――――――――――――――――――――――――――――――
[   16s] stdout:
[   16s] # random seed: R02Se409341646f807cd297687b9042afab9
[   16s] 1..17
[   16s] # Start of umockdev-record tests
[   16s] # GLib-DEBUG: setenv()/putenv() are not thread-safe and should not be used after threads are created
[   16s] # DEBUG: umockdev.vala:110: Created udev test bed /tmp/umockdev.ZI5SJ1
[   16s] # DEBUG: umockdev.vala:137: Removing test bed /tmp/umockdev.ZI5SJ1
[   16s] # GLib-DEBUG: unsetenv() is not thread-safe and should not be used after threads are created
[   16s] ok 1 /umockdev-record/testbed-all-empty
[   16s] # GLib-DEBUG: setenv()/putenv() are not thread-safe and should not be used after threads are created
[   16s] # DEBUG: umockdev.vala:110: Created udev test bed /tmp/umockdev.WI9SJ1
[   16s] # DEBUG: umockdev.vala:765: umockdev_testbed_uevent: lazily initializing uevent_sender
[   16s] # DEBUG: umockdev.vala:769: umockdev_testbed_uevent: sending uevent add for device /sys/devices/dev1
[   16s] stderr:
[   16s] test-umockdev-record: ../src/uevent_sender.c:248: uevent_sender_send: Assertion `subsystem != NULL' failed.

As seen, we hit the following assert uevent_sender.c:248: uevent_sender_send: Assertion subsystem != NULL' failed..

So the question is: how can I debug it? Am I correct that umockdev shadows (implements) some of the libudev.so functions?

@siddhesh

[benzea]

In this case, umockdev primarily redirects the path when opening files to point to a fake temporary directory instead.

So we are looking at udev_device_get_subsystem returning NULL suddenly (after udev_device_new_from_syspath). My thinking would be, that we are more likely looking at another change here (e.g. systemd/systemd@13659527efd4cf00677ccacba5994bb77dc866b0), and you are simply compiling a newer libudev.

Could that be the case?

[marxin]

In this case, umockdev primarily redirects the path when opening files to point to a fake temporary directory instead.

I see. How exactly is utilized the libdev library? I noticed it's not a shared library dependency of test binary test-umockdev-record, right? Maybe the returned udev_device object is somehow changed, can I dump it?

So we are looking at udev_device_get_subsystem returning NULL suddenly (after udev_device_new_from_syspath). My thinking would be, that we are more likely looking at another change here (e.g. systemd/systemd@1365952), and you are simply compiling a newer libudev.

Note I'm using a stable release package that is fixed. The only change is that I build the package with -D_FORTIFY_SOURCE=3 instead of -D_FORTIFY_SOURCE=2. Plus I include the following fix: https://github.com/systemd/systemd/pull/22917

Could that be the case?

No.

[benzea]

I see. How exactly is utilized the libdev library? I noticed it's not a shared library dependency of test binary test-umockdev-record, right? Maybe the returned udev_device object is somehow changed, can I dump it?

The assert you are seeing is coming from umockdev itself. i.e. the test code crashes while creating a umockdev testbed.

I believe this is the crashing call:

• https://github.com/martinpitt/umockdev/blob/master/tests/test-umockdev-record.vala#L97 which goes further to:
• https://github.com/martinpitt/umockdev/blob/master/src/umockdev.vala#L548
• https://github.com/martinpitt/umockdev/blob/master/src/uevent_sender.c#L248

So, looking at the udev code, it seems that it does a readlink on the subsystem link. I suspect, that this readlink is not intercepted properly for some reason.

You could:

1. strace the process, to see if you see the readlink and verify it returns ENOENT (and, that it is looking at the wrong path)
2. break in sd_device_get_subsystem and step through to see if the LD_PRELOAD library is used in the end for the readlink call to glibc.

So, not sure what _FORTIFY_SOURCE does, but verifying the umockdev LD_PRELOAD is working seems like a good plan.

[marxin]

So, looking at the udev code, it seems that it does a readlink on the subsystem link. I suspect, that this readlink is not intercepted properly for some reason.

Oh, that would be the root cause as readlink is one of the fortified functions and it's very likely that libudev.so calls the foritifed version: __readlink_chk:

nm /lib64/libudev.so | grep readlink
                 U __readlinkat_chk@GLIBC_2.5
...

You could:

1. strace the process, to see if you see the readlink and verify it returns ENOENT (and, that it is looking at the wrong path)

I see the following:

$ LD_LIBRARY_PATH=/home/abuild/rpmbuild/BUILD/umockdev-0.17.8/x86_64-suse-linux GI_TYPELIB_PATH=/home/abuild/rpmbuild/BUILD/umockdev-0.17.8/x86_64-suse-linux G_DEBUG=fatal-warnings,fatal-criticals,gc-friendly PATH=/home/abuild/rpmbuild/BUILD/umockdev-0.17.8/x86_64-suse-linux:/usr/local/bin:/usr/bin:/bin TOP_SRCDIR=/home/abuild/rpmbuild/BUILD/umockdev-0.17.8 strace -f -s512 /home/abuild/rpmbuild/BUILD/umockdev-0.17.8/src/umockdev-wrapper /home/abuild/rpmbuild/BUILD/umockdev-0.17.8/x86_64-suse-linux/test-umockdev-run 2>&1 | grep readlink
readlink("/proc/self/exe", "/home/abuild/rpmbuild/BUILD/umockdev-0.17.8/x86_64-suse-linux/test-umockdev-run", 4096) = 79
[pid 31145] readlink("/proc/self/exe", "/home/abuild/rpmbuild/BUILD/umockdev-0.17.8/x86_64-suse-linux/umockdev-run", 4096) = 74
[pid 31147] readlink("/proc/self/exe", "/home/abuild/rpmbuild/BUILD/umockdev-0.17.8/x86_64-suse-linux/umockdev-run", 4096) = 74
[pid 31153] readlink("/proc/self/exe", "/home/abuild/rpmbuild/BUILD/umockdev-0.17.8/x86_64-suse-linux/umockdev-run", 4096) = 74
[pid 31158] readlink("/proc/self/exe", "/home/abuild/rpmbuild/BUILD/umockdev-0.17.8/x86_64-suse-linux/umockdev-run", 4096) = 74
[pid 31173] readlink("/proc/self/exe", "/home/abuild/rpmbuild/BUILD/umockdev-0.17.8/x86_64-suse-linux/umockdev-run", 4096) = 74
[pid 31177] readlink("/proc/self/exe", "/home/abuild/rpmbuild/BUILD/umockdev-0.17.8/x86_64-suse-linux/umockdev-run", 4096) = 74
[pid 31182] readlink("/proc/self/exe", "/home/abuild/rpmbuild/BUILD/umockdev-0.17.8/x86_64-suse-linux/umockdev-run", 4096) = 74
[pid 31182] readlink("/proc/self/fd/8", "/", 4096) = 1
[pid 31182] readlink("/home/abuild/rpmbuild/BUILD/umockdev-0.17.8/devices", 0x7fffffff93c0, 1023) = -1 EINVAL (Invalid argument)
[pid 31182] readlink("/home/abuild/rpmbuild/BUILD/umockdev-0.17.8/virtual", 0x7fffffff93c0, 1023) = -1 ENOENT (No such file or directory)
[pid 31182] readlink("/home/abuild/rpmbuild/BUILD/umockdev-0.17.8/block", 0x7fffffff93c0, 1023) = -1 ENOENT (No such file or directory)
[pid 31182] readlink("/home/abuild/rpmbuild/BUILD/umockdev-0.17.8/loop23", 0x7fffffff93c0, 1023) = -1 ENOENT (No such file or directory)
[pid 31182] readlinkat(AT_FDCWD, "/sys/devices/virtual/block/loop23/subsystem", 0x55555557ab20, 4096) = -1 ENOENT (No such file or directory)

Does it explain our problem?

Anyway, thanks for help!

2. break in sd_device_get_subsystem and step through to see if the LD_PRELOAD library is used in the end for the readlink call to glibc.

So, not sure what _FORTIFY_SOURCE does, but verifying the umockdev LD_PRELOAD is working seems like a good plan.

Please take a look at explanation: https://developers.redhat.com/blog/2021/04/16/broadening-compiler-checks-for-buffer-overflows-in-_fortify_source#_fortify_source_under_the_hood

[marxin]

Nice! I can confirm the following patch fixes that:

diff --git a/src/libumockdev-preload.c b/src/libumockdev-preload.c
index 87b64a8..098640b 100644
--- a/src/libumockdev-preload.c
+++ b/src/libumockdev-preload.c
@@ -1516,6 +1516,13 @@ ssize_t readlinkat(int dirfd, const char *pathname, char *buf, size_t bufsiz)
     return r;
 }

+
+ssize_t __readlinkat_chk (int dirfd, const char *pathname, char *buf, size_t bufsiz,
+         size_t buflen)
+{
+       return readlinkat (dirfd, pathname, buf, bufsiz);
+}
+
 WRAP_2ARGS_PATHRET(char *, NULL, realpath, char *);

 char *__realpath_chk(const char *path, char *resolved, size_t size);

I'm going to do a proper fix tomorrow.

[martinpitt]

Thanks @marxin ! Indeed umockdev doesn't currently test with fortify at all. I am adding a test for _FORTIFY_SOURCE=2, and that succeeds.

Unfortunately I can't yet enable =3 yet everywhere:

/usr/include/features.h:424:5: error: #warning _FORTIFY_SOURCE > 2 is treated like 2 on this platform [-Werror=cpp]
  424 | #   warning _FORTIFY_SOURCE > 2 is treated like 2 on this platform
      |     ^~~~~~~

That is with gcc 11.2 on Fedora 35. It does work with gcc 12.0.1 in Fedora rawhide, though, so I'll make the test conditional on the compiler version.

However, this also means that I cannot reproduce this bug. (I ran this before your PR). Still, I think adding the test makes sense to cover other cases of breaking fortify. I'll land your PR and then add the test.

Thanks!

[marxin]

However, this also means that I cannot reproduce this bug. (I ran this before your PR). Still, I think adding the test makes sense to cover other cases of breaking fortify. I'll land your PR and then add the test.

As mentioned, you will see the error with GCC 12 if you would use systemd (libudev) package compiled with _FORTIFY_SOURCE=3.