summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorAidan MacDonald <amachronic@protonmail.com>2022-05-02 15:23:37 +0100
committerAidan MacDonald <amachronic@protonmail.com>2022-05-02 15:38:48 +0100
commit6b8c94a6e3094ab75fcfe319fa2bc100f4e329ec (patch)
tree6c6ee0a2e7f77084f721ed33ff5b8856ec43fb61
parent366f00a3d3dc6517e7dcb1bbebb887c4f795320b (diff)
downloadrockbox-6b8c94a6e3.tar.gz
rockbox-6b8c94a6e3.zip
Fix some non-portable alignment values
UBSan reports an avalanche of unaligned pointer bugs stemming from hardcoded 4-byte alignments used in certain places. Use sizeof(long) instead to align to the machine word size. Change-Id: I28e505212462c5268afa24e95df3a103ac3e2213
-rw-r--r--apps/gui/skin_engine/skin_parser.c5
-rw-r--r--apps/plugins/pictureflow/pictureflow.c34
-rw-r--r--apps/recorder/jpeg_load.c4
-rw-r--r--apps/tagcache.c9
-rw-r--r--lib/skin_parser/skin_buffer.c4
5 files changed, 19 insertions, 37 deletions
diff --git a/apps/gui/skin_engine/skin_parser.c b/apps/gui/skin_engine/skin_parser.c
index ee32c06ace..1a6861ff16 100644
--- a/apps/gui/skin_engine/skin_parser.c
+++ b/apps/gui/skin_engine/skin_parser.c
@@ -2538,8 +2538,9 @@ bool skin_data_load(enum screen_type screen, struct wps_data *wps_data,
skin_buffer = wps_buffer;
wps_buffer = (char*)buf;
}
- skin_buffer = ALIGN_UP(skin_buffer, 4); /* align on 4-byte boundary */
- buffersize -= 3;
+
+ /* align to long */
+ ALIGN_BUFFER(skin_buffer, buffersize, sizeof(long));
#ifdef HAVE_BACKDROP_IMAGE
backdrop_filename = "-";
wps_data->backdrop_id = -1;
diff --git a/apps/plugins/pictureflow/pictureflow.c b/apps/plugins/pictureflow/pictureflow.c
index 5136fc56e9..83d4bb5a59 100644
--- a/apps/plugins/pictureflow/pictureflow.c
+++ b/apps/plugins/pictureflow/pictureflow.c
@@ -1303,9 +1303,8 @@ static int build_artist_index(struct tagcache_search *tcs,
if (res < SUCCESS)
return res;
- ALIGN_BUFFER(*buf, *bufsz, 4);
-
/* finalize the artist index */
+ ALIGN_BUFFER(*buf, *bufsz, alignof(struct artist_data));
tmp_artist = (struct artist_data*)*buf;
for (i = pf_idx.artist_ct - 1; i >= 0; i--)
tmp_artist[i] = pf_idx.artist_index[-i];
@@ -1313,7 +1312,6 @@ static int build_artist_index(struct tagcache_search *tcs,
pf_idx.artist_index = tmp_artist;
/* move buf ptr to end of artist_index */
*buf = pf_idx.artist_index + pf_idx.artist_ct;
- ALIGN_BUFFER(*buf, *bufsz, 4);
if (res == SUCCESS)
{
@@ -1384,14 +1382,14 @@ static int create_album_index(void)
int i, j, last, final, retry, res;
draw_splashscreen(buf, buf_size);
- ALIGN_BUFFER(buf, buf_size, 4);
+ ALIGN_BUFFER(buf, buf_size, sizeof(long));
-/* Artists */
+ /* Artists */
res = build_artist_index(&tcs, &buf, &buf_size);
if (res < SUCCESS)
return res;
-/* Albums */
+ /* Albums */
pf_idx.album_ct = 0;
pf_idx.album_len =0;
pf_idx.album_untagged_idx = 0;
@@ -1407,7 +1405,6 @@ static int create_album_index(void)
rb->tagcache_search_finish(&tcs);
if (res < SUCCESS)
return res;
- ALIGN_BUFFER(buf, buf_size, 4);
/* Build artist list for untagged albums */
res = create_album_untagged(&tcs, &buf, &buf_size);
@@ -1415,9 +1412,8 @@ static int create_album_index(void)
if (res < SUCCESS)
return res;
- ALIGN_BUFFER(buf, buf_size, 4);
-
/* finalize the album index */
+ ALIGN_BUFFER(buf, buf_size, alignof(struct album_data));
tmp_album = (struct album_data*)buf;
for (i = pf_idx.album_ct - 1; i >= 0; i--)
tmp_album[i] = pf_idx.album_index[-i];
@@ -1425,9 +1421,8 @@ static int create_album_index(void)
pf_idx.album_index = tmp_album;
/* move buf ptr to end of album_index */
buf = pf_idx.album_index + pf_idx.album_ct;
- ALIGN_BUFFER(buf, buf_size, 4);
-/* Assign indices */
+ /* Assign indices */
draw_splashscreen(buf, buf_size);
draw_progressbar(0, pf_idx.album_ct, "Assigning Albums");
for (j = 0; j < pf_idx.album_ct; j++)
@@ -1541,7 +1536,6 @@ retry_artist_lookup:
}
}
- ALIGN_BUFFER(buf, buf_size, 4);
pf_idx.buf = buf;
pf_idx.buf_sz = buf_size;
pf_idx.artist_index = 0;
@@ -1618,7 +1612,6 @@ static int load_album_index(void){
//rb->lseek(fr, sizeof(data) + 1, SEEK_SET);
/* artist names */
- ALIGN_BUFFER(buf, buf_size, 4);
if (read2buf(fr, buf, data.artist_len) == 0)
goto failure;
@@ -1627,7 +1620,6 @@ static int load_album_index(void){
buf_size -= data.artist_len;
/* album names */
- ALIGN_BUFFER(buf, buf_size, 4);
if (read2buf(fr, buf, data.album_len) == 0)
goto failure;
@@ -1636,7 +1628,6 @@ static int load_album_index(void){
buf_size -= data.album_len;
/* index of album names */
- ALIGN_BUFFER(buf, buf_size, 4);
if (read2buf(fr, buf, album_idx_sz) == 0)
goto failure;
@@ -4286,8 +4277,6 @@ static int pictureflow_main(const char* selected_file)
init_scroll_lines();
init_reflect_table();
- ALIGN_BUFFER(pf_idx.buf, pf_idx.buf_sz, 4);
-
/*Scan will trigger when no file is found or the option was activated*/
if ((pf_cfg.cache_version != CACHE_VERSION)||(load_album_index() < 0)){
rb->splash(HZ/2,"Creating index, please wait");
@@ -4312,23 +4301,20 @@ static int pictureflow_main(const char* selected_file)
return PLUGIN_ERROR;
}
- ALIGN_BUFFER(pf_idx.buf, pf_idx.buf_sz, 4);
- number_of_slides = pf_idx.album_ct;
-
- size_t aa_bufsz = ALIGN_DOWN(pf_idx.buf_sz / 4, 0x4);
+ number_of_slides = pf_idx.album_ct;
+ size_t aa_bufsz = pf_idx.buf_sz / 4 + sizeof(long) - 1;
if (aa_bufsz < DISPLAY_WIDTH * DISPLAY_HEIGHT * sizeof(pix_t))
{
error_wait("Not enough memory for album art cache");
return PLUGIN_ERROR;
}
- pf_idx.buf_sz -= aa_bufsz;
- ALIGN_BUFFER(pf_idx.buf, pf_idx.buf_sz, 4);
+ ALIGN_BUFFER(pf_idx.buf, pf_idx.buf_sz, sizeof(long));
aa_cache.buf = (char*) pf_idx.buf;
aa_cache.buf_sz = aa_bufsz;
pf_idx.buf += aa_bufsz;
- ALIGN_BUFFER(pf_idx.buf, pf_idx.buf_sz, 4);
+ pf_idx.buf_sz -= aa_bufsz;
if (!create_empty_slide(pf_cfg.cache_version != CACHE_VERSION)) {
config_save(CACHE_REBUILD);
diff --git a/apps/recorder/jpeg_load.c b/apps/recorder/jpeg_load.c
index 5b287aff75..34d543b56e 100644
--- a/apps/recorder/jpeg_load.c
+++ b/apps/recorder/jpeg_load.c
@@ -2024,7 +2024,7 @@ int clip_jpeg_fd(int fd,
#else
struct jpeg *p_jpeg = (struct jpeg*)bm->data;
int tmp_size = maxsize;
- ALIGN_BUFFER(p_jpeg, tmp_size, sizeof(int));
+ ALIGN_BUFFER(p_jpeg, tmp_size, sizeof(long));
/* not enough memory for our struct jpeg */
if ((size_t)tmp_size < sizeof(struct jpeg))
return -1;
@@ -2133,7 +2133,7 @@ int clip_jpeg_fd(int fd,
char *buf_end = (char *)bm->data + maxsize;
maxsize = buf_end - buf_start;
#ifndef JPEG_FROM_MEM
- ALIGN_BUFFER(buf_start, maxsize, sizeof(uint32_t));
+ ALIGN_BUFFER(buf_start, maxsize, sizeof(long));
if (maxsize < (int)sizeof(struct jpeg))
return -1;
memmove(buf_start, p_jpeg, sizeof(struct jpeg));
diff --git a/apps/tagcache.c b/apps/tagcache.c
index c18380854e..b6d15e7a1f 100644
--- a/apps/tagcache.c
+++ b/apps/tagcache.c
@@ -2258,17 +2258,12 @@ static int tempbuf_sort(int fd)
while (idlist->next != NULL)
idlist = idlist->next;
+ ALIGN_BUFFER(tempbuf_pos, tempbuf_left, alignof(struct tempbuf_id_list));
tempbuf_left -= sizeof(struct tempbuf_id_list);
- if (tempbuf_left - 4 < 0)
+ if (tempbuf_left < 0)
return -1;
idlist->next = (struct tempbuf_id_list *)&tempbuf[tempbuf_pos];
- if (tempbuf_pos & 0x03)
- {
- tempbuf_pos = (tempbuf_pos & ~0x03) + 0x04;
- tempbuf_left -= 3;
- idlist->next = (struct tempbuf_id_list *)&tempbuf[tempbuf_pos];
- }
tempbuf_pos += sizeof(struct tempbuf_id_list);
idlist = idlist->next;
diff --git a/lib/skin_parser/skin_buffer.c b/lib/skin_parser/skin_buffer.c
index d18122ef20..021746ba82 100644
--- a/lib/skin_parser/skin_buffer.c
+++ b/lib/skin_parser/skin_buffer.c
@@ -80,8 +80,8 @@ void* skin_buffer_alloc(size_t size)
{
void *retval = NULL;
#endif
- /* 32-bit aligned */
- size = (size + 3) & ~3;
+ /* align to long which is enough for most types */
+ size = (size + sizeof(long) - 1) & ~(sizeof(long) - 1);
if (size > skin_buffer_freespace())
{
skin_error(MEMORY_LIMIT_EXCEEDED, NULL);