FS#9235 - pacman crashes on file opening error

Attached to Project: Pacman
Opened by Dawid Wróbel (cromo) - Wednesday, 16 January 2008, 20:17 GMT
Last edited by Dan McGee (toofishes) - Friday, 08 February 2008, 02:22 GMT
Task Type Bug Report
Category General
Status Closed
Assigned To Aaron Griffin (phrakture)
Xavier (shining)
Dan McGee (toofishes)
Architecture All
Severity Low
Priority Normal
Reported Version 3.1.0
Due in Version 3.1.2
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

Description:

I did pacman -Sf --debug syslog-ng and pacman crashed. It turned out there was a problem in the filesystem, a symbolic link pointing to itself:
552:cromo@kromka:~$ ls -l /etc/syslog-ng.conf
lrwxrwxrwx 1 root root 19 sty 16 14:30 /etc/syslog-ng.conf -> /etc/syslog-ng.conf
However I think pacman should not crash in that case and report an file opening error.

Using pacman 3.1 from Core.

debug: config: finished parsing /etc/pacman.conf
debug: registering local database
debug: opening database 'local'
debug: opening database from path '/var/lib/pacman/local/'
debug: loading package cache for repository 'core'
debug: loading package cache for repository 'local'
debug: package 'syslog-ng' not found in sync
debug: adding target 'syslog-ng' to the transaction set
resolving dependencies...
debug: resolving target's dependencies
debug: started resolving dependencies
debug: checkdeps: package syslog-ng-2.0.6-1
debug: found package 'syslog-ng-2.0.6-1' in sync
debug: started sorting dependencies
debug: sorting dependencies finished
looking for inter-conflicts...
debug: looking for conflicts
debug: check targets vs db
debug: check db vs targets
debug: check targets vs targets
debug: checking dependencies
debug: checkdeps: package syslog-ng-2.0.6-1
debug: found cached pkg: /var/cache/pacman/pkg/syslog-ng-2.0.6-1-i686.pkg.tar.gz

Celuje: syslog-ng-2.0.6-1

Total Download Size: 0,00 MB
Całkowity rozmiar po instalacji: 0,12 MB

Kontynuować instalację? [T/n] t
debug: using cachedir: /var/cache/pacman/pkg/
debug: found cached pkg: /var/cache/pacman/pkg/syslog-ng-2.0.6-1-i686.pkg.tar.gz
debug: found cached pkg: /var/cache/pacman/pkg/syslog-ng-2.0.6-1-i686.pkg.tar.gz
checking package integrity...
debug: found cached pkg: /var/cache/pacman/pkg/syslog-ng-2.0.6-1-i686.pkg.tar.gz
debug: md5(/var/cache/pacman/pkg/syslog-ng-2.0.6-1-i686.pkg.tar.gz) = 9151976d0468bedef092e3e9c7fe0026
debug: installing packages
debug: found cached pkg: /var/cache/pacman/pkg/syslog-ng-2.0.6-1-i686.pkg.tar.gz
debug: loading target '/var/cache/pacman/pkg/syslog-ng-2.0.6-1-i686.pkg.tar.gz'
debug: sorting package filelist for /var/cache/pacman/pkg/syslog-ng-2.0.6-1-i686.pkg.tar.gz
installing syslog-ng...
debug: adding package syslog-ng-2.0.6-1
debug: extracting files
debug: decompression progress: 7,897152% (10240 / 129667)
debug: skipping extraction of '.PKGINFO'
debug: decompression progress: 7,897152% (10240 / 129667)
debug: skipping extraction of '.FILELIST'
debug: decompression progress: 7,897152% (10240 / 129667)
debug: extract: skipping dir extraction of etc/
debug: decompression progress: 7,897152% (10240 / 129667)
błąd: md5: /etc/syslog-ng.conf can't be opened
debug: md5(/tmp/alpm_mkyjNZ) = ee95af3a4f969f3759e477289305c8c6
debug: checking hashes for etc/syslog-ng.conf
debug: current: (null)
debug: new: ee95af3a4f969f3759e477289305c8c6
debug: original:
błąd: segmentation fault
błąd: Internal pacman error: Segmentation fault.
Please submit a full bug report with --debug if appropriate.
This task depends upon

Closed by  Dan McGee (toofishes)
Friday, 08 February 2008, 02:22 GMT
Reason for closing:  Fixed
Additional comments about closing:  Fixed in commit 0c2206f542ce6df2606586d43f190cd5a423fb13 .
Comment by Dan McGee (toofishes) - Wednesday, 16 January 2008, 23:22 GMT
line 503ish, libalpm/add.c

We currently don't do null checks on the return from alpm_get_md5sum(). We could implement them, but without sitting down for a bit and thinking I'm not sure what to do in this case.

Perhaps setting them to "" (empty string) as is done with hash_orig around line 441 would be best?

What happens if I delete a conf file from a package and attempt to reinstall? I'd like to see the current/new/original lines in that case. Ahh, nevermind- we completely skip the backkup in this case. Thus we are assuming if we can stat the file, we could get an md5sum.

Thoughts?
Comment by Xavier (shining) - Wednesday, 16 January 2008, 23:58 GMT
> Perhaps setting them to "" (empty string) as is done with hash_orig around line 441 would be best?

I think we could do that indeed.
Comment by Xavier (shining) - Thursday, 17 January 2008, 00:41 GMT
Hmm, actually, I am not sure. Even if we do this, alpm_get_md5sum prints an error when failing, so that would look confusing imo.
And by setting the current hash to "", are we sure it will always extract the file as .pacnew? It looks like that is what it should do,
but I got something weird once while testing, where it tried to overwrite the broken symlink, but failed. I couldn't reproduce it though.

But since we already stat the file, maybe we could just fail when it doesn't find it? I am not sure this is always a wise thing to do however.
But a broken symlink stays a corner case. So I think pacman can fail in this case, as long as it doesn't segfault.
   symlink.diff (0.6 KiB)
Comment by Dan McGee (toofishes) - Friday, 18 January 2008, 05:00 GMT
  • Field changed: Due in Version (3.1.1 → 3.1.2)
Let's peg this for 3.1.2 so we can make sure we get a real fix working.
Comment by Dan McGee (toofishes) - Thursday, 07 February 2008, 04:56 GMT
Ping to myself and Xavier. Can we resolve this for 3.1.2? I think it just needs someone to sit down and think out the critical cases.

I think your solution above is on the right track, but at the same time I don't know that skipping the extraction of the file completely is our best solution. I feel like there could be some situation where this could occur naturally.
Comment by Xavier (shining) - Thursday, 07 February 2008, 14:09 GMT
What about this?
   0001-libalpm-add.c-safety-che... (1.6 KiB)
Comment by Dan McGee (toofishes) - Thursday, 07 February 2008, 14:41 GMT
Ahh, I like this much more. Anyway we can pactest this particular case here?
Comment by Xavier (shining) - Thursday, 07 February 2008, 14:56 GMT
Unfortunately, no we can't. a broken symlink makes the pactest program itself fail for the same reason.
It is trying to get the md5sum of the broken symlink, and fails.

To be honest, I was more interested in fixing pacman than pactest :)
Comment by Dan McGee (toofishes) - Thursday, 07 February 2008, 14:59 GMT
Sounds like we need a if(stat) check there too just like we did in pacman. :)

I might take a look at this later and see if it is an easy fix in pactest- it should be pretty straightforward.
Comment by Xavier (shining) - Thursday, 07 February 2008, 15:50 GMT
Sorry, I find pactest program even more difficult to fix than pacman :P

I managed to make this upgrade pactest pass after adding 2 safety checks to pactest.
But when trying it before applying my patch to pacman, pacman segfaults, and then
pactest is doing weird stuff and dies like this :
IOError: [Errno 2] No such file or directory: 'blabla/root/var/lib/pacman/local/foo-1.0-2/desc'
   upgrade045.py (0.4 KiB)
   pactest.diff (0.8 KiB)

Loading...