Hello,

Recently, in order to access the data of an imsg, I have ran into an
issue replacing a memcpy(3) call with a imsg_get_data(3) call, which
turned out to not just be plug and play.  The conditions needed for this
are specifying a size in the imsg_get_data(3) that is less than the
internally allocated buffer size.  For example the specific situation I am
working with is a struct of size 16 bytes that I am pulling out of a
imsg with an internally allocated buffer of 64 bytes.

imsg_get_data(struct imsg *imsg, void *data, size_t len)
{
        if (len == 0) {
                errno = EINVAL;
                return (-1);
        }
        if (ibuf_size(imsg->buf) != len) { // Fails here
                errno = EBADMSG;
                return (-1);
        }
        return ibuf_get(imsg->buf, data, len);
}

I am not entirely sure why we have the constraint that requested length
in the has to be exactly the size of the imsg->buf.  Having to know the
internally allocated buffer size in order to use the imsg_get_data IMO
makes the imsg struct less opaque.  Maybe there is a better way we can
do this? I don't see the other imsg_get_* functions preforming this
check.

The above calls ibuf_get(3), which does the bounds checking:

ibuf_get(struct ibuf *buf, void *data, size_t len)
{
        if (ibuf_size(buf) < len) {
                errno = EBADMSG;
                return (-1);
        }

        memcpy(data, ibuf_data(buf), len);
        buf->rpos += len;
        return (0);
}



I propose this diff, which removes the check that the requested length
of data is equal to the size of the allocated buffer.  This would: rely
on the bounds check preformed in the ibuf_get(3) functions, removes a
ibuf_size(3) call, and allow the use of imsg_get_data(3) without really
knowing anything about the imsg struct.

diff /usr/src
path + /usr/src
commit - 63d30f6f93ff9faed796cfea1af89df9fa4894de
blob - b8d93d4ece10b8ae89b8c469d17d179126f22b3b
file + lib/libutil/imsg.c
--- lib/libutil/imsg.c
+++ lib/libutil/imsg.c
@@ -194,10 +194,6 @@ imsg_get_data(struct imsg *imsg, void *data, size_t le
 		errno = EINVAL;
 		return (-1);
 	}
-	if (ibuf_size(imsg->buf) != len) {
-		errno = EBADMSG;
-		return (-1);
-	}
 	return ibuf_get(imsg->buf, data, len);
 }
 


Thoughts/Comments/Suggestions? I can also share more about the specific
use-case I am running into if that makes a difference.