imsg_get_data Question

Kyle Ackerman <[email protected]>
Newsgroups gmane.os.openbsd.tech
Message-ID <[email protected]>
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.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.