From 56e1161b09c38c90d27182414f49d042046dded8 Mon Sep 17 00:00:00 2001 From: Warner Losh Date: Mon, 28 Jun 2021 15:56:08 -0600 Subject: [PATCH] cam: fix UB behavior The trick of subtracting one from the poitner returned from malloc results in undefined behavior: >>C89: 3.3.6 Unless both the pointer operand and the result point to a >>member of the same array object, or one past the last member of the >>array object, the behavior is undefined. Instead, allocate 1 extra element and stop adjusting the pointer. While a little wasteful, the extra is in the noise on today's systems. Reviewed by: scottl@ Sponsored by: Netflix Differential Revision: https://reviews.freebsd.org/D30847 --- sys/cam/cam_queue.c | 27 ++++++++++----------------- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/sys/cam/cam_queue.c b/sys/cam/cam_queue.c index 074b0545a1fd..1bb1713d65c5 100644 --- a/sys/cam/cam_queue.c +++ b/sys/cam/cam_queue.c @@ -60,17 +60,16 @@ camq_init(struct camq *camq, int size) bzero(camq, sizeof(*camq)); camq->array_size = size; if (camq->array_size != 0) { - camq->queue_array = (cam_pinfo**)malloc(size*sizeof(cam_pinfo*), - M_CAMQ, M_NOWAIT); + /* + * Heap algorithms like everything numbered from 1, so + * allocate one more to account for 0 base. + */ + camq->queue_array = malloc((size + 1) * sizeof(cam_pinfo*), + M_CAMQ, M_NOWAIT); if (camq->queue_array == NULL) { printf("camq_init: - cannot malloc array!\n"); return (1); } - /* - * Heap algorithms like everything numbered from 1, so - * offset our pointer into the heap array by one element. - */ - camq->queue_array--; } return (0); } @@ -85,11 +84,6 @@ void camq_fini(struct camq *queue) { if (queue->queue_array != NULL) { - /* - * Heap algorithms like everything numbered from 1, so - * our pointer into the heap array is offset by one element. - */ - queue->queue_array++; free(queue->queue_array, M_CAMQ); } } @@ -102,8 +96,8 @@ camq_resize(struct camq *queue, int new_size) KASSERT(new_size >= queue->entries, ("camq_resize: " "New queue size can't accommodate queued entries (%d < %d).", new_size, queue->entries)); - new_array = (cam_pinfo **)malloc(new_size * sizeof(cam_pinfo *), - M_CAMQ, M_NOWAIT); + new_array = malloc((new_size + 1) * sizeof(cam_pinfo *), M_CAMQ, + M_NOWAIT); if (new_array == NULL) { /* Couldn't satisfy request */ return (CAM_RESRC_UNAVAIL); @@ -114,12 +108,11 @@ camq_resize(struct camq *queue, int new_size) * by one element. */ if (queue->queue_array != NULL) { - queue->queue_array++; bcopy(queue->queue_array, new_array, - queue->entries * sizeof(cam_pinfo *)); + (queue->entries + 1) * sizeof(cam_pinfo *)); free(queue->queue_array, M_CAMQ); } - queue->queue_array = new_array-1; + queue->queue_array = new_array; queue->array_size = new_size; return (CAM_REQ_CMP); }