From 5afe721aeb241bd21b832994ec2f2b7d8ff84766 Mon Sep 17 00:00:00 2001 From: Andre Noll Date: Mon, 26 Dec 2016 23:50:34 +0100 Subject: [PATCH] load_chunk_table(): Don't trust afhi->chunks_total. Both callers pass a buffer to load_chunk_table() to initialize afhi->chunk_table. The function does not check the size of the passed buffer but relies on afhi->chunks_total to tell how many values to copy from the buffer. In other words, the buffer size gives an upper bound for the number of chunks in the chunk table, but this bould is not checked. This patch makes the code more robust by adding a suitable check. To this aim, load_chunk_table() is changed to receive a pointer to an osl object (which contains the buffer size) rather than only the buffer. One caller, load_afd(), initializes the chunk table from a shared memory area buffer. It does not know or care about the buffer size so far. We introduce the the new IPC helper shm_size() to let it tell the size of the area and pass it to load_chunk_table(). --- aft.c | 20 ++++++++++++++------ ipc.c | 21 +++++++++++++++++++++ ipc.h | 1 + 3 files changed, 36 insertions(+), 6 deletions(-) diff --git a/aft.c b/aft.c index 8b9d85b5..1756ec04 100644 --- a/aft.c +++ b/aft.c @@ -437,13 +437,13 @@ static uint32_t save_chunk_table(struct afh_info *afhi, char *buf) return max; } -static void load_chunk_table(struct afh_info *afhi, char *buf) +static void load_chunk_table(struct afh_info *afhi, const struct osl_object *ct) { int i; afhi->chunk_table = para_malloc(sizeof_chunk_table(afhi)); - for (i = 0; i <= afhi->chunks_total; i++) - afhi->chunk_table[i] = read_u32(buf + 4 * i); + for (i = 0; i <= afhi->chunks_total && i * 4 + 3 < ct->size; i++) + afhi->chunk_table[i] = read_u32(ct->data + 4 * i); } /** @@ -663,14 +663,22 @@ int load_afd(int shmid, struct audio_file_data *afd) { void *shm_afd; int ret; + struct osl_object obj; ret = shm_attach(shmid, ATTACH_RO, &shm_afd); if (ret < 0) return ret; + ret = shm_size(shmid, &obj.size); + if (ret < 0) + goto detach; *afd = *(struct audio_file_data *)shm_afd; - load_chunk_table(&afd->afhi, shm_afd + sizeof(*afd)); + obj.data = shm_afd + sizeof(*afd); + obj.size -= sizeof(*afd); + load_chunk_table(&afd->afhi, &obj); + ret = 1; +detach: shm_detach(shm_afd); - return 1; + return ret; } static int get_local_time(uint64_t *seconds, char *buf, size_t size, @@ -1076,7 +1084,7 @@ again: save_afsi(&new_afsi, &afsi_obj); /* in-place update */ afd->audio_format_id = d->afsi.audio_format_id; - load_chunk_table(&afd->afhi, chunk_table_obj.data); + load_chunk_table(&afd->afhi, &chunk_table_obj); aced.aft_row = current_aft_row; aced.old_afsi = &d->afsi; /* diff --git a/ipc.c b/ipc.c index 9488224a..d7f515df 100644 --- a/ipc.c +++ b/ipc.c @@ -161,6 +161,27 @@ int shm_attach(int id, enum shm_attach_mode mode, void **result) return *result == (void *) -1? -ERRNO_TO_PARA_ERROR(errno) : 1; } +/** + * Get the size of a shared memory segment. + * + * \param id The shared memory segment identifier. + * \param result Size in bytes is returned here, zero on errors. + * + * \return Standard. + * + * \sa shmctl(2). + */ +int shm_size(int id, size_t *result) +{ + struct shmid_ds ds; /* data structure */ + + *result = 0; + if (shmctl(id, IPC_STAT, &ds) < 0) + return -ERRNO_TO_PARA_ERROR(errno); + *result = ds.shm_segsz; + return 1; +} + /** * Detach a shared memory segment. * diff --git a/ipc.h b/ipc.h index c8d31c0c..c4437104 100644 --- a/ipc.h +++ b/ipc.h @@ -11,4 +11,5 @@ int shm_new(size_t size); int shm_attach(int id, enum shm_attach_mode mode, void **result); int shm_detach(void *addr); int shm_destroy(int id); +int shm_size(int id, size_t *result); size_t shm_get_shmmax(void); -- 2.39.5