From f680d83cec4c2f9c3caa6067f805a45c06f91657 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Mon, 21 Sep 2015 16:44:38 -0400 Subject: [PATCH] Fix: don't dereference NULL pointers Detected by scan-build. Signed-off-by: Mathieu Desnoyers --- liblttng-ust-comm/lttng-ust-comm.c | 2 + liblttng-ust-ctl/ustctl.c | 14 ++- liblttng-ust/lttng-ring-buffer-client.h | 18 ++++ .../lttng-ring-buffer-metadata-client.h | 6 ++ libringbuffer/ring_buffer_backend.c | 101 ++++++++++++++---- libringbuffer/ring_buffer_frontend.c | 17 ++- 6 files changed, 132 insertions(+), 26 deletions(-) diff --git a/liblttng-ust-comm/lttng-ust-comm.c b/liblttng-ust-comm/lttng-ust-comm.c index 6edb395f..1e9972d2 100644 --- a/liblttng-ust-comm/lttng-ust-comm.c +++ b/liblttng-ust-comm/lttng-ust-comm.c @@ -377,6 +377,8 @@ ssize_t ustcomm_send_fds_unix_sock(int sock, int *fds, size_t nb_fd) msg.msg_controllen = CMSG_LEN(sizeof_fds); cmptr = CMSG_FIRSTHDR(&msg); + if (!cmptr) + return -EINVAL; cmptr->cmsg_level = SOL_SOCKET; cmptr->cmsg_type = SCM_RIGHTS; cmptr->cmsg_len = CMSG_LEN(sizeof_fds); diff --git a/liblttng-ust-ctl/ustctl.c b/liblttng-ust-ctl/ustctl.c index b44d9e34..90e3094e 100644 --- a/liblttng-ust-ctl/ustctl.c +++ b/liblttng-ust-ctl/ustctl.c @@ -1334,6 +1334,8 @@ int ustctl_get_mmap_read_offset(struct ustctl_consumer_stream *stream, unsigned long sb_bindex; struct lttng_ust_lib_ring_buffer *buf; struct ustctl_consumer_channel *consumer_chan; + struct lttng_ust_lib_ring_buffer_backend_pages_shmp *barray_idx; + struct lttng_ust_lib_ring_buffer_backend_pages *pages; if (!stream) return -EINVAL; @@ -1344,8 +1346,14 @@ int ustctl_get_mmap_read_offset(struct ustctl_consumer_stream *stream, return -EINVAL; sb_bindex = subbuffer_id_get_index(&chan->backend.config, buf->backend.buf_rsb.id); - *off = shmp(consumer_chan->chan->handle, - shmp_index(consumer_chan->chan->handle, buf->backend.array, sb_bindex)->shmp)->mmap_offset; + barray_idx = shmp_index(consumer_chan->chan->handle, buf->backend.array, + sb_bindex); + if (!barray_idx) + return -EINVAL; + pages = shmp(consumer_chan->chan->handle, barray_idx->shmp); + if (!pages) + return -EINVAL; + *off = pages->mmap_offset; return 0; } @@ -1511,6 +1519,8 @@ struct lttng_ust_client_lib_ring_buffer_client_cb *get_client_cb( struct lttng_ust_client_lib_ring_buffer_client_cb *client_cb; chan = shmp(handle, buf->backend.chan); + if (!chan) + return NULL; config = &chan->backend.config; if (!config->cb_ptr) return NULL; diff --git a/liblttng-ust/lttng-ring-buffer-client.h b/liblttng-ust/lttng-ring-buffer-client.h index 96aeb1e5..7dfb144d 100644 --- a/liblttng-ust/lttng-ring-buffer-client.h +++ b/liblttng-ust/lttng-ring-buffer-client.h @@ -339,6 +339,9 @@ static void client_buffer_begin(struct lttng_ust_lib_ring_buffer *buf, uint64_t handle); struct lttng_channel *lttng_chan = channel_get_private(chan); + assert(header); + if (!header) + return; header->magic = CTF_MAGIC_NUMBER; memcpy(header->uuid, lttng_chan->uuid, sizeof(lttng_chan->uuid)); header->stream_id = lttng_chan->id; @@ -366,6 +369,9 @@ static void client_buffer_end(struct lttng_ust_lib_ring_buffer *buf, uint64_t ts handle); unsigned long records_lost = 0; + assert(header); + if (!header) + return; header->ctx.timestamp_end = tsc; header->ctx.content_size = (uint64_t) data_size * CHAR_BIT; /* in bits */ @@ -401,6 +407,8 @@ static int client_timestamp_begin(struct lttng_ust_lib_ring_buffer *buf, struct packet_header *header; header = client_packet_header(buf, handle); + if (!header) + return -1; *timestamp_begin = header->ctx.timestamp_begin; return 0; } @@ -412,6 +420,8 @@ static int client_timestamp_end(struct lttng_ust_lib_ring_buffer *buf, struct packet_header *header; header = client_packet_header(buf, handle); + if (!header) + return -1; *timestamp_end = header->ctx.timestamp_end; return 0; } @@ -423,6 +433,8 @@ static int client_events_discarded(struct lttng_ust_lib_ring_buffer *buf, struct packet_header *header; header = client_packet_header(buf, handle); + if (!header) + return -1; *events_discarded = header->ctx.events_discarded; return 0; } @@ -434,6 +446,8 @@ static int client_content_size(struct lttng_ust_lib_ring_buffer *buf, struct packet_header *header; header = client_packet_header(buf, handle); + if (!header) + return -1; *content_size = header->ctx.content_size; return 0; } @@ -445,6 +459,8 @@ static int client_packet_size(struct lttng_ust_lib_ring_buffer *buf, struct packet_header *header; header = client_packet_header(buf, handle); + if (!header) + return -1; *packet_size = header->ctx.packet_size; return 0; } @@ -456,6 +472,8 @@ static int client_stream_id(struct lttng_ust_lib_ring_buffer *buf, struct packet_header *header; header = client_packet_header(buf, handle); + if (!header) + return -1; *stream_id = header->stream_id; return 0; } diff --git a/liblttng-ust/lttng-ring-buffer-metadata-client.h b/liblttng-ust/lttng-ring-buffer-metadata-client.h index 0d2a1f83..3352e1f2 100644 --- a/liblttng-ust/lttng-ring-buffer-metadata-client.h +++ b/liblttng-ust/lttng-ring-buffer-metadata-client.h @@ -101,6 +101,9 @@ static void client_buffer_begin(struct lttng_ust_lib_ring_buffer *buf, uint64_t handle); struct lttng_channel *lttng_chan = channel_get_private(chan); + assert(header); + if (!header) + return; header->magic = TSDL_MAGIC_NUMBER; memcpy(header->uuid, lttng_chan->uuid, sizeof(lttng_chan->uuid)); header->checksum = 0; /* 0 if unused */ @@ -129,6 +132,9 @@ static void client_buffer_end(struct lttng_ust_lib_ring_buffer *buf, uint64_t ts handle); unsigned long records_lost = 0; + assert(header); + if (!header) + return; header->content_size = data_size * CHAR_BIT; /* in bits */ header->packet_size = PAGE_ALIGN(data_size) * CHAR_BIT; /* in bits */ /* diff --git a/libringbuffer/ring_buffer_backend.c b/libringbuffer/ring_buffer_backend.c index 368a71e8..d1a5959a 100644 --- a/libringbuffer/ring_buffer_backend.c +++ b/libringbuffer/ring_buffer_backend.c @@ -46,12 +46,16 @@ int lib_ring_buffer_backend_allocate(const struct lttng_ust_lib_ring_buffer_conf struct lttng_ust_shm_handle *handle, struct shm_object *shmobj) { - struct channel_backend *chanb = &shmp(handle, bufb->chan)->backend; + struct channel_backend *chanb; unsigned long subbuf_size, mmap_offset = 0; unsigned long num_subbuf_alloc; unsigned long i; long page_size; + chanb = &shmp(handle, bufb->chan)->backend; + if (!chanb) + return -EINVAL; + subbuf_size = chanb->subbuf_size; num_subbuf_alloc = num_subbuf; @@ -97,8 +101,14 @@ int lib_ring_buffer_backend_allocate(const struct lttng_ust_lib_ring_buffer_conf if (caa_unlikely(!shmp(handle, bufb->buf_wsb))) goto free_array; - for (i = 0; i < num_subbuf; i++) - shmp_index(handle, bufb->buf_wsb, i)->id = subbuffer_id(config, 0, 1, i); + for (i = 0; i < num_subbuf; i++) { + struct lttng_ust_lib_ring_buffer_backend_subbuffer *sb; + + sb = shmp_index(handle, bufb->buf_wsb, i); + if (!sb) + goto free_array; + sb->id = subbuffer_id(config, 0, 1, i); + } /* Assign read-side subbuffer table */ if (extra_reader_sb) @@ -109,16 +119,23 @@ int lib_ring_buffer_backend_allocate(const struct lttng_ust_lib_ring_buffer_conf /* Assign pages to page index */ for (i = 0; i < num_subbuf_alloc; i++) { + struct lttng_ust_lib_ring_buffer_backend_pages_shmp *sbp; + struct lttng_ust_lib_ring_buffer_backend_pages *pages; struct shm_ref ref; ref.index = bufb->memory_map._ref.index; ref.offset = bufb->memory_map._ref.offset; ref.offset += i * subbuf_size; - set_shmp(shmp(handle, shmp_index(handle, bufb->array, i)->shmp)->p, - ref); + sbp = shmp_index(handle, bufb->array, i); + if (!sbp) + goto free_array; + pages = shmp(handle, sbp->shmp); + if (!pages) + goto free_array; + set_shmp(pages->p, ref); if (config->output == RING_BUFFER_MMAP) { - shmp(handle, shmp_index(handle, bufb->array, i)->shmp)->mmap_offset = mmap_offset; + pages->mmap_offset = mmap_offset; mmap_offset += subbuf_size; } } @@ -153,17 +170,28 @@ int lib_ring_buffer_backend_create(struct lttng_ust_lib_ring_buffer_backend *buf void lib_ring_buffer_backend_reset(struct lttng_ust_lib_ring_buffer_backend *bufb, struct lttng_ust_shm_handle *handle) { - struct channel_backend *chanb = &shmp(handle, bufb->chan)->backend; - const struct lttng_ust_lib_ring_buffer_config *config = &chanb->config; + struct channel_backend *chanb; + const struct lttng_ust_lib_ring_buffer_config *config; unsigned long num_subbuf_alloc; unsigned int i; + chanb = &shmp(handle, bufb->chan)->backend; + if (!chanb) + abort(); + config = &chanb->config; + num_subbuf_alloc = chanb->num_subbuf; if (chanb->extra_reader_sb) num_subbuf_alloc++; - for (i = 0; i < chanb->num_subbuf; i++) - shmp_index(handle, bufb->buf_wsb, i)->id = subbuffer_id(config, 0, 1, i); + for (i = 0; i < chanb->num_subbuf; i++) { + struct lttng_ust_lib_ring_buffer_backend_subbuffer *sb; + + sb = shmp_index(handle, bufb->buf_wsb, i); + if (!sb) + abort(); + sb->id = subbuffer_id(config, 0, 1, i); + } if (chanb->extra_reader_sb) bufb->buf_rsb.id = subbuffer_id(config, 0, 1, num_subbuf_alloc - 1); @@ -171,10 +199,19 @@ void lib_ring_buffer_backend_reset(struct lttng_ust_lib_ring_buffer_backend *buf bufb->buf_rsb.id = subbuffer_id(config, 0, 1, 0); for (i = 0; i < num_subbuf_alloc; i++) { + struct lttng_ust_lib_ring_buffer_backend_pages_shmp *sbp; + struct lttng_ust_lib_ring_buffer_backend_pages *pages; + + sbp = shmp_index(handle, bufb->array, i); + if (!sbp) + abort(); + pages = shmp(handle, sbp->shmp); + if (!pages) + abort(); /* Don't reset mmap_offset */ - v_set(config, &shmp(handle, shmp_index(handle, bufb->array, i)->shmp)->records_commit, 0); - v_set(config, &shmp(handle, shmp_index(handle, bufb->array, i)->shmp)->records_unread, 0); - shmp(handle, shmp_index(handle, bufb->array, i)->shmp)->data_size = 0; + v_set(config, &pages->records_commit, 0); + v_set(config, &pages->records_unread, 0); + pages->data_size = 0; /* Don't reset backend page and virt addresses */ } /* Don't reset num_pages_per_subbuf, cpu, allocated */ @@ -368,13 +405,17 @@ void channel_backend_free(struct channel_backend *chanb, size_t lib_ring_buffer_read(struct lttng_ust_lib_ring_buffer_backend *bufb, size_t offset, void *dest, size_t len, struct lttng_ust_shm_handle *handle) { - struct channel_backend *chanb = &shmp(handle, bufb->chan)->backend; - const struct lttng_ust_lib_ring_buffer_config *config = &chanb->config; + struct channel_backend *chanb; + const struct lttng_ust_lib_ring_buffer_config *config; ssize_t orig_len; struct lttng_ust_lib_ring_buffer_backend_pages_shmp *rpages; unsigned long sb_bindex, id; void *src; + chanb = &shmp(handle, bufb->chan)->backend; + if (!chanb) + return 0; + config = &chanb->config; orig_len = len; offset &= chanb->buf_size - 1; @@ -412,13 +453,17 @@ size_t lib_ring_buffer_read(struct lttng_ust_lib_ring_buffer_backend *bufb, size int lib_ring_buffer_read_cstr(struct lttng_ust_lib_ring_buffer_backend *bufb, size_t offset, void *dest, size_t len, struct lttng_ust_shm_handle *handle) { - struct channel_backend *chanb = &shmp(handle, bufb->chan)->backend; - const struct lttng_ust_lib_ring_buffer_config *config = &chanb->config; + struct channel_backend *chanb; + const struct lttng_ust_lib_ring_buffer_config *config; ssize_t string_len, orig_offset; char *str; struct lttng_ust_lib_ring_buffer_backend_pages_shmp *rpages; unsigned long sb_bindex, id; + chanb = &shmp(handle, bufb->chan)->backend; + if (!chanb) + return -EINVAL; + config = &chanb->config; if (caa_unlikely(!len)) return -EINVAL; offset &= chanb->buf_size - 1; @@ -460,10 +505,14 @@ void *lib_ring_buffer_read_offset_address(struct lttng_ust_lib_ring_buffer_backe struct lttng_ust_shm_handle *handle) { struct lttng_ust_lib_ring_buffer_backend_pages_shmp *rpages; - struct channel_backend *chanb = &shmp(handle, bufb->chan)->backend; - const struct lttng_ust_lib_ring_buffer_config *config = &chanb->config; + struct channel_backend *chanb; + const struct lttng_ust_lib_ring_buffer_config *config; unsigned long sb_bindex, id; + chanb = &shmp(handle, bufb->chan)->backend; + if (!chanb) + return NULL; + config = &chanb->config; offset &= chanb->buf_size - 1; id = bufb->buf_rsb.id; sb_bindex = subbuffer_id_get_index(config, id); @@ -489,13 +538,21 @@ void *lib_ring_buffer_offset_address(struct lttng_ust_lib_ring_buffer_backend *b { size_t sbidx; struct lttng_ust_lib_ring_buffer_backend_pages_shmp *rpages; - struct channel_backend *chanb = &shmp(handle, bufb->chan)->backend; - const struct lttng_ust_lib_ring_buffer_config *config = &chanb->config; + struct channel_backend *chanb; + const struct lttng_ust_lib_ring_buffer_config *config; unsigned long sb_bindex, id; + struct lttng_ust_lib_ring_buffer_backend_subbuffer *sb; + chanb = &shmp(handle, bufb->chan)->backend; + if (!chanb) + return NULL; + config = &chanb->config; offset &= chanb->buf_size - 1; sbidx = offset >> chanb->subbuf_size_order; - id = shmp_index(handle, bufb->buf_wsb, sbidx)->id; + sb = shmp_index(handle, bufb->buf_wsb, sbidx); + if (!sb) + return NULL; + id = sb->id; sb_bindex = subbuffer_id_get_index(config, id); rpages = shmp_index(handle, bufb->array, sb_bindex); CHAN_WARN_ON(chanb, config->mode == RING_BUFFER_OVERWRITE diff --git a/libringbuffer/ring_buffer_frontend.c b/libringbuffer/ring_buffer_frontend.c index aeb7f7d9..77019e9c 100644 --- a/libringbuffer/ring_buffer_frontend.c +++ b/libringbuffer/ring_buffer_frontend.c @@ -159,10 +159,14 @@ static struct timer_signal_data timer_signal = { void lib_ring_buffer_reset(struct lttng_ust_lib_ring_buffer *buf, struct lttng_ust_shm_handle *handle) { - struct channel *chan = shmp(handle, buf->backend.chan); - const struct lttng_ust_lib_ring_buffer_config *config = &chan->backend.config; + struct channel *chan; + const struct lttng_ust_lib_ring_buffer_config *config; unsigned int i; + chan = shmp(handle, buf->backend.chan); + if (!chan) + abort(); + config = &chan->backend.config; /* * Reset iterator first. It will put the subbuffer if it currently holds * it. @@ -302,6 +306,9 @@ void lib_ring_buffer_channel_switch_timer(int sig, siginfo_t *si, void *uc) for_each_possible_cpu(cpu) { struct lttng_ust_lib_ring_buffer *buf = shmp(handle, chan->backend.buf[cpu].shmp); + + if (!buf) + abort(); if (uatomic_read(&buf->active_readers)) lib_ring_buffer_switch_slow(buf, SWITCH_ACTIVE, chan->handle); @@ -310,6 +317,8 @@ void lib_ring_buffer_channel_switch_timer(int sig, siginfo_t *si, void *uc) struct lttng_ust_lib_ring_buffer *buf = shmp(handle, chan->backend.buf[0].shmp); + if (!buf) + abort(); if (uatomic_read(&buf->active_readers)) lib_ring_buffer_switch_slow(buf, SWITCH_ACTIVE, chan->handle); @@ -337,6 +346,8 @@ void lib_ring_buffer_channel_do_read(struct channel *chan) struct lttng_ust_lib_ring_buffer *buf = shmp(handle, chan->backend.buf[cpu].shmp); + if (!buf) + abort(); if (uatomic_read(&buf->active_readers) && lib_ring_buffer_poll_deliver(config, buf, chan, handle)) { @@ -347,6 +358,8 @@ void lib_ring_buffer_channel_do_read(struct channel *chan) struct lttng_ust_lib_ring_buffer *buf = shmp(handle, chan->backend.buf[0].shmp); + if (!buf) + abort(); if (uatomic_read(&buf->active_readers) && lib_ring_buffer_poll_deliver(config, buf, chan, handle)) { -- 2.34.1