2009-01-27 20:51:21

by David Mansfield

[permalink] [raw]
Subject: bug in audio/pcm_bluetooth.c causing memory corruption

I've been debugging a problem using pulseaudio on top of an alsa
bluetooth device for a week or so and I've found the cause of the
problem (which manifests as a pulseaudio daemon segfault).

The bug is an line audio/pcm_bluetooth.c:802:


if (pfds[1].revents & (POLLERR | POLLHUP | POLLNVAL))
io->state = SND_PCM_STATE_DISCONNECTED;

revents[0] = (pfds[0].revents & ~POLLIN) | POLLOUT;
-->here revents[1] = (pfds[1].revents & ~POLLIN);

return 0;

The 'unsigned short *revents' argument is NOT an array of shorts, but in
fact a pointer to a single short. The assignment to revents[1] trashes
memory.

My guess is that all the flags should be combined into revents[0] (or
*revents, as that would be more semantically correct), but I'm not
really sure what the exact fix should be.

See this post by Jaroslav Kysela on the method
snd_pcm_poll_descriptors_revents, which ultimately ends up in the above
code:

http://osdir.com/ml/linux.alsa.devel/2002-07/msg00258.html

Thanks,
David




2009-02-02 15:24:33

by David Mansfield

[permalink] [raw]
Subject: Re: bug in audio/pcm_bluetooth.c causing memory corruption

On Sun, 2009-02-01 at 17:59 +0100, Marcel Holtmann wrote:
> Hi David,
>
> > I've been debugging a problem using pulseaudio on top of an alsa
> > bluetooth device for a week or so and I've found the cause of the
> > problem (which manifests as a pulseaudio daemon segfault).
> >
> > The bug is an line audio/pcm_bluetooth.c:802:
> >
> >
> > if (pfds[1].revents & (POLLERR | POLLHUP | POLLNVAL))
> > io->state = SND_PCM_STATE_DISCONNECTED;
> >
> > revents[0] = (pfds[0].revents & ~POLLIN) | POLLOUT;
> > -->here revents[1] = (pfds[1].revents & ~POLLIN);
> >
> > return 0;
> >
> > The 'unsigned short *revents' argument is NOT an array of shorts, but in
> > fact a pointer to a single short. The assignment to revents[1] trashes
> > memory.
> >
> > My guess is that all the flags should be combined into revents[0] (or
> > *revents, as that would be more semantically correct), but I'm not
> > really sure what the exact fix should be.
> >
> > See this post by Jaroslav Kysela on the method
> > snd_pcm_poll_descriptors_revents, which ultimately ends up in the above
> > code:
> >
> > http://osdir.com/ml/linux.alsa.devel/2002-07/msg00258.html
>
> this is a real problem since it seem audicious seems to break if we not
> using revents[1]. So I have no clue what's the right fix is here. The
> problem seems to be more complex. Seems we need an ALSA expert to fix
> this for us.
>

Well, I should think it's important to get confirmation first of what
the correct semantics of the function are. I looked at the docs and
they're vague as hell. Should I open a bug for this issue so we can
track the issue? It's DEFINITELY causing memory corruption and
segmentation fault on x86_64 with pulseaudio via module-alsa-sink (on
Fedora 10).

Thanks,
David

2009-02-01 16:59:44

by Marcel Holtmann

[permalink] [raw]
Subject: Re: bug in audio/pcm_bluetooth.c causing memory corruption

Hi David,

> I've been debugging a problem using pulseaudio on top of an alsa
> bluetooth device for a week or so and I've found the cause of the
> problem (which manifests as a pulseaudio daemon segfault).
>
> The bug is an line audio/pcm_bluetooth.c:802:
>
>
> if (pfds[1].revents & (POLLERR | POLLHUP | POLLNVAL))
> io->state = SND_PCM_STATE_DISCONNECTED;
>
> revents[0] = (pfds[0].revents & ~POLLIN) | POLLOUT;
> -->here revents[1] = (pfds[1].revents & ~POLLIN);
>
> return 0;
>
> The 'unsigned short *revents' argument is NOT an array of shorts, but in
> fact a pointer to a single short. The assignment to revents[1] trashes
> memory.
>
> My guess is that all the flags should be combined into revents[0] (or
> *revents, as that would be more semantically correct), but I'm not
> really sure what the exact fix should be.
>
> See this post by Jaroslav Kysela on the method
> snd_pcm_poll_descriptors_revents, which ultimately ends up in the above
> code:
>
> http://osdir.com/ml/linux.alsa.devel/2002-07/msg00258.html

this is a real problem since it seem audicious seems to break if we not
using revents[1]. So I have no clue what's the right fix is here. The
problem seems to be more complex. Seems we need an ALSA expert to fix
this for us.

Regards

Marcel