| Summary: | sys-apps/openrc - /sbin/openrc: double free or corruption in do_start_services (parallel=false, start_services=<optimized out>) at rc.c:648 #8 main (argc=<optimized out>, argv=<optimized out>) at rc.c:1115 | ||
|---|---|---|---|
| Product: | Gentoo Hosted Projects | Reporter: | n0t3p4d.opensource |
| Component: | OpenRC | Assignee: | OpenRC Team <openrc> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | blueness, dwfreed, n0t3p4d.opensource |
| Priority: | Normal | ||
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Package list: | Runtime testing required: | --- | |
| Bug Depends on: | |||
| Bug Blocks: | 520144 | ||
| Attachments: |
gdb backtrace
vdr init script build.log with replaced queue.h 0001-fix-double-freee.patch 0001-fix-double-free.patch |
||
|
Description
n0t3p4d.opensource
2014-12-04 00:08:43 UTC
Created attachment 390888 [details]
gdb backtrace
Created attachment 390890 [details]
vdr init script
I am unable to reproduce this. What about anyone else? I've successfully reproduced it on my main machine. Just starting vdr and running openrc doesn't suffice. I had to add it to the default runlevenl first. The first call to openrc starts it successfully, the second one leads to the same crash. Calling openrc when vdr is started but does not belong to any runlevel does not lead to a crash. Instead, the service gets stopped successfully. In order to get a working VDR installation, emerging media-plugins/vdr-dummydevice and enabling it with eselect vdr-plugin enable should suffice. Does this happen with all versions of OpenRC in the tree? If not, which is the first one it happens with? =sys-apps/openrc-0.13.1 works, =sys-apps/openrc-0.13.2 crashes. Could you paste all of the files in /run/openrc/daemons/vdr after replicating the issue into this bug? There is generally only one file named 001, but if there are multiple, paste them all, and mention which one is which. 001: exec=/usr/bin/vdr argv_0=/usr/bin/vdr argv_1=-u argv_2=vdr argv_3=--watchdog=60 argv_4=--epgfile=/var/vdr/epg.data argv_5=--cachedir=/dev/shm argv_6=--log=3 argv_7=--video=/var/vdr/video argv_8=--record=/usr/share/vdr/bin/vdrrecord-gate.sh argv_9=-D argv_10=1 argv_11=-D argv_12=2 argv_13=--shutdown=/usr/share/vdr/bin/vdrshutdown-gate.sh argv_14=--plugin=softhddevice -d :0.0 -v vdpau -a hw:0,0 -p hw:0,1 -f -s -w alsa-no-close-open argv_15=--plugin=femon argv_16=--plugin=skinnopacity --iconpath=/usr/share/vdr/plugins/skinnopacity/icons/ --logopath=/usr/share/channel-logos/dvbviewer/ --epgimages=/dev/shm/ argv_17=--plugin=tvguide --logodir=/usr/share/channel-logos/dvbviewer/ --epgimages=/dev/shm/ argv_18=--plugin=vnsiserver argv_19=--daemon pidfile= 002: exec=/usr/sbin/vdr-watchdogd argv_0=vdr-watchdogd pidfile=/var/run/vdrwatchdog.pid Since there were so few commits, this is porbably due to netbsd's queue.h vs glibc's queue.h although I haven't looked carefully why. @reporter, let's settle this question. If its not too much trouble, rebuild openrc using /usr/include/sys/queue.h replacing openrc's ./src/includes/queue.h. Then test. If this doesn't double free, then its in the implementation details of TAILQ_* macros. My recommendation would be to fix up openrc's assumptions and keep the netbsd version since it is better maintained. Created attachment 391796 [details]
build.log with replaced queue.h
Replacing queue.h leads to compile errors. See the attached build.log.
(In reply to n0t3p4d.opensource from comment #10) > Created attachment 391796 [details] > build.log with replaced queue.h > > Replacing queue.h leads to compile errors. See the attached build.log. Yep confirmed, because there were *two* changes. You also need to revert commit f9d1742a909f41d8a7994bb58be630eedfc0f574. It then does compile. Please try that and see if the double free goes away. That wasn't the cause. I ran git bisect and can confirm that it was introduced by commit f9acd65497c6e561fbf5420386a99d681fede859. This issue was solved in be952bebb3647069fb93b9791ee3439698f697ca and a new openrc was released just after that.. I wonder why broken version is still in tree :/ As far as I can tell, it's included in at least openrc-0.13.6. Nevertheless, I still get the crash with current git master so apparently there's still some work to do :) (In reply to n0t3p4d.opensource from comment #12) > That wasn't the cause. I ran git bisect and can confirm that it was > introduced by commit f9acd65497c6e561fbf5420386a99d681fede859. Thanks, this makes sense now. We just missed this double free before. Can we tell which variable is being double free'd? Created attachment 393644 [details, diff]
0001-fix-double-freee.patch
Can you please test with this patch applied?
This makes the algorithm more similar to the way it was before the commit
you cited; the spidfile variable is now only used as temporary storage.
Thanks much,
William
Created attachment 393656 [details, diff]
0001-fix-double-free.patch
This is a cleaned up version of the previous patch that fixes
indentations. I am putting it here because it may be easier to read.
I've applied the patch to openrc-0.13.6 and can confirm that it works as expected. Thanks a lot for fixing this! After some rubber duck debugging, I found the cause. The first run through the loop reads the 002 file, and sets pidfile to /var/run/vdrwatchdog.pid. This reference is replicated to spidfile and then the chroot check is done. After that, the pidfile is read, and then spidfile is freed and NULLed. However, pidfile is *not* NULLed, and so in the second loop around, pidfile is still pointing to a freed memory location, and gets freed again. Because glibc doesn't do a heap check on every free (because it's expensive), the double free check doesn't trigger until much later, producing a sort of red herring making us think it's the stringlist breaking. This is fixed in commit 7447883 and will be in OpenRC-0.14 and 0.13.7. Thanks for the report. |
