Calling malloc() with a zero length is entering underspecified territory.
So ibuf_open(0) but more importantly ibuf_dynamic(0, max) step right into
that trap. In the first case the call makes little sense and we should
error out. In the second case it is better to skip the allocation of the
buffer and have a later ibuf_realloc() allocate space.
Also ensure errno is always set when erroring out in ibuf_open() and
ibuf_dynamic().
While changing this code use calloc() for the buffers so they are 0
initialized by default. ibuf_realloc() already uses recallocarray()
so the code was halfway there.
And since we are pushing the initialize memory train also do a memset() in
ibuf_reserve(). Now this part is probably more like belt and suspenders
since the buffer is already 0 (unless the caller somehow scribbled into it).
At least iked includes code to do this memset() but I'm happy to remove
this bit.
--
:wq Claudio
Index: imsg-buffer.c
===================================================================
RCS file: /cvs/src/lib/libutil/imsg-buffer.c,v
retrieving revision 1.14
diff -u -p -r1.14 imsg-buffer.c
--- imsg-buffer.c 23 Apr 2022 08:57:52 -0000 1.14
+++ imsg-buffer.c 22 May 2023 15:25:03 -0000
@@ -38,9 +38,13 @@ ibuf_open(size_t len)
{
struct ibuf *buf;
+ if (len == 0) {
+ errno = EINVAL;
+ return (NULL);
+ }
if ((buf = calloc(1, sizeof(struct ibuf))) == NULL)
return (NULL);
- if ((buf->buf = malloc(len)) == NULL) {
+ if ((buf->buf = calloc(len, 1)) == NULL) {
free(buf);
return (NULL);
}
@@ -55,14 +59,22 @@ ibuf_dynamic(size_t len, size_t max)
{
struct ibuf *buf;
- if (max < len)
+ if (max < len) {
+ errno = EINVAL;
return (NULL);
+ }
- if ((buf = ibuf_open(len)) == NULL)
+ if ((buf = calloc(1, sizeof(struct ibuf))) == NULL)
return (NULL);
-
- if (max > 0)
- buf->max = max;
+ if (len > 0) {
+ if ((buf->buf = calloc(len, 1)) == NULL) {
+ free(buf);
+ return (NULL);
+ }
+ }
+ buf->size = len;
+ buf->max = max;
+ buf->fd = -1;
return (buf);
}
@@ -120,6 +132,7 @@ ibuf_reserve(struct ibuf *buf, size_t le
b = buf->buf + buf->wpos;
buf->wpos += len;
+ memset(b, 0, len);
return (b);
}
