From: Claudio Jeker <claudio@openbsd.org>
Subject: Re: imsg_get_data Question
To: Kyle Ackerman <kack@kyleackerman.net>
Cc: tech@openbsd.org
Date: Sun, 16 Aug 2026 22:40:09 +0200

On Sun, Aug 16, 2026 at 10:04:25AM -0600, Kyle Ackerman wrote:
> 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.
> 

imsg_get_data is exactly designed to be strict:
     imsg_get_data() can be used if the structure of the payload is
     known and can be extracted in one go.
So no this diff is not acceptable.

If this is not the case like here then use imsg_get_buf() and
imsg_get_strbuf() these functions advance the buffer but then you should
also check that at the end all data was consumed using
	imsg_get_len(&imsg) == 0
For even more complex cases imsg_get_ibuf() can be used, it returns an
ibuf and can be used by all getters from the ibuf_get family.

-- 
:wq Claudio