stream-restore,device-restore: Avoid unaligned access 12/243512/1 accepted/tizen/unified/20200908.130450 submit/tizen/20200908.035839 submit/tizen/20200908.061751
authorArun Raghavan <arun@asymptotic.io>
Thu, 23 Jul 2020 23:42:47 +0000 (19:42 -0400)
committerJaechul Lee <jcsing.lee@samsung.com>
Tue, 8 Sep 2020 00:28:10 +0000 (09:28 +0900)
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))) {
      |

[jcsing.lee: cherry-pick from mainstream commit merge_requests/333]
Signed-off-by: Jaechul Lee <jcsing.lee@samsung.com>
Change-Id: I37a2ac9e0a352457a1fd882862f9186715d1a253

src/modules/module-device-restore.c
src/modules/module-stream-restore.c

index d15d9ff..684c836 100644 (file)
@@ -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;
     }
index cbef478..e4f0b9a 100644 (file)
@@ -1037,6 +1037,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);
@@ -1081,12 +1083,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;
     }