Bug 441960 - PA isn't typesafe in its use of SHM when taking into account wordsize
Summary: PA isn't typesafe in its use of SHM when taking into account wordsize
Keywords:
Status: CLOSED UPSTREAM
Alias: None
Product: Fedora
Classification: Fedora
Component: pulseaudio
Version: rawhide
Hardware: All
OS: Linux
low
low
Target Milestone: ---
Assignee: Lennart Poettering
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2008-04-10 20:53 UTC by Bill Nottingham
Modified: 2014-03-17 03:13 UTC (History)
5 users (show)

Fixed In Version:
Clone Of:
Environment:
Last Closed: 2008-04-16 14:47:05 UTC
Type: ---
Embargoed:


Attachments (Terms of Use)
patch - use uint64_t everywhere, so the segments can match across wordsizes (5.78 KB, patch)
2008-04-10 20:53 UTC, Bill Nottingham
no flags Details | Diff

Description Bill Nottingham 2008-04-10 20:53:28 UTC
Description of problem:

Basically, flash opens the shm segment, tries to use it, and gets kicked out.

Thanks to Peter Jones for the investigation and the patch. It doesn't fix flash
completely, but would love some comment on whether or not it makes sense, and if
there are similar places that need fixed.

Version-Release number of selected component (if applicable):

pulseaudio-0.9.10-1.fc9.x86_64

Comment 1 Bill Nottingham 2008-04-10 20:53:28 UTC
Created attachment 302073 [details]
patch - use uint64_t everywhere, so the segments can match across wordsizes

Comment 2 Lennart Poettering 2008-04-10 21:01:31 UTC
Uh, the second part of the patch doesn't make sense and is also superfluous as
far as I can see. The first part does make sense.

multiarch is our downfall.

Comment 3 Peter Jones 2008-04-10 21:20:46 UTC
The second part still seems necessary to me -- without it, on x86_64 pa_atomic_t
winds up being 64-bits, and on i386 it's 32-bits.  This means if an i386 process
checks the pid field of a structure that was written by an x86_64 process, it's
actually looking at the (msb, I think?) end of marker, not at pid.

Comment 4 Peter Jones 2008-04-10 21:21:10 UTC
(though tbf, I don't really see why they need to use pa_atomic_t for those two
fields, but I haven't looked especially hard)

Comment 5 Lennart Poettering 2008-04-10 21:24:59 UTC
First, you are modifying pa_atomic_ptr_t and not pa_atomic_t. 

Then, extending atomic types to 64bit for all archs just like that won't work.
Because only the fewest archs can do atomic stuff with 64bit entities.

Then, pa_atomic_t is basically defined as int. Which is the same for x86 and x84-64.

Comment 6 Peter Jones 2008-04-10 21:58:58 UTC
Oh, blah.  Yeah, clearly I'm off the deep end on that bit.  Thanks for noticing.

Comment 7 Bill Nottingham 2008-04-11 01:17:32 UTC
For the shm_marker, if it's supposed to be portable, why not just do char
reserved[xx] with packed?

Comment 8 Lennart Poettering 2008-04-16 14:47:05 UTC
I changed it to uint64_t now upstream. Should be safe.

Also, this marker struct is not very important. It is just there to detect
leftover SHM areas so that we can purge them when we encounter them. So what
happens right now is that 64bit PA can only clean up leftover shm areas from
earlier 64bit sessions. And 32bit PA can only clean ip leftover shm areas from
earlier 32bit sessions. This isn't much of a limitation I would say.

I will thus not fix this in Fedora for now. Eventually the upstream fix will be
pulled into Fedora and then we'll able to cleanup both SHM types from both types
of processes.


Note You need to log in before you can comment on or make changes to this bug.