From 9055f5baf37e379d5e3b556bff1741565442723e Mon Sep 17 00:00:00 2001 From: Arun Raghavan Date: Thu, 23 Jul 2020 19:42:47 -0400 Subject: stream-restore,device-restore: Avoid unaligned access MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Newer GCC warns us that the channel_map and volume in legacy entries are accessed via pointers, and these might be unaligned as the legacy entry is a packed structure. For this reason, we read out those values into local variables before accessing them as pointers. The warnings are: [146/433] Compiling C object src/modules/module-device-restore.so.p/module-device-restore.c.o ../src/modules/module-device-restore.c: In function ‘legacy_entry_read’: ../src/modules/module-device-restore.c:554:51: warning: taking address of packed member of ‘struct legacy_entry’ may result in an unaligned pointer value [-Waddress-of-packed-member] 554 | if (le->volume_valid && !pa_channel_map_valid(&le->channel_map)) { | ^~~~~~~~~~~~~~~~ ../src/modules/module-device-restore.c:559:48: warning: taking address of packed member of ‘struct legacy_entry’ may result in an unaligned pointer value [-Waddress-of-packed-member] 559 | if (le->volume_valid && (!pa_cvolume_valid(&le->volume) || !pa_cvolume_compatible_with_channel_map(&le->volume, &le->channel_map))) { | ^~~~~~~~~~~ ../src/modules/module-device-restore.c:559:104: warning: taking address of packed member of ‘struct legacy_entry’ may result in an unaligned pointer value [-Waddress-of-packed-member] 559 | if (le->volume_valid && (!pa_cvolume_valid(&le->volume) || !pa_cvolume_compatible_with_channel_map(&le->volume, &le->channel_map))) { | ^~~~~~~~~~~ ../src/modules/module-device-restore.c:559:117: warning: taking address of packed member of ‘struct legacy_entry’ may result in an unaligned pointer value [-Waddress-of-packed-member] 559 | if (le->volume_valid && (!pa_cvolume_valid(&le->volume) || !pa_cvolume_compatible_with_channel_map(&le->volume, &le->channel_map))) { | ^~~~~~~~~~~~~~~~ [211/433] Compiling C object src/modules/module-stream-restore.so.p/module-stream-restore.c.o ../src/modules/module-stream-restore.c: In function ‘legacy_entry_read’: ../src/modules/module-stream-restore.c:1076:51: warning: taking address of packed member of ‘struct legacy_entry’ may result in an unaligned pointer value [-Waddress-of-packed-member] 1076 | if (le->volume_valid && !pa_channel_map_valid(&le->channel_map)) { | ^~~~~~~~~~~~~~~~ ../src/modules/module-stream-restore.c:1081:48: warning: taking address of packed member of ‘struct legacy_entry’ may result in an unaligned pointer value [-Waddress-of-packed-member] 1081 | if (le->volume_valid && (!pa_cvolume_valid(&le->volume) || !pa_cvolume_compatible_with_channel_map(&le->volume, &le->channel_map))) { | ^~~~~~~~~~~ ../src/modules/module-stream-restore.c:1081:104: warning: taking address of packed member of ‘struct legacy_entry’ may result in an unaligned pointer value [-Waddress-of-packed-member] 1081 | if (le->volume_valid && (!pa_cvolume_valid(&le->volume) || !pa_cvolume_compatible_with_channel_map(&le->volume, &le->channel_map))) { | ^~~~~~~~~~~ ../src/modules/module-stream-restore.c:1081:117: warning: taking address of packed member of ‘struct legacy_entry’ may result in an unaligned pointer value [-Waddress-of-packed-member] 1081 | if (le->volume_valid && (!pa_cvolume_valid(&le->volume) || !pa_cvolume_compatible_with_channel_map(&le->volume, &le->channel_map))) { | --- src/modules/module-device-restore.c | 11 +++++++++-- src/modules/module-stream-restore.c | 11 +++++++++-- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/src/modules/module-device-restore.c b/src/modules/module-device-restore.c index d15d9ffa3..684c8360d 100644 --- a/src/modules/module-device-restore.c +++ b/src/modules/module-device-restore.c @@ -528,6 +528,8 @@ static bool legacy_entry_read(struct userdata *u, pa_datum *data, struct entry * char port[PA_NAME_MAX]; } PA_GCC_PACKED; struct legacy_entry *le; + pa_channel_map channel_map; + pa_cvolume volume; pa_assert(u); pa_assert(data); @@ -551,12 +553,17 @@ static bool legacy_entry_read(struct userdata *u, pa_datum *data, struct entry * return false; } - if (le->volume_valid && !pa_channel_map_valid(&le->channel_map)) { + /* Read these out before accessing contents via pointers as struct legacy_entry may not be adequately aligned for these + * members to be accessed directly */ + channel_map = le->channel_map; + volume = le->volume; + + if (le->volume_valid && !pa_channel_map_valid(&channel_map)) { pa_log_warn("Invalid channel map."); return false; } - if (le->volume_valid && (!pa_cvolume_valid(&le->volume) || !pa_cvolume_compatible_with_channel_map(&le->volume, &le->channel_map))) { + if (le->volume_valid && (!pa_cvolume_valid(&volume) || !pa_cvolume_compatible_with_channel_map(&volume, &channel_map))) { pa_log_warn("Volume and channel map don't match."); return false; } diff --git a/src/modules/module-stream-restore.c b/src/modules/module-stream-restore.c index 7144a664b..d26543bde 100644 --- a/src/modules/module-stream-restore.c +++ b/src/modules/module-stream-restore.c @@ -1029,6 +1029,8 @@ static struct entry *legacy_entry_read(struct userdata *u, const char *name) { pa_datum data; struct legacy_entry *le; struct entry *e; + pa_channel_map channel_map; + pa_cvolume volume; pa_assert(u); pa_assert(name); @@ -1073,12 +1075,17 @@ static struct entry *legacy_entry_read(struct userdata *u, const char *name) { goto fail; } - if (le->volume_valid && !pa_channel_map_valid(&le->channel_map)) { + /* Read these out before accessing contents via pointers as struct legacy_entry may not be adequately aligned for these + * members to be accessed directly */ + channel_map = le->channel_map; + volume = le->volume; + + if (le->volume_valid && !pa_channel_map_valid(&channel_map)) { pa_log_warn("Invalid channel map stored in database for legacy stream"); goto fail; } - if (le->volume_valid && (!pa_cvolume_valid(&le->volume) || !pa_cvolume_compatible_with_channel_map(&le->volume, &le->channel_map))) { + if (le->volume_valid && (!pa_cvolume_valid(&volume) || !pa_cvolume_compatible_with_channel_map(&volume, &channel_map))) { pa_log_warn("Invalid volume stored in database for legacy stream"); goto fail; } -- cgit v1.2.1