On 01/27/15 00:19, Lennart Poettering wrote: > On Sun, 25.01.15 07:10, Topi Miettinen (toiwo...@gmail.com) wrote: > >> On 01/25/15 03:34, Zbigniew Jędrzejewski-Szmek wrote: >>> On Sat, Jan 24, 2015 at 10:39:56AM +0200, Topi Miettinen wrote: >>>> Leave space for the terminating zero when reading and make sure >>>> that the last byte is zero. This also makes the check for long packets >>>> equivalent to code before 9c89c1ca: reject also packets with size 8192. >>>> --- >>>> src/libudev/libudev-monitor.c | 4 +++- >>>> 1 file changed, 3 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/src/libudev/libudev-monitor.c b/src/libudev/libudev-monitor.c >>>> index 4cfb2f6..b7fc031 100644 >>>> --- a/src/libudev/libudev-monitor.c >>>> +++ b/src/libudev/libudev-monitor.c >>>> @@ -590,7 +590,7 @@ retry: >>>> if (udev_monitor == NULL) >>>> return NULL; >>>> iov.iov_base = &buf; >>>> - iov.iov_len = sizeof(buf); >>>> + iov.iov_len = sizeof(buf) - 1; /* Leave space for terminating >>>> zero */ >>>> memzero(&smsg, sizeof(struct msghdr)); >>>> smsg.msg_iov = &iov; >>>> smsg.msg_iovlen = 1; >>>> @@ -642,6 +642,8 @@ retry: >>>> if (udev_device == NULL) >>>> return NULL; >>>> >>>> + buf.raw[sizeof(buf.raw) - 1] = '\0'; >>>> + >>>> if (memcmp(buf.raw, "libudev", 8) == 0) { >>>> /* udev message needs proper version magic */ >>>> if (buf.nlh.magic != htonl(UDEV_MONITOR_MAGIC)) { >>> A buffer only needs to be terminated by a zero in certain cases: usually if >>> it >>> is passed to a function which expectes that. iovecs can contain binary data, >>> and have an explicit size field, so they do not need to be zero-terminated. >>> Is there a reason why the buffer has to be zero-terminated in this case? >> >> String functions strcmp, strlen and strstr, used a few lines later, >> expect null byte terminated strings. Alternatively they could be changed >> to strncmp and friends where the scope can be limited to only the buffer. > > But the data comes from the kernel (and that's verified, securely), > hence I am wondering what kind of vulnerability you have precisely in > mind. If we don't trust the kernel, then we'll have quite a problem...
Right, I didn't look at the context. In that case, this would be just defensive programming. -Topi > > Lennart > _______________________________________________ systemd-devel mailing list systemd-devel@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/systemd-devel