From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dragonfly.birch.relay.mailchannels.net (dragonfly.birch.relay.mailchannels.net [23.83.209.51]) by sourceware.org (Postfix) with ESMTPS id E74AC3858297 for ; Fri, 29 Jul 2022 16:40:47 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org E74AC3858297 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=gotplt.org Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gotplt.org X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org Received: from relay.mailchannels.net (localhost [127.0.0.1]) by relay.mailchannels.net (Postfix) with ESMTP id 14429562277; Fri, 29 Jul 2022 16:40:46 +0000 (UTC) Received: from pdx1-sub0-mail-a305.dreamhost.com (unknown [127.0.0.6]) (Authenticated sender: dreamhost) by relay.mailchannels.net (Postfix) with ESMTPA id 8882E5621F6; Fri, 29 Jul 2022 16:40:45 +0000 (UTC) ARC-Seal: i=1; s=arc-2022; d=mailchannels.net; t=1659112845; a=rsa-sha256; cv=none; b=WkGIHu/pZ/Vk4WE1uUjBl1n5W99GzcSDvWZew8WOHrieayvXSWAd2jGhUu10DoKl3UrP8k zQJJmaWe3I8LuCO23x0bddKHgoEaJANdzWG6NsWnsNUqf+cb5FVIOVq69jNqFYahlokP29 pgvRU3vtSFbEg+iK1eaXft6ve+VNqdeJWHhJS7jFmE1lbHer4BLaJaM8vMrC7w6qwqvFoB xDtthf1MaTHr8k4vjcPscZcMomCIZkIaYyxGALPSF3DOTf578W2d5JKWxjnLcAduhdIf5H VQD1KfBYsEaoPyayfFUwujlZNxbKOvPN85PyUeO9eoepRzHWQ/1nn7Jci/4eiQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=mailchannels.net; s=arc-2022; t=1659112845; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=IDtrmA1GD5CYZCd5F7WpSGkgccvA/FEvuBsL1owrjrw=; b=WDMl5gfIeb3itS/wK7vEFlcu+5Lurv83AP4n3tF8zhMJov+kV1wKM5RwNUqWOoZ8r4hcQ/ /RBTTRWwr+z9pk4VmsVUWnfFwJfuNQBRBl9nvFZyCQN6jhvnKHaqz8o2rr1nc3D4fWe7PP L0ku3JQ5iOVa0gwGaxV7bKsYoQuiWeERPFke8BK6UchM6hw9mkBaH0h6PJPTB8PjeN+r9K lTYvgEgGSA3VIRpJJnF8VkMDUS55lk0a03R/jE++TQOwM6p0zDuO16610Z8x5grDEs+6h6 U7AawZcTj132AAhkuni5g7z2AFm/WcvLebRDXwidX+vggbDZ49qnC3Ml9yLxzw== ARC-Authentication-Results: i=1; rspamd-6db896cf57-zxb7n; auth=pass smtp.auth=dreamhost smtp.mailfrom=siddhesh@gotplt.org X-Sender-Id: dreamhost|x-authsender|siddhesh@gotplt.org X-MC-Relay: Neutral X-MailChannels-SenderId: dreamhost|x-authsender|siddhesh@gotplt.org X-MailChannels-Auth-Id: dreamhost X-Gusty-Thread: 7006cd774562f0e4_1659112845826_55990228 X-MC-Loop-Signature: 1659112845826:2075519176 X-MC-Ingress-Time: 1659112845826 Received: from pdx1-sub0-mail-a305.dreamhost.com (pop.dreamhost.com [64.90.62.162]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384) by 100.116.106.119 (trex/6.7.1); Fri, 29 Jul 2022 16:40:45 +0000 Received: from [192.168.2.151] (bras-base-bmtnon1328w-grc-12-174-91-14-188.dsl.bell.ca [174.91.14.188]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits)) (No client certificate requested) (Authenticated sender: siddhesh@gotplt.org) by pdx1-sub0-mail-a305.dreamhost.com (Postfix) with ESMTPSA id 4LvYDD6drMz3B; Fri, 29 Jul 2022 09:40:44 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gotplt.org; s=dreamhost; t=1659112845; bh=IDtrmA1GD5CYZCd5F7WpSGkgccvA/FEvuBsL1owrjrw=; h=Date:Subject:To:From:Content-Type:Content-Transfer-Encoding; b=vbwarKsf3cR++ClbUQqUPjRKaC52C894hKCRVmwQGZOKJ52436mfUGGhjRmfInSSe PAEC7K48MX7NjxJfTHgmPvi8/zprA0odwTqGYO5Eh70BJwPeygTY+hPLY3VwYk4ZU+ uffrh4JNHCM+SyWp2b39SglKZFXQd2BC+nqqgfznsWzWiXPWoAGd1tAX75fFv2Xs4K pZPqUbiJzCCq+T6dxbroslTqc1KqpW74IoOM1uovfu55Jsql8cd86dQfw6tpKc8z0+ WlqgfvthErKV8qcJLF97vnJp7IYGP2wjUtXN+izXbfQ2abRmbg3Em+0aXxXDtK4VUy ZOMPIJ0+nGs5w== Message-ID: Date: Fri, 29 Jul 2022 12:40:43 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [PATCH v4] socket: Check lengths before advancing pointer in CMSG_NXTHDR Content-Language: en-US To: Arjun Shankar , libc-alpha@sourceware.org References: <20220729132637.1693027-1-arjun@redhat.com> From: Siddhesh Poyarekar In-Reply-To: <20220729132637.1693027-1-arjun@redhat.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-3037.1 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, GIT_PATCH_0, KAM_SHORT, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP, URIBL_BLACK autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 29 Jul 2022 16:40:51 -0000 On 2022-07-29 09:26, Arjun Shankar wrote: > The inline and library functions that the CMSG_NXTHDR macro may expand > to increment the pointer to the header before checking the stride of > the increment against available space. Since C only allows incrementing > pointers to one past the end of an array, the increment must be done > after a length check. This commit fixes that and includes a regression > test for CMSG_FIRSTHDR and CMSG_NXTHDR. > > The Linux, Hurd, and generic headers are all changed. > > Tested on Linux on armv7hl, i686, x86_64, aarch64, ppc64le, and s390x. > > [BZ #28846] > --- > v3: https://sourceware.org/pipermail/libc-alpha/2022-July/140854.html > > Notes on v4: > > * Addressed review comments from Siddhesh: > > 1. (sizeof (struct cmsghdr) + __CMSG_PADDING (cmsg_len)): > defined as size_needed. > > 2. >> OK, but I wonder if there's utility in making the padding a generic >> macro, e.g. > >> #define ALIGN_PADDING(n, a) ((a - (n & (a - 1))) & (a - 1)) > > This sounds useful, and actually it would be great to move *all* of the > duplicate code between these versions into a separate file and include it > in these variants. I'll try to do a follow-up with this soon. I'm going to > note it down in my TODO. > > 3. >> __msg_control_ptr doesn't really need the __ since it's a local variable. > > I thought so too. But Florian pointed out that it would interfere with > things like users #define'ing msg_control_ptr before including socket.h. > --- > bits/socket.h | 40 ++++++++++-- > socket/Makefile | 1 + > socket/tst-cmsghdr-skeleton.c | 92 +++++++++++++++++++++++++++ > socket/tst-cmsghdr.c | 56 ++++++++++++++++ > sysdeps/mach/hurd/bits/socket.h | 40 ++++++++++-- > sysdeps/unix/sysv/linux/bits/socket.h | 40 ++++++++++-- > sysdeps/unix/sysv/linux/cmsg_nxthdr.c | 36 ++++++++--- > 7 files changed, 276 insertions(+), 29 deletions(-) > create mode 100644 socket/tst-cmsghdr-skeleton.c > create mode 100644 socket/tst-cmsghdr.c LGTM. Reviewed-by: Siddhesh Poyarekar > > diff --git a/bits/socket.h b/bits/socket.h > index 2b99dea33b..aac8c49b00 100644 > --- a/bits/socket.h > +++ b/bits/socket.h > @@ -245,6 +245,12 @@ struct cmsghdr > + CMSG_ALIGN (sizeof (struct cmsghdr))) > #define CMSG_LEN(len) (CMSG_ALIGN (sizeof (struct cmsghdr)) + (len)) > > +/* Given a length, return the additional padding necessary such that > + len + __CMSG_PADDING(len) == CMSG_ALIGN (len). */ > +#define __CMSG_PADDING(len) ((sizeof (size_t) \ > + - ((len) & (sizeof (size_t) - 1))) \ > + & (sizeof (size_t) - 1)) > + > extern struct cmsghdr *__cmsg_nxthdr (struct msghdr *__mhdr, > struct cmsghdr *__cmsg) __THROW; > #ifdef __USE_EXTERN_INLINES > @@ -254,18 +260,38 @@ extern struct cmsghdr *__cmsg_nxthdr (struct msghdr *__mhdr, > _EXTERN_INLINE struct cmsghdr * > __NTH (__cmsg_nxthdr (struct msghdr *__mhdr, struct cmsghdr *__cmsg)) > { > + /* We may safely assume that __cmsg lies between __mhdr->msg_control and > + __mhdr->msg_controllen because the user is required to obtain the first > + cmsg via CMSG_FIRSTHDR, set its length, then obtain subsequent cmsgs > + via CMSG_NXTHDR, setting lengths along the way. However, we don't yet > + trust the value of __cmsg->cmsg_len and therefore do not use it in any > + pointer arithmetic until we check its value. */ > + > + unsigned char * __msg_control_ptr = (unsigned char *) __mhdr->msg_control; > + unsigned char * __cmsg_ptr = (unsigned char *) __cmsg; > + > + size_t __size_needed = sizeof (struct cmsghdr) > + + __CMSG_PADDING (__cmsg->cmsg_len); > + > + /* The current header is malformed, too small to be a full header. */ > if ((size_t) __cmsg->cmsg_len < sizeof (struct cmsghdr)) > - /* The kernel header does this so there may be a reason. */ > return (struct cmsghdr *) 0; > > + /* There isn't enough space between __cmsg and the end of the buffer to > + hold the current cmsg *and* the next one. */ > + if (((size_t) > + (__msg_control_ptr + __mhdr->msg_controllen - __cmsg_ptr) > + < __size_needed) > + || ((size_t) > + (__msg_control_ptr + __mhdr->msg_controllen - __cmsg_ptr > + - __size_needed) > + < __cmsg->cmsg_len)) > + > + return (struct cmsghdr *) 0; > + > + /* Now, we trust cmsg_len and can use it to find the next header. */ > __cmsg = (struct cmsghdr *) ((unsigned char *) __cmsg > + CMSG_ALIGN (__cmsg->cmsg_len)); > - if ((unsigned char *) (__cmsg + 1) > ((unsigned char *) __mhdr->msg_control > - + __mhdr->msg_controllen) > - || ((unsigned char *) __cmsg + CMSG_ALIGN (__cmsg->cmsg_len) > - > ((unsigned char *) __mhdr->msg_control + __mhdr->msg_controllen))) > - /* No more entries. */ > - return (struct cmsghdr *) 0; > return __cmsg; > } > #endif /* Use `extern inline'. */ > diff --git a/socket/Makefile b/socket/Makefile > index 156eec6c85..2bde78387f 100644 > --- a/socket/Makefile > +++ b/socket/Makefile > @@ -34,6 +34,7 @@ routines := accept bind connect getpeername getsockname getsockopt \ > tests := \ > tst-accept4 \ > tst-sockopt \ > + tst-cmsghdr \ > # tests > > tests-internal := \ > diff --git a/socket/tst-cmsghdr-skeleton.c b/socket/tst-cmsghdr-skeleton.c > new file mode 100644 > index 0000000000..4c6898569b > --- /dev/null > +++ b/socket/tst-cmsghdr-skeleton.c > @@ -0,0 +1,92 @@ > +/* Test ancillary data header creation. > + Copyright (C) 2022 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + . */ > + > +/* We use the preprocessor to generate the function/macro tests instead of > + using indirection because having all the macro expansions alongside > + each other lets the compiler warn us about suspicious pointer > + arithmetic across subsequent CMSG_{FIRST,NXT}HDR expansions. */ > + > +#include > + > +#define RUN_TEST_CONCAT(suffix) run_test_##suffix > +#define RUN_TEST_FUNCNAME(suffix) RUN_TEST_CONCAT (suffix) > + > +static void > +RUN_TEST_FUNCNAME (CMSG_NXTHDR_IMPL) (void) > +{ > + struct msghdr m = {0}; > + struct cmsghdr *cmsg; > + char cmsgbuf[3 * CMSG_SPACE (sizeof (PAYLOAD))] = {0}; > + > + m.msg_control = cmsgbuf; > + m.msg_controllen = sizeof (cmsgbuf); > + > + /* First header should point to the start of the buffer. */ > + cmsg = CMSG_FIRSTHDR (&m); > + TEST_VERIFY_EXIT ((char *) cmsg == cmsgbuf); > + > + /* If the first header length consumes the entire buffer, there is no > + space remaining for additional headers. */ > + cmsg->cmsg_len = sizeof (cmsgbuf); > + cmsg = CMSG_NXTHDR_IMPL (&m, cmsg); > + TEST_VERIFY_EXIT (cmsg == NULL); > + > + /* The first header length is so big, using it would cause an overflow. */ > + cmsg = CMSG_FIRSTHDR (&m); > + TEST_VERIFY_EXIT ((char *) cmsg == cmsgbuf); > + cmsg->cmsg_len = SIZE_MAX; > + cmsg = CMSG_NXTHDR_IMPL (&m, cmsg); > + TEST_VERIFY_EXIT (cmsg == NULL); > + > + /* The first header leaves just enough space to hold another header. */ > + cmsg = CMSG_FIRSTHDR (&m); > + TEST_VERIFY_EXIT ((char *) cmsg == cmsgbuf); > + cmsg->cmsg_len = sizeof (cmsgbuf) - sizeof (struct cmsghdr); > + cmsg = CMSG_NXTHDR_IMPL (&m, cmsg); > + TEST_VERIFY_EXIT (cmsg != NULL); > + > + /* The first header leaves space but not enough for another header. */ > + cmsg = CMSG_FIRSTHDR (&m); > + TEST_VERIFY_EXIT ((char *) cmsg == cmsgbuf); > + cmsg->cmsg_len ++; > + cmsg = CMSG_NXTHDR_IMPL (&m, cmsg); > + TEST_VERIFY_EXIT (cmsg == NULL); > + > + /* The second header leaves just enough space to hold another header. */ > + cmsg = CMSG_FIRSTHDR (&m); > + TEST_VERIFY_EXIT ((char *) cmsg == cmsgbuf); > + cmsg->cmsg_len = CMSG_LEN (sizeof (PAYLOAD)); > + cmsg = CMSG_NXTHDR_IMPL (&m, cmsg); > + TEST_VERIFY_EXIT (cmsg != NULL); > + cmsg->cmsg_len = sizeof (cmsgbuf) > + - CMSG_SPACE (sizeof (PAYLOAD)) /* First header. */ > + - sizeof (struct cmsghdr); > + cmsg = CMSG_NXTHDR_IMPL (&m, cmsg); > + TEST_VERIFY_EXIT (cmsg != NULL); > + > + /* The second header leaves space but not enough for another header. */ > + cmsg = CMSG_FIRSTHDR (&m); > + TEST_VERIFY_EXIT ((char *) cmsg == cmsgbuf); > + cmsg = CMSG_NXTHDR_IMPL (&m, cmsg); > + TEST_VERIFY_EXIT (cmsg != NULL); > + cmsg->cmsg_len ++; > + cmsg = CMSG_NXTHDR_IMPL (&m, cmsg); > + TEST_VERIFY_EXIT (cmsg == NULL); > + > + return; > +} > diff --git a/socket/tst-cmsghdr.c b/socket/tst-cmsghdr.c > new file mode 100644 > index 0000000000..68c96d3c9d > --- /dev/null > +++ b/socket/tst-cmsghdr.c > @@ -0,0 +1,56 @@ > +/* Test ancillary data header creation. > + Copyright (C) 2022 Free Software Foundation, Inc. > + This file is part of the GNU C Library. > + > + The GNU C Library is free software; you can redistribute it and/or > + modify it under the terms of the GNU Lesser General Public > + License as published by the Free Software Foundation; either > + version 2.1 of the License, or (at your option) any later version. > + > + The GNU C Library is distributed in the hope that it will be useful, > + but WITHOUT ANY WARRANTY; without even the implied warranty of > + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > + Lesser General Public License for more details. > + > + You should have received a copy of the GNU Lesser General Public > + License along with the GNU C Library; if not, see > + . */ > + > +#include > +#include > +#include > +#include > + > +#define PAYLOAD "Hello, World!" > + > +/* CMSG_NXTHDR is a macro that calls an inline function defined in > + bits/socket.h. In case the function cannot be inlined, libc.so carries > + a copy. Both versions need to be tested. */ > + > +#define CMSG_NXTHDR_IMPL CMSG_NXTHDR > +#include "tst-cmsghdr-skeleton.c" > +#undef CMSG_NXTHDR_IMPL > + > +static struct cmsghdr * (* cmsg_nxthdr) (struct msghdr *, struct cmsghdr *); > + > +#define CMSG_NXTHDR_IMPL cmsg_nxthdr > +#include "tst-cmsghdr-skeleton.c" > +#undef CMSG_NXTHDR_IMPL > + > +static int > +do_test (void) > +{ > + static void *handle; > + > + run_test_CMSG_NXTHDR (); > + > + handle = xdlopen (LIBC_SO, RTLD_LAZY); > + cmsg_nxthdr = (struct cmsghdr * (*) (struct msghdr *, struct cmsghdr *)) > + xdlsym (handle, "__cmsg_nxthdr"); > + > + run_test_cmsg_nxthdr (); > + > + return 0; > +} > + > +#include > diff --git a/sysdeps/mach/hurd/bits/socket.h b/sysdeps/mach/hurd/bits/socket.h > index 5b35ea81ec..70fce4fb27 100644 > --- a/sysdeps/mach/hurd/bits/socket.h > +++ b/sysdeps/mach/hurd/bits/socket.h > @@ -249,6 +249,12 @@ struct cmsghdr > + CMSG_ALIGN (sizeof (struct cmsghdr))) > #define CMSG_LEN(len) (CMSG_ALIGN (sizeof (struct cmsghdr)) + (len)) > > +/* Given a length, return the additional padding necessary such that > + len + __CMSG_PADDING(len) == CMSG_ALIGN (len). */ > +#define __CMSG_PADDING(len) ((sizeof (size_t) \ > + - ((len) & (sizeof (size_t) - 1))) \ > + & (sizeof (size_t) - 1)) > + > extern struct cmsghdr *__cmsg_nxthdr (struct msghdr *__mhdr, > struct cmsghdr *__cmsg) __THROW; > #ifdef __USE_EXTERN_INLINES > @@ -258,18 +264,38 @@ extern struct cmsghdr *__cmsg_nxthdr (struct msghdr *__mhdr, > _EXTERN_INLINE struct cmsghdr * > __NTH (__cmsg_nxthdr (struct msghdr *__mhdr, struct cmsghdr *__cmsg)) > { > + /* We may safely assume that __cmsg lies between __mhdr->msg_control and > + __mhdr->msg_controllen because the user is required to obtain the first > + cmsg via CMSG_FIRSTHDR, set its length, then obtain subsequent cmsgs > + via CMSG_NXTHDR, setting lengths along the way. However, we don't yet > + trust the value of __cmsg->cmsg_len and therefore do not use it in any > + pointer arithmetic until we check its value. */ > + > + unsigned char * __msg_control_ptr = (unsigned char *) __mhdr->msg_control; > + unsigned char * __cmsg_ptr = (unsigned char *) __cmsg; > + > + size_t __size_needed = sizeof (struct cmsghdr) > + + __CMSG_PADDING (__cmsg->cmsg_len); > + > + /* The current header is malformed, too small to be a full header. */ > if ((size_t) __cmsg->cmsg_len < sizeof (struct cmsghdr)) > - /* The kernel header does this so there may be a reason. */ > return (struct cmsghdr *) 0; > > + /* There isn't enough space between __cmsg and the end of the buffer to > + hold the current cmsg *and* the next one. */ > + if (((size_t) > + (__msg_control_ptr + __mhdr->msg_controllen - __cmsg_ptr) > + < __size_needed) > + || ((size_t) > + (__msg_control_ptr + __mhdr->msg_controllen - __cmsg_ptr > + - __size_needed) > + < __cmsg->cmsg_len)) > + > + return (struct cmsghdr *) 0; > + > + /* Now, we trust cmsg_len and can use it to find the next header. */ > __cmsg = (struct cmsghdr *) ((unsigned char *) __cmsg > + CMSG_ALIGN (__cmsg->cmsg_len)); > - if ((unsigned char *) (__cmsg + 1) > ((unsigned char *) __mhdr->msg_control > - + __mhdr->msg_controllen) > - || ((unsigned char *) __cmsg + CMSG_ALIGN (__cmsg->cmsg_len) > - > ((unsigned char *) __mhdr->msg_control + __mhdr->msg_controllen))) > - /* No more entries. */ > - return (struct cmsghdr *) 0; > return __cmsg; > } > #endif /* Use `extern inline'. */ > diff --git a/sysdeps/unix/sysv/linux/bits/socket.h b/sysdeps/unix/sysv/linux/bits/socket.h > index 4f1f810ea1..539b8d7716 100644 > --- a/sysdeps/unix/sysv/linux/bits/socket.h > +++ b/sysdeps/unix/sysv/linux/bits/socket.h > @@ -307,6 +307,12 @@ struct cmsghdr > + CMSG_ALIGN (sizeof (struct cmsghdr))) > #define CMSG_LEN(len) (CMSG_ALIGN (sizeof (struct cmsghdr)) + (len)) > > +/* Given a length, return the additional padding necessary such that > + len + __CMSG_PADDING(len) == CMSG_ALIGN (len). */ > +#define __CMSG_PADDING(len) ((sizeof (size_t) \ > + - ((len) & (sizeof (size_t) - 1))) \ > + & (sizeof (size_t) - 1)) > + > extern struct cmsghdr *__cmsg_nxthdr (struct msghdr *__mhdr, > struct cmsghdr *__cmsg) __THROW; > #ifdef __USE_EXTERN_INLINES > @@ -316,18 +322,38 @@ extern struct cmsghdr *__cmsg_nxthdr (struct msghdr *__mhdr, > _EXTERN_INLINE struct cmsghdr * > __NTH (__cmsg_nxthdr (struct msghdr *__mhdr, struct cmsghdr *__cmsg)) > { > + /* We may safely assume that __cmsg lies between __mhdr->msg_control and > + __mhdr->msg_controllen because the user is required to obtain the first > + cmsg via CMSG_FIRSTHDR, set its length, then obtain subsequent cmsgs > + via CMSG_NXTHDR, setting lengths along the way. However, we don't yet > + trust the value of __cmsg->cmsg_len and therefore do not use it in any > + pointer arithmetic until we check its value. */ > + > + unsigned char * __msg_control_ptr = (unsigned char *) __mhdr->msg_control; > + unsigned char * __cmsg_ptr = (unsigned char *) __cmsg; > + > + size_t __size_needed = sizeof (struct cmsghdr) > + + __CMSG_PADDING (__cmsg->cmsg_len); > + > + /* The current header is malformed, too small to be a full header. */ > if ((size_t) __cmsg->cmsg_len < sizeof (struct cmsghdr)) > - /* The kernel header does this so there may be a reason. */ > return (struct cmsghdr *) 0; > > + /* There isn't enough space between __cmsg and the end of the buffer to > + hold the current cmsg *and* the next one. */ > + if (((size_t) > + (__msg_control_ptr + __mhdr->msg_controllen - __cmsg_ptr) > + < __size_needed) > + || ((size_t) > + (__msg_control_ptr + __mhdr->msg_controllen - __cmsg_ptr > + - __size_needed) > + < __cmsg->cmsg_len)) > + > + return (struct cmsghdr *) 0; > + > + /* Now, we trust cmsg_len and can use it to find the next header. */ > __cmsg = (struct cmsghdr *) ((unsigned char *) __cmsg > + CMSG_ALIGN (__cmsg->cmsg_len)); > - if ((unsigned char *) (__cmsg + 1) > ((unsigned char *) __mhdr->msg_control > - + __mhdr->msg_controllen) > - || ((unsigned char *) __cmsg + CMSG_ALIGN (__cmsg->cmsg_len) > - > ((unsigned char *) __mhdr->msg_control + __mhdr->msg_controllen))) > - /* No more entries. */ > - return (struct cmsghdr *) 0; > return __cmsg; > } > #endif /* Use `extern inline'. */ > diff --git a/sysdeps/unix/sysv/linux/cmsg_nxthdr.c b/sysdeps/unix/sysv/linux/cmsg_nxthdr.c > index 15b7a3a925..24f72b797a 100644 > --- a/sysdeps/unix/sysv/linux/cmsg_nxthdr.c > +++ b/sysdeps/unix/sysv/linux/cmsg_nxthdr.c > @@ -23,18 +23,38 @@ > struct cmsghdr * > __cmsg_nxthdr (struct msghdr *mhdr, struct cmsghdr *cmsg) > { > + /* We may safely assume that cmsg lies between mhdr->msg_control and > + mhdr->msg_controllen because the user is required to obtain the first > + cmsg via CMSG_FIRSTHDR, set its length, then obtain subsequent cmsgs > + via CMSG_NXTHDR, setting lengths along the way. However, we don't yet > + trust the value of cmsg->cmsg_len and therefore do not use it in any > + pointer arithmetic until we check its value. */ > + > + unsigned char * msg_control_ptr = (unsigned char *) mhdr->msg_control; > + unsigned char * cmsg_ptr = (unsigned char *) cmsg; > + > + size_t size_needed = sizeof (struct cmsghdr) > + + __CMSG_PADDING (cmsg->cmsg_len); > + > + /* The current header is malformed, too small to be a full header. */ > if ((size_t) cmsg->cmsg_len < sizeof (struct cmsghdr)) > - /* The kernel header does this so there may be a reason. */ > - return NULL; > + return (struct cmsghdr *) 0; > + > + /* There isn't enough space between cmsg and the end of the buffer to > + hold the current cmsg *and* the next one. */ > + if (((size_t) > + (msg_control_ptr + mhdr->msg_controllen - cmsg_ptr) > + < size_needed) > + || ((size_t) > + (msg_control_ptr + mhdr->msg_controllen - cmsg_ptr > + - size_needed) > + < cmsg->cmsg_len)) > + > + return (struct cmsghdr *) 0; > > + /* Now, we trust cmsg_len and can use it to find the next header. */ > cmsg = (struct cmsghdr *) ((unsigned char *) cmsg > + CMSG_ALIGN (cmsg->cmsg_len)); > - if ((unsigned char *) (cmsg + 1) > ((unsigned char *) mhdr->msg_control > - + mhdr->msg_controllen) > - || ((unsigned char *) cmsg + CMSG_ALIGN (cmsg->cmsg_len) > - > ((unsigned char *) mhdr->msg_control + mhdr->msg_controllen))) > - /* No more entries. */ > - return NULL; > return cmsg; > } > libc_hidden_def (__cmsg_nxthdr)