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
This commit is contained in:
Aidan MacDonald 2022-05-02 15:23:37 +01:00
parent 366f00a3d3
commit 6b8c94a6e3
5 changed files with 19 additions and 37 deletions

View file

@ -2538,8 +2538,9 @@ bool skin_data_load(enum screen_type screen, struct wps_data *wps_data,
skin_buffer = wps_buffer; skin_buffer = wps_buffer;
wps_buffer = (char*)buf; 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 #ifdef HAVE_BACKDROP_IMAGE
backdrop_filename = "-"; backdrop_filename = "-";
wps_data->backdrop_id = -1; wps_data->backdrop_id = -1;

View file

@ -1303,9 +1303,8 @@ static int build_artist_index(struct tagcache_search *tcs,
if (res < SUCCESS) if (res < SUCCESS)
return res; return res;
ALIGN_BUFFER(*buf, *bufsz, 4);
/* finalize the artist index */ /* finalize the artist index */
ALIGN_BUFFER(*buf, *bufsz, alignof(struct artist_data));
tmp_artist = (struct artist_data*)*buf; tmp_artist = (struct artist_data*)*buf;
for (i = pf_idx.artist_ct - 1; i >= 0; i--) for (i = pf_idx.artist_ct - 1; i >= 0; i--)
tmp_artist[i] = pf_idx.artist_index[-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; pf_idx.artist_index = tmp_artist;
/* move buf ptr to end of artist_index */ /* move buf ptr to end of artist_index */
*buf = pf_idx.artist_index + pf_idx.artist_ct; *buf = pf_idx.artist_index + pf_idx.artist_ct;
ALIGN_BUFFER(*buf, *bufsz, 4);
if (res == SUCCESS) if (res == SUCCESS)
{ {
@ -1384,14 +1382,14 @@ static int create_album_index(void)
int i, j, last, final, retry, res; int i, j, last, final, retry, res;
draw_splashscreen(buf, buf_size); 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); res = build_artist_index(&tcs, &buf, &buf_size);
if (res < SUCCESS) if (res < SUCCESS)
return res; return res;
/* Albums */ /* Albums */
pf_idx.album_ct = 0; pf_idx.album_ct = 0;
pf_idx.album_len =0; pf_idx.album_len =0;
pf_idx.album_untagged_idx = 0; pf_idx.album_untagged_idx = 0;
@ -1407,7 +1405,6 @@ static int create_album_index(void)
rb->tagcache_search_finish(&tcs); rb->tagcache_search_finish(&tcs);
if (res < SUCCESS) if (res < SUCCESS)
return res; return res;
ALIGN_BUFFER(buf, buf_size, 4);
/* Build artist list for untagged albums */ /* Build artist list for untagged albums */
res = create_album_untagged(&tcs, &buf, &buf_size); res = create_album_untagged(&tcs, &buf, &buf_size);
@ -1415,9 +1412,8 @@ static int create_album_index(void)
if (res < SUCCESS) if (res < SUCCESS)
return res; return res;
ALIGN_BUFFER(buf, buf_size, 4);
/* finalize the album index */ /* finalize the album index */
ALIGN_BUFFER(buf, buf_size, alignof(struct album_data));
tmp_album = (struct album_data*)buf; tmp_album = (struct album_data*)buf;
for (i = pf_idx.album_ct - 1; i >= 0; i--) for (i = pf_idx.album_ct - 1; i >= 0; i--)
tmp_album[i] = pf_idx.album_index[-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; pf_idx.album_index = tmp_album;
/* move buf ptr to end of album_index */ /* move buf ptr to end of album_index */
buf = pf_idx.album_index + pf_idx.album_ct; 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_splashscreen(buf, buf_size);
draw_progressbar(0, pf_idx.album_ct, "Assigning Albums"); draw_progressbar(0, pf_idx.album_ct, "Assigning Albums");
for (j = 0; j < pf_idx.album_ct; j++) 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 = buf;
pf_idx.buf_sz = buf_size; pf_idx.buf_sz = buf_size;
pf_idx.artist_index = 0; pf_idx.artist_index = 0;
@ -1618,7 +1612,6 @@ static int load_album_index(void){
//rb->lseek(fr, sizeof(data) + 1, SEEK_SET); //rb->lseek(fr, sizeof(data) + 1, SEEK_SET);
/* artist names */ /* artist names */
ALIGN_BUFFER(buf, buf_size, 4);
if (read2buf(fr, buf, data.artist_len) == 0) if (read2buf(fr, buf, data.artist_len) == 0)
goto failure; goto failure;
@ -1627,7 +1620,6 @@ static int load_album_index(void){
buf_size -= data.artist_len; buf_size -= data.artist_len;
/* album names */ /* album names */
ALIGN_BUFFER(buf, buf_size, 4);
if (read2buf(fr, buf, data.album_len) == 0) if (read2buf(fr, buf, data.album_len) == 0)
goto failure; goto failure;
@ -1636,7 +1628,6 @@ static int load_album_index(void){
buf_size -= data.album_len; buf_size -= data.album_len;
/* index of album names */ /* index of album names */
ALIGN_BUFFER(buf, buf_size, 4);
if (read2buf(fr, buf, album_idx_sz) == 0) if (read2buf(fr, buf, album_idx_sz) == 0)
goto failure; goto failure;
@ -4286,8 +4277,6 @@ static int pictureflow_main(const char* selected_file)
init_scroll_lines(); init_scroll_lines();
init_reflect_table(); 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*/ /*Scan will trigger when no file is found or the option was activated*/
if ((pf_cfg.cache_version != CACHE_VERSION)||(load_album_index() < 0)){ if ((pf_cfg.cache_version != CACHE_VERSION)||(load_album_index() < 0)){
rb->splash(HZ/2,"Creating index, please wait"); rb->splash(HZ/2,"Creating index, please wait");
@ -4312,23 +4301,20 @@ static int pictureflow_main(const char* selected_file)
return PLUGIN_ERROR; return PLUGIN_ERROR;
} }
ALIGN_BUFFER(pf_idx.buf, pf_idx.buf_sz, 4); number_of_slides = pf_idx.album_ct;
number_of_slides = pf_idx.album_ct;
size_t aa_bufsz = ALIGN_DOWN(pf_idx.buf_sz / 4, 0x4);
size_t aa_bufsz = pf_idx.buf_sz / 4 + sizeof(long) - 1;
if (aa_bufsz < DISPLAY_WIDTH * DISPLAY_HEIGHT * sizeof(pix_t)) if (aa_bufsz < DISPLAY_WIDTH * DISPLAY_HEIGHT * sizeof(pix_t))
{ {
error_wait("Not enough memory for album art cache"); error_wait("Not enough memory for album art cache");
return PLUGIN_ERROR; return PLUGIN_ERROR;
} }
pf_idx.buf_sz -= aa_bufsz; ALIGN_BUFFER(pf_idx.buf, pf_idx.buf_sz, sizeof(long));
ALIGN_BUFFER(pf_idx.buf, pf_idx.buf_sz, 4);
aa_cache.buf = (char*) pf_idx.buf; aa_cache.buf = (char*) pf_idx.buf;
aa_cache.buf_sz = aa_bufsz; aa_cache.buf_sz = aa_bufsz;
pf_idx.buf += 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)) { if (!create_empty_slide(pf_cfg.cache_version != CACHE_VERSION)) {
config_save(CACHE_REBUILD); config_save(CACHE_REBUILD);

View file

@ -2024,7 +2024,7 @@ int clip_jpeg_fd(int fd,
#else #else
struct jpeg *p_jpeg = (struct jpeg*)bm->data; struct jpeg *p_jpeg = (struct jpeg*)bm->data;
int tmp_size = maxsize; 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 */ /* not enough memory for our struct jpeg */
if ((size_t)tmp_size < sizeof(struct jpeg)) if ((size_t)tmp_size < sizeof(struct jpeg))
return -1; return -1;
@ -2133,7 +2133,7 @@ int clip_jpeg_fd(int fd,
char *buf_end = (char *)bm->data + maxsize; char *buf_end = (char *)bm->data + maxsize;
maxsize = buf_end - buf_start; maxsize = buf_end - buf_start;
#ifndef JPEG_FROM_MEM #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)) if (maxsize < (int)sizeof(struct jpeg))
return -1; return -1;
memmove(buf_start, p_jpeg, sizeof(struct jpeg)); memmove(buf_start, p_jpeg, sizeof(struct jpeg));

View file

@ -2258,17 +2258,12 @@ static int tempbuf_sort(int fd)
while (idlist->next != NULL) while (idlist->next != NULL)
idlist = idlist->next; idlist = idlist->next;
ALIGN_BUFFER(tempbuf_pos, tempbuf_left, alignof(struct tempbuf_id_list));
tempbuf_left -= sizeof(struct tempbuf_id_list); tempbuf_left -= sizeof(struct tempbuf_id_list);
if (tempbuf_left - 4 < 0) if (tempbuf_left < 0)
return -1; return -1;
idlist->next = (struct tempbuf_id_list *)&tempbuf[tempbuf_pos]; 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); tempbuf_pos += sizeof(struct tempbuf_id_list);
idlist = idlist->next; idlist = idlist->next;

View file

@ -80,8 +80,8 @@ void* skin_buffer_alloc(size_t size)
{ {
void *retval = NULL; void *retval = NULL;
#endif #endif
/* 32-bit aligned */ /* align to long which is enough for most types */
size = (size + 3) & ~3; size = (size + sizeof(long) - 1) & ~(sizeof(long) - 1);
if (size > skin_buffer_freespace()) if (size > skin_buffer_freespace())
{ {
skin_error(MEMORY_LIMIT_EXCEEDED, NULL); skin_error(MEMORY_LIMIT_EXCEEDED, NULL);