* [PATCH] net: compat: assert the size of cmsg copied in is as expected
@ 2017-09-19 17:19 Meng Xu
2017-09-20 22:36 ` David Miller
0 siblings, 1 reply; 2+ messages in thread
From: Meng Xu @ 2017-09-19 17:19 UTC (permalink / raw)
To: davem, netdev, linux-kernel; +Cc: meng.xu, sanidhya, taesoo, Meng Xu
The actual length of cmsg fetched in during the second loop
(i.e., kcmsg - kcmsg_base) could be different from what we
get from the first loop (i.e., kcmlen).
The main reason is that the two get_user() calls in the two
loops (i.e., get_user(ucmlen, &ucmsg->cmsg_len) and
__get_user(ucmlen, &ucmsg->cmsg_len)) could cause ucmlen
to have different values even they fetch from the same userspace
address, as user can race to change the memory content in
&ucmsg->cmsg_len across fetches.
Although in the second loop, the sanity check
if ((char *)kcmsg_base + kcmlen - (char *)kcmsg < CMSG_ALIGN(tmp))
is inplace, it only ensures that the cmsg fetched in during the
second loop does not exceed the length of kcmlen, but not
necessarily equal to kcmlen. But indicated by the assignment
kmsg->msg_controllen = kcmlen, we should enforce that.
This patch adds this additional sanity check and ensures that
what is recorded in kmsg->msg_controllen is the actual cmsg length.
Signed-off-by: Meng Xu <mengxu.gatech@gmail.com>
---
net/compat.c | 7 +++++++
1 file changed, 7 insertions(+)
diff --git a/net/compat.c b/net/compat.c
index 6ded6c8..2238171 100644
--- a/net/compat.c
+++ b/net/compat.c
@@ -185,6 +185,13 @@ int cmsghdr_from_user_compat_to_kern(struct msghdr *kmsg, struct sock *sk,
ucmsg = cmsg_compat_nxthdr(kmsg, ucmsg, ucmlen);
}
+ /*
+ * check the length of messages copied in is the same as the
+ * what we get from the first loop
+ */
+ if ((char *)kcmsg - (char *)kcmsg_base != kcmlen)
+ goto Einval;
+
/* Ok, looks like we made it. Hook it up and return success. */
kmsg->msg_control = kcmsg_base;
kmsg->msg_controllen = kcmlen;
--
2.7.4
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] net: compat: assert the size of cmsg copied in is as expected
2017-09-19 17:19 [PATCH] net: compat: assert the size of cmsg copied in is as expected Meng Xu
@ 2017-09-20 22:36 ` David Miller
0 siblings, 0 replies; 2+ messages in thread
From: David Miller @ 2017-09-20 22:36 UTC (permalink / raw)
To: mengxu.gatech; +Cc: netdev, linux-kernel, meng.xu, sanidhya, taesoo
From: Meng Xu <mengxu.gatech@gmail.com>
Date: Tue, 19 Sep 2017 13:19:13 -0400
> The actual length of cmsg fetched in during the second loop
> (i.e., kcmsg - kcmsg_base) could be different from what we
> get from the first loop (i.e., kcmlen).
>
> The main reason is that the two get_user() calls in the two
> loops (i.e., get_user(ucmlen, &ucmsg->cmsg_len) and
> __get_user(ucmlen, &ucmsg->cmsg_len)) could cause ucmlen
> to have different values even they fetch from the same userspace
> address, as user can race to change the memory content in
> &ucmsg->cmsg_len across fetches.
>
> Although in the second loop, the sanity check
> if ((char *)kcmsg_base + kcmlen - (char *)kcmsg < CMSG_ALIGN(tmp))
> is inplace, it only ensures that the cmsg fetched in during the
> second loop does not exceed the length of kcmlen, but not
> necessarily equal to kcmlen. But indicated by the assignment
> kmsg->msg_controllen = kcmlen, we should enforce that.
>
> This patch adds this additional sanity check and ensures that
> what is recorded in kmsg->msg_controllen is the actual cmsg length.
>
> Signed-off-by: Meng Xu <mengxu.gatech@gmail.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2017-09-20 22:36 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-09-19 17:19 [PATCH] net: compat: assert the size of cmsg copied in is as expected Meng Xu
2017-09-20 22:36 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).