On 2015-10-16 17:58, Takashi Iwai wrote:
On Fri, 16 Oct 2015 14:59:38 +0200,
Takashi Iwai wrote:
On Fri, 16 Oct 2015 13:50:58 +0200,
David Henningsson wrote:
On 2015-10-16 10:35, Takashi Iwai wrote:
On Fri, 16 Oct 2015 09:16:04 +0200,
David Henningsson wrote:
(Adding pulseaudio-discuss to CC)
On 2015-10-15 16:26, Takashi Iwai wrote:
Hi David,
we got bug reports with PA 7.0 where the recent KDE crashes.
It seems that srbchannel=no works around it, so there is still
something fishy there.
The bug report is found at
http://bugzilla.opensuse.org/show_bug.cgi?id=950487
Hi Takashi and thanks for reporting.
I've tried running PA 7.0's pactl under valgrind, and it reports no
errors here. Still, looking at the one of the backtraces the value of f
is something interesting:
#6 flush (f=f@entry=0x4545454545454545) at pulsecore/fdsem.c:143
#7 0x00007fe30f378fc2 in pa_fdsem_before_poll (f=0x4545454545454545) at
pulsecore/fdsem.c:295
#8 0x00007fe30f38f697 in srbchannel_rwloop (sr=0x25bdd40) at
pulsecore/srbchannel.c:203
Does 0x4545454545454545 mean anything specific on OpenSUSE? (Like, a
magic clear value or something?)
I don't think it's openSUSE specific. It's likely the guard put by
either gcc or glibc.
FWIW, we pass the default optimization flags like:
CFLAGS=-fmessage-length=0 -grecord-gcc-switches -O2 -Wall \
-D_FORTIFY_SOURCE=2 -fstack-protector -funwind-tables \
-fasynchronous-unwind-tables -g -fPIE
The problem was reported from both gcc-4.8 and gcc-5.x systems, so the
gcc version is likely irrelevant.
Also, are there any distro patches to OpenSUSE and if so, where can I
find them?
No, there is no patches apparently relevant with this. Actually there are
three patches, one is to check an additional environment check in
start-pulseaudio-x11, another is to suppress an error log at
sockaddr_prepare(), and the last is a fix in memset() size in
echo-cancel/adrian-aec.c. But all these should be safe.
All sources, patches, build log and binaries are found in OBS, e.g. at
https://build.opensuse.org/package/show/multimedia:libs/pulseaudio
Ok, thanks.
I've been trying to analyze the backtrace.
My guess is that the srbchannel is being destroyed somehow, but I don't
see how. Any chance we can get more info from this, e g, build
pulseaudio's client library with -DDEBUG_SRBCHANNEL=1 and then get a log
like this:
PULSE_LOG=99 pactl info
...which includes the crash?
OK, I'm building a package with the debug enabled and will ask
reporters to test with it.
Attached below.
Thanks.
Freeing srbchannel
In rw loop from srbchannel, after callback, count = -1680245669
Errore di segmentazione
That's the culprit, after freeing srbchannel it should say "Aborting
read loop from srbchannel" instead of "In rw loop from srbchannel".
I believe that I've fixed it - could you try the attached patch and
verify that it fixes the problem?
(I think when I'm running it under valgrind the timing changes somehow
so that the bug does not occur, that's why I didn't find it myself...)
--
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic
>From 9d370181ec4bc1e252b54dd0e7bb52016f01b238 Mon Sep 17 00:00:00 2001
From: David Henningsson <[email protected]>
Date: Fri, 16 Oct 2015 22:12:32 +0200
Subject: [PATCH] pstream: Fix use-after-free in srb_callback
We need to guard the pstream with an extra ref to ensure
it is not destroyed at the time we check whether or not the
srbchannel is destroyed.
Reported-by: Takashi Iwai <[email protected]>
BugLink: http://bugzilla.opensuse.org/show_bug.cgi?id=950487
Signed-off-by: David Henningsson <[email protected]>
---
src/pulsecore/pstream.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/src/pulsecore/pstream.c b/src/pulsecore/pstream.c
index 8c14fbb..98a8382 100644
--- a/src/pulsecore/pstream.c
+++ b/src/pulsecore/pstream.c
@@ -216,14 +216,23 @@ fail:
}
static bool srb_callback(pa_srbchannel *srb, void *userdata) {
+ bool b;
pa_pstream *p = userdata;
pa_assert(p);
pa_assert(PA_REFCNT_VALUE(p) > 0);
pa_assert(p->srb == srb);
+ pa_pstream_ref(p);
+
do_pstream_read_write(p);
- return p->srb != NULL;
+
+ /* If either pstream or the srb is going away, return false.
+ We need to check this before p is destroyed. */
+ b = (PA_REFCNT_VALUE(p) > 1) && (p->srb == srb);
+ pa_pstream_unref(p);
+
+ return b;
}
static void io_callback(pa_iochannel*io, void *userdata) {
--
1.9.1
_______________________________________________
pulseaudio-discuss mailing list
[email protected]
http://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss
