-
Notifications
You must be signed in to change notification settings - Fork 327
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ci: Various fixes for update to Ubuntu 24.04 #1461
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ac000
force-pushed
the
ci-fix
branch
14 times, most recently
from
October 16, 2024 21:23
ffd49ed
to
8e2f442
Compare
With Ubuntu 24.04 this service is no longer enabled/installed and so this bit would fail. This commit makes it handle both cases (installed/not-installed). Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
With Ubuntu 24.04 installing it via pip gave this error error: externally-managed-environment × This environment is externally managed ╰─> To install Python packages system-wide, try apt install python3-xyz, where xyz is the package you are trying to install. If you wish to install a non-Debian-packaged Python package, create a virtual environment using python3 -m venv path/to/venv. Then use path/to/venv/bin/python and path/to/venv/bin/pip. Make sure you have python3-full installed. If you wish to install a non-Debian packaged Python application, it may be easiest to use pipx install xyz, which will manage a virtual environment for you. Make sure you have pipx installed. See /usr/share/doc/python3.12/README.venv for more information. Installing it via the package manager is the better option anyway... Under Ubuntu 22.04 it only installs a /usr/bin/pytest-3 binary, rather than installing a /usr/bin/pytest binary and symlink for pytest-3, so use pytest-3 as the command. Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
Under Ubuntu 24.04 the pytest for test/test_php_isolation.py::test_php_isolation_rootfs fails due to Unit aborting (SIGABRT) in the PHP language module due to FORIFY_SOURCE hardening detecting a buffer overflow 2024/10/16 16:46:54 [info] 11661#11661 "phpinfo" application started *** buffer overflow detected ***: terminated 2024/10/16 16:46:54 [alert] 11660#11660 app process 11661 exited on signal 6 After spending an extraordinary amount of time faffing around with Ubuntu and pytests (they don't make for a pleasant combination) I was able to reproduce it. The crash was occurring here nginx#4 0x00007ebe818288ff in __GI_abort () at ./stdlib/abort.c:79 nginx#5 0x00007ebe818297b6 in __libc_message_impl ( fmt=fmt@entry=0x7ebe819ce765 "*** %s ***: terminated\n") at ../sysdeps/posix/libc_fatal.c:132 nginx#6 0x00007ebe81936c19 in __GI___fortify_fail ( msg=msg@entry=0x7ebe819ce74c "buffer overflow detected") at ./debug/fortify_fail.c:24 nginx#7 0x00007ebe819365d4 in __GI___chk_fail () at ./debug/chk_fail.c:28 nginx#8 0x00007ebe8134a055 in mempcpy (__len=10, __src=0x7ebe8160ade8, __dest=0x571ba9bd0930) at /usr/include/x86_64-linux-gnu/bits/string_fortified.h:45 nginx#9 fake_data_segment (info=0x0, sysdb=0x571ba9bcf080) at /usr/src/php8.1-8.1.30-1+ubuntu24.04.1+deb.sury.org+1/ext/date/lib/parse_tz.c:921 nginx#10 timelib_builtin_db () at /usr/src/php8.1-8.1.30-1+ubuntu24.04.1+deb.sury.org+1/ext/date/lib/parse_tz.c:1084 nginx#11 0x00007ebe812e0885 in zm_info_date (zend_module=0x571ba9a14420) [Well as best as I can tell, as this is from the php 8.1 packages from <https://github.com/oerdnj/deb.sury.org>, I don't know where the packages (I'm assuming it's packages) shivammathur/setup-php@v2 installs come from.] So we get killed in fake_data_segment(), the thing is, that function (as well as timelib_builtin_db()) doesn't exist in upstream PHP. It turns out these come from a patch that is applied by distributions to make PHP use the system installed timezone database rather than the one built into PHP. I was unable to reproduce this with vanilla PHP 8.1. It can be triggered on affected builds with the following config { "listeners": { "[::1]:8080": { "pass": "applications/php" } }, "applications": { "php": { "type": "php", "root": "/app/php", "isolation": { "rootfs": "/tmp/unit-root", "namespaces": { "mount": true, "credential": true, "pid": true } } } } } The crux of the issue seems to come down to in this case PHP can't open the tz database as it's not contained in the new mount namespace. 190437 openat(AT_FDCWD, "/usr/share/zoneinfo/", O_RDONLY|O_NONBLOCK|O_CLOEXEC|O_DIRECTORY) = -1 ENOENT (No such file or directory) 190437 openat(AT_FDCWD, "/usr/share/zoneinfo/zone.tab", O_RDONLY) = -1 ENOENT (No such file or directory) 190437 writev(2, [{iov_base="*** ", iov_len=4}, {iov_base="buffer overflow detected", iov_len=24}, {iov_base=" ***: terminated\n", iov_len=17}], 3) = 45 ... 190437 --- SIGABRT {si_signo=SIGABRT, si_code=SI_TKILL, si_pid=2, si_uid=65534} --- 190437 +++ killed by SIGABRT +++ Specifically the issue is with the following code in the patch (certainly an earlier version of the patch, this is from a Debian patch <https://sources.debian.org/src/php8.2/8.2.20-1~deb12u1/debian/patches/0007-Add-support-for-use-of-the-system-timezone-database.patch/>) + data = malloc(3 * sysdb->index_size + 7); + + p = mempcpy(data, FAKE_HEADER, sizeof(FAKE_HEADER) - 1); If the zone file hasn't been found then sysdb->index_size is 0. So we malloc(3) a total of 7 bytes. However, sizeof(FAKE_HEADER) - 1 is 10. (Hence the __len=10 in the mempcpy(3) in the above backtrace). Of course 10 doesn't fit into 7 and the FORTIFY_SOURCE hardening kicks in and SIGABRTs the process. Now, it's worth noting that this issue doesn't occur with PHP 8.2 and 8.3. As can been seen from the Fedora patch for this <https://src.fedoraproject.org/rpms/php/blob/rawhide/f/php-8.4.0-systzdata-v24.patch> They actually have a fix incorporated r23: fix possible buffer overflow So the above patch now does + data = malloc(3 * sysdb->index_size + sizeof(FAKE_HEADER) - 1); + + p = mempcpy(data, FAKE_HEADER, sizeof(FAKE_HEADER) - 1); So you will always get at least the required 10 bytes allocated. I assume the PHP 8.2 & 8.3 packages either no longer use this patch or have the fixed version. I don't know... I haven't found the sources... Anyway the above was more about satisfying myself that the problem wasn't with Unit. PHP 8.1 is now in security maintenance mode and people are actively encouraged to upgrade to 8.2/8.3 So lets just remove 8.1 from our testing... [It's also worth noting that after all this, the ubuntu-latest runners seemed to have switched back from 24.04 to 22.04. However lets stick with this and the other ci fixes as who knows when it'll go back to 24.04 (or some other version) again...] Link: <https://www.php.net/supported-versions.php> Signed-off-by: Andrew Clayton <a.clayton@nginx.com>
ac000
changed the title
ci: Fix disabling of mono-xsp4.service
ci: Various fixes for update to Ubuntu 24.04
Oct 17, 2024
javorszky
approved these changes
Oct 17, 2024
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed all separate commits. I really appreciate the explanation on the php issue, that was an exhaustive reasoning!
This patch is g2g from my end.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull-request compreises three patches fixing various issues with the CI due to the runners moving to Ubuntu 24.04 (though they seem to have sinnce reverted back, but we may as well keep these fixes...)
The first patch fixes the disabling of the mono service which may or may not be installed.
The second patch fixes the installation of pytest by installing it via the apt(8) package manager.
The third drops PHP 8.1 from our tests. See the commit message for the gory details.