Pacman-dev

Download

pacman-dev@lists.archlinux.org

August 2026

1 participants
1 discussions

[BUG] pacman -Qii: NULL from get_backup_file_status() passed to, strlen() (regression in 3a112668)
by Albrecht Scheidig 26 Aug '26

26 Aug '26

Hi, pacman -Qii segfaults on any package that has a backup= entry whose checksum cannot be computed. get_backup_file_status() returns NULL after printing the error, and its only caller passes that straight to strlen(). Because the error message is printed immediately before the crash, this looks at first glance like a graceful failure, but the process dies with SIGSEGV and exit 139. This is reachable from an ordinary repo package: extra/ldc currently ships backup=('etc/ldc2.conf') where that path is a directory, so pacman -Qii returns 139 on every machine that has ldc installed. I have filed that separately against the package; this mail is about pacman crashing on the malformed entry, which seems worth fixing on its own. Affected versions ----------------- Present in 6.1.0, 7.0.0, 7.1.0 and current master (src/pacman/package.c line 425 in master, line 431 in 7.1.0). Not present in 6.0.x. Tested on pacman 7.1.0 / libalpm 16.0.1, Arch Linux x86_64, kernel 7.1.9-arch1-2, glibc 2.42. Reproducer ---------- Self-contained, needs no third-party package, and touches nothing outside a throwaway root. Save this as PKGBUILD in an empty directory: pkgname=backup-dir-repro pkgver=1 pkgrel=1 pkgdesc="Reproducer: a directory listed in backup=()" arch=('any') license=('MIT') backup=('etc/repro.conf') package() { install -d "$pkgdir/etc/repro.conf" echo 'key = value' > "$pkgdir/etc/repro.conf/10-example.conf" } then: makepkg -f R=$PWD/root; mkdir -p "$R/var/lib/pacman" sudo pacman --root "$R" --dbpath "$R/var/lib/pacman" --noconfirm \ -U backup-dir-repro-1-1-any.pkg.tar.zst pacman --root "$R" --dbpath "$R/var/lib/pacman" -Qii \ backup-dir-repro; echo $? Result: error: could not calculate checksums for .../root/etc/repro.conf error: segmentation fault Please submit a full bug report with --debug if appropriate. 139 Backtrace --------- gdb against the distro binary, symbols from debuginfod: #0 __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:76 #1 dump_pkg_backups (pkg=0x55555564f1c0, cols=<optimized out>) at ../src/pacman/package.c:431 #2 dump_pkg_full (pkg=pkg@entry=0x55555564f1c0, extra=1) at ../src/pacman/package.c:351 #3 display (pkg=0x55555564f1c0) at ../src/pacman/query.c:309 #4 pacman_query (targets=<optimized out>) at ../src/pacman/query.c:503 #5 main (argc=<optimized out>, argv=<optimized out>) at ../src/pacman/pacman.c:1341 Locals in frame #1: value = 0x0, text = 0x0, root = "/", and *backup = {name = "etc/ldc2.conf", hash = "(null)"}. Root cause ---------- get_backup_file_status() returns NULL after reporting the error (master lines 380-385): if(access(path, R_OK) == 0) { char *md5sum = alpm_compute_md5sum(path); if(md5sum == NULL) { pm_printf(ALPM_LOG_ERROR, _("could not calculate checksums for %s\n"), path); return NULL; } dump_pkg_backups() dereferences it unconditionally (master 418-425): if(!backup->hash) { continue; } value = get_backup_file_status(root, backup); needed = strlen(root) + strlen(backup->name) + 1 + strlen(value) + 1; /* ^ value == NULL */ Why the existing !backup->hash guard does not catch it: the stored hash is not a NULL pointer. libalpm serialises the failed checksum through "%s" in lib/libalpm/be_local.c:1118, fprintf(fp, "%s\t%s\n", backup->name, backup->hash); so a NULL hash is written out via the glibc "%s" extension as the literal six-character string "(null)", which reads back as a perfectly valid non-empty string. The guard passes and the crash path is taken. Two consequences independent of the crash: - The bad value is written at install time with no diagnostic, so the broken state is persistent: once such a package is installed, pacman -Qii stays broken until the package is reinstalled. - If the path later were a readable regular file, its real md5 would be compared against "(null)" and always report [modified]. Regression ---------- The NULL return is old and used to be harmless. Commit 3a112668 ("pacman: improve backup printing", 2021-12-29) replaced a tolerant printf with the strlen arithmetic: /* before 3a112668 - glibc prints the string "(null)", no crash */ value = get_backup_file_status(root, backup); printf("%s\t%s%s\n", value, root, backup->name); /* after 3a112668 */ value = get_backup_file_status(root, backup); needed = strlen(root) + strlen(backup->name) + 1 + strlen(value) + 1; Tags containing 3a112668: v6.1.0, v7.0.0, v7.1.0. Which backup entry types are affected ------------------------------------- Only the directory case reaches the NULL return; the others diverge earlier. I verified the first and third in isolated roots; the fifo behaviour is from issue #97: - broken symlink: access(path, R_OK) fails with ENOENT, so the entry is reported [missing] and pacman exits 0. No crash - even though the database also holds "(null)" for it, which confirms the crash is specific to reaching the NULL return rather than to the stored string. - fifo: access() succeeds, the md5 open() blocks, pacman hangs. - directory: access() succeeds, alpm_compute_md5sum() returns NULL, SIGSEGV. Related ------- Issue #97 ("Silly behaviour when installing a symlink and marking it as a backup file", open since 2024-02-21) reports the "(null)" database value and the fifo hang for non-regular backup entries, and asks for an install-time error, defined behaviour, or documentation that backup=() must contain regular files. It does not mention directories, -Qii, or any crash. Fixing the NULL deref does not address #97, and addressing #97 by rejecting non-regular entries at build/install time would not fix this crash for databases already on disk. MR !324 ("libalpm: add SHA-256 hashes to %BACKUP% entries") touches the same serialisation format. Proposed fix ------------ Preferred, keeping the entry visible with a defined status ([unknown] already exists for the unclassified case): --- a/src/pacman/package.c +++ b/src/pacman/package.c @@ get_backup_file_status if(md5sum == NULL) { pm_printf(ALPM_LOG_ERROR, _("could not calculate checksums for %s\n"), path); - return NULL; + return "[unknown]"; } More conservative, if the NULL return should stay as "no status available": --- a/src/pacman/package.c +++ b/src/pacman/package.c @@ dump_pkg_backups value = get_backup_file_status(root, backup); + if(!value) { + continue; + } needed = strlen(root) + strlen(backup->name) + 1 + strlen(value) + 1; Separately, be_local.c arguably should not write a NULL hash as "(null)". Omitting the entry, or failing loudly during the transaction, would keep the malformed state from becoming persistent - which is roughly what #97 is asking for. -- Albrecht

1 0