Bug 65474

Summary:	libpulsecommon.so unaligned memory access issue
Product:	PulseAudio	Reporter:	Yupeng Chang <changyp6>
Component:	core	Assignee:	pulseaudio-bugs
Status:	RESOLVED MOVED	QA Contact:	pulseaudio-bugs
Severity:	major
Priority:	high	CC:	changyp6, lennart
Version:	unspecified	Keywords:	love
Hardware:	ARM
OS:	Linux (All)
Whiteboard:
i915 platform:		i915 features:

Description Yupeng Chang 2013-06-06 18:31:53 UTC

I cross compiled PulseAudio 4.0 using codesourcery 2011.09 to ARM platform with the following CFLAGS
-march=armv6k -mtune=arm1136j-s -msoft-float -O3 -fPIC

pulseaudio daemon runs perfectly without any problem, but when I start to run client application like pactl info, the connection always fail with the following error messages:

# pactl  info
I: [pulseaudio][pulsecore/client.c:80 pa_client_new()] Created 0 "Native client (UNIX socket client)"
D: [pulseaudio][pulsecore/protocol-dbus.c:773 pa_dbus_protocol_add_interface()] Interface org.PulseAudio.Core1.Client added for object /org/pulseaudio/core1/client0
D: [pulseaudio][pulsecore/protocol-native.c:2599 command_auth()] Protocol version: remote 28, local 28
I: [pulseaudio][pulsecore/protocol-native.c:2632 command_auth()] Got credentials: uid=0 gid=0 success=1
D: [pulseaudio][pulsecore/protocol-native.c:2662 command_auth()] SHM possible: yes
D: [pulseaudio][pulsecore/protocol-native.c:2680 command_auth()] Negotiated SHM: yes
E: [pulseaudio][pulsecore/protocol-native.c:1939 protocol_error()] protocol error, kicking client
I: [pulseaudio][pulsecore/client.c:102 pa_client_free()] Freed 0 "Native client (UNIX socket client)"
D: [pulseaudio][pulsecore/protocol-dbus.c:835 pa_dbus_protocol_remove_interface()] Interface org.PulseAudio.Core1.Client removed from object /org/pulseaudio/core1/client0
Connection failure: Connection terminated

I did some digging on this error, I found connection is terminated on function: command_set_client_name(), and the root failure is in function: pa_tagstruct_get_arbitrary(), on this check "if (t->data[t->rindex] != PA_TAG_ARBITRARY)".

Then I print out t->data[t->rindex] value to see if it's PA_TAG_ARBITRARY, what is strange is that this value is a randome value.

I go back to check the compiling log, and found the following warnings:
pulsecore/hashmap.c: In function 'remove_entry':
pulsecore/hashmap.c:94:9: warning: cast increases required alignment of target type [-Wcast-align]
pulsecore/hashmap.c: In function 'hash_scan':
pulsecore/hashmap.c:116:14: warning: cast increases required alignment of target type [-Wcast-align]
pulsecore/hashmap.c: In function 'pa_hashmap_put':
pulsecore/hashmap.c:141:22: warning: cast increases required alignment of target type [-Wcast-align]
pulsecore/hashmap.c:143:9: warning: cast increases required alignment of target type [-Wcast-align]
pulsecore/hashmap.c:144:9: warning: cast increases required alignment of target type [-Wcast-align]
pulsecore/hashmap.c:145:5: warning: cast increases required alignment of target type [-Wcast-align]

These files are included in libpulsecommon.so

Then I have to add -mno-unaligned-access to CLFAGS, then re-compile pulseaudio again. I replaced the original one with newly compiled libpulsecommon.so, leaving other libraries untouched, everything goes well without any issue.

So I think this must be an unaligned memory access issue in libpulsecommon.so

Please help to fix this!!

Comment 1 Arun Raghavan 2013-11-22 10:19:24 UTC

Right, I can see where we need to fix these unaligned reads reads (the hashmap has an array of hashmap_entry just allocated at after the structure using malloc. which us just awful).

Patches welcome, and attaching a 'love' keyword since it shouldn't be terribly complicated to fix this.

Comment 2 Yupeng Chang 2013-12-03 10:59:56 UTC

Hi Arun,
The simplest way to fix this is to add another structure

struct hasmap_table {
    struct pa_hashmap hashmap;
    struct hashmap_entry* entry[NBUCKETS];
};

change
#define BY_HASH(h) ((struct hashmap_entry**) ((uint8_t*) (h) + PA_ALIGN(sizeof(pa_hashmap))))

to
#define BY_HASH(h) (((struct hashmap_table*)h)->entry)

Then, change
h = pa_xmalloc0(PA_ALIGN(sizeof(pa_hashmap)) + NBUCKETS*sizeof(struct hashmap_entry*));

to

h = pa_xmalloc0(sizeof(struct hashmap_table));

I found there are lots of files in pulseaudio have cast-align issue.

I did this modification and compiled, no warnings anymore. But I have test if this modification is OK.

If you think this modification is OK, please help test..
Thanks you!

Comment 3 Yupeng Chang 2013-12-03 11:01:13 UTC

Sorry, I haven't tested if this modification is OK or not.

Comment 4 Peter Meerwald 2014-11-17 09:01:55 UTC

is the proposed change supposed to fix the compiler warnings or the unaligned memory access?

the current code is using PA_ALIGN(), so the entries should start on an aligned address -- the memory layout won't be affected I think?

Comment 5 GitLab Migration User 2018-07-30 10:09:09 UTC

-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/pulseaudio/pulseaudio/issues/241.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.