| 1 |
From security-bounces@linux.kernel.org Tue Sep 6 01:31:17 2005
|
| 2 |
From: David Woodhouse <dwmw2@infradead.org>
|
| 3 |
To: Sebastian Krahmer <krahmer@suse.de>
|
| 4 |
Date: Tue, 06 Sep 2005 09:30:10 +0100
|
| 5 |
Subject: [PATCH] 32bit sendmsg() flaw (CAN-2005-2490)
|
| 6 |
Cc: viro@ZenIV.linux.org.uk, "David S. Miller" <davem@davemloft.net>, David Woodhouse <dwmw2@infradead.org>
|
| 7 |
|
| 8 |
When we copy 32bit ->msg_control contents to kernel, we walk the same
|
| 9 |
userland data twice without sanity checks on the second pass.
|
| 10 |
|
| 11 |
Second version of this patch: the original broke with 64-bit arches
|
| 12 |
running 32-bit-compat-mode executables doing sendmsg() syscalls with
|
| 13 |
unaligned CMSG data areas
|
| 14 |
|
| 15 |
Another thing is that we use kmalloc() to allocate and sock_kfree_s()
|
| 16 |
to free afterwards; less serious, but also needs fixing.
|
| 17 |
|
| 18 |
Patch by Al Viro, David Miller, David Woodhouse
|
| 19 |
(sparc64 clean compile fix from David Miller)
|
| 20 |
|
| 21 |
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
|
| 22 |
Signed-off-by: David Woodhouse <dwmw2@infradead.org>
|
| 23 |
Signed-off-by: Chris Wright <chrisw@osdl.org>
|
| 24 |
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>
|
| 25 |
---
|
| 26 |
include/net/compat.h | 5 +++--
|
| 27 |
net/compat.c | 44 ++++++++++++++++++++++++++------------------
|
| 28 |
net/socket.c | 3 ++-
|
| 29 |
3 files changed, 31 insertions(+), 21 deletions(-)
|
| 30 |
|
| 31 |
Index: linux-2.6.13.y/include/net/compat.h
|
| 32 |
===================================================================
|
| 33 |
--- linux-2.6.13.y.orig/include/net/compat.h
|
| 34 |
+++ linux-2.6.13.y/include/net/compat.h
|
| 35 |
@@ -33,7 +33,8 @@ extern asmlinkage long compat_sys_sendms
|
| 36 |
extern asmlinkage long compat_sys_recvmsg(int,struct compat_msghdr __user *,unsigned);
|
| 37 |
extern asmlinkage long compat_sys_getsockopt(int, int, int, char __user *, int __user *);
|
| 38 |
extern int put_cmsg_compat(struct msghdr*, int, int, int, void *);
|
| 39 |
-extern int cmsghdr_from_user_compat_to_kern(struct msghdr *, unsigned char *,
|
| 40 |
- int);
|
| 41 |
+
|
| 42 |
+struct sock;
|
| 43 |
+extern int cmsghdr_from_user_compat_to_kern(struct msghdr *, struct sock *, unsigned char *, int);
|
| 44 |
|
| 45 |
#endif /* NET_COMPAT_H */
|
| 46 |
Index: linux-2.6.13.y/net/compat.c
|
| 47 |
===================================================================
|
| 48 |
--- linux-2.6.13.y.orig/net/compat.c
|
| 49 |
+++ linux-2.6.13.y/net/compat.c
|
| 50 |
@@ -135,13 +135,14 @@ static inline struct compat_cmsghdr __us
|
| 51 |
* thus placement) of cmsg headers and length are different for
|
| 52 |
* 32-bit apps. -DaveM
|
| 53 |
*/
|
| 54 |
-int cmsghdr_from_user_compat_to_kern(struct msghdr *kmsg,
|
| 55 |
+int cmsghdr_from_user_compat_to_kern(struct msghdr *kmsg, struct sock *sk,
|
| 56 |
unsigned char *stackbuf, int stackbuf_size)
|
| 57 |
{
|
| 58 |
struct compat_cmsghdr __user *ucmsg;
|
| 59 |
struct cmsghdr *kcmsg, *kcmsg_base;
|
| 60 |
compat_size_t ucmlen;
|
| 61 |
__kernel_size_t kcmlen, tmp;
|
| 62 |
+ int err = -EFAULT;
|
| 63 |
|
| 64 |
kcmlen = 0;
|
| 65 |
kcmsg_base = kcmsg = (struct cmsghdr *)stackbuf;
|
| 66 |
@@ -156,6 +157,7 @@ int cmsghdr_from_user_compat_to_kern(str
|
| 67 |
|
| 68 |
tmp = ((ucmlen - CMSG_COMPAT_ALIGN(sizeof(*ucmsg))) +
|
| 69 |
CMSG_ALIGN(sizeof(struct cmsghdr)));
|
| 70 |
+ tmp = CMSG_ALIGN(tmp);
|
| 71 |
kcmlen += tmp;
|
| 72 |
ucmsg = cmsg_compat_nxthdr(kmsg, ucmsg, ucmlen);
|
| 73 |
}
|
| 74 |
@@ -167,30 +169,34 @@ int cmsghdr_from_user_compat_to_kern(str
|
| 75 |
* until we have successfully copied over all of the data
|
| 76 |
* from the user.
|
| 77 |
*/
|
| 78 |
- if(kcmlen > stackbuf_size)
|
| 79 |
- kcmsg_base = kcmsg = kmalloc(kcmlen, GFP_KERNEL);
|
| 80 |
- if(kcmsg == NULL)
|
| 81 |
+ if (kcmlen > stackbuf_size)
|
| 82 |
+ kcmsg_base = kcmsg = sock_kmalloc(sk, kcmlen, GFP_KERNEL);
|
| 83 |
+ if (kcmsg == NULL)
|
| 84 |
return -ENOBUFS;
|
| 85 |
|
| 86 |
/* Now copy them over neatly. */
|
| 87 |
memset(kcmsg, 0, kcmlen);
|
| 88 |
ucmsg = CMSG_COMPAT_FIRSTHDR(kmsg);
|
| 89 |
while(ucmsg != NULL) {
|
| 90 |
- __get_user(ucmlen, &ucmsg->cmsg_len);
|
| 91 |
+ if (__get_user(ucmlen, &ucmsg->cmsg_len))
|
| 92 |
+ goto Efault;
|
| 93 |
+ if (!CMSG_COMPAT_OK(ucmlen, ucmsg, kmsg))
|
| 94 |
+ goto Einval;
|
| 95 |
tmp = ((ucmlen - CMSG_COMPAT_ALIGN(sizeof(*ucmsg))) +
|
| 96 |
CMSG_ALIGN(sizeof(struct cmsghdr)));
|
| 97 |
+ if ((char *)kcmsg_base + kcmlen - (char *)kcmsg < CMSG_ALIGN(tmp))
|
| 98 |
+ goto Einval;
|
| 99 |
kcmsg->cmsg_len = tmp;
|
| 100 |
- __get_user(kcmsg->cmsg_level, &ucmsg->cmsg_level);
|
| 101 |
- __get_user(kcmsg->cmsg_type, &ucmsg->cmsg_type);
|
| 102 |
-
|
| 103 |
- /* Copy over the data. */
|
| 104 |
- if(copy_from_user(CMSG_DATA(kcmsg),
|
| 105 |
- CMSG_COMPAT_DATA(ucmsg),
|
| 106 |
- (ucmlen - CMSG_COMPAT_ALIGN(sizeof(*ucmsg)))))
|
| 107 |
- goto out_free_efault;
|
| 108 |
+ tmp = CMSG_ALIGN(tmp);
|
| 109 |
+ if (__get_user(kcmsg->cmsg_level, &ucmsg->cmsg_level) ||
|
| 110 |
+ __get_user(kcmsg->cmsg_type, &ucmsg->cmsg_type) ||
|
| 111 |
+ copy_from_user(CMSG_DATA(kcmsg),
|
| 112 |
+ CMSG_COMPAT_DATA(ucmsg),
|
| 113 |
+ (ucmlen - CMSG_COMPAT_ALIGN(sizeof(*ucmsg)))))
|
| 114 |
+ goto Efault;
|
| 115 |
|
| 116 |
/* Advance. */
|
| 117 |
- kcmsg = (struct cmsghdr *)((char *)kcmsg + CMSG_ALIGN(tmp));
|
| 118 |
+ kcmsg = (struct cmsghdr *)((char *)kcmsg + tmp);
|
| 119 |
ucmsg = cmsg_compat_nxthdr(kmsg, ucmsg, ucmlen);
|
| 120 |
}
|
| 121 |
|
| 122 |
@@ -199,10 +205,12 @@ int cmsghdr_from_user_compat_to_kern(str
|
| 123 |
kmsg->msg_controllen = kcmlen;
|
| 124 |
return 0;
|
| 125 |
|
| 126 |
-out_free_efault:
|
| 127 |
- if(kcmsg_base != (struct cmsghdr *)stackbuf)
|
| 128 |
- kfree(kcmsg_base);
|
| 129 |
- return -EFAULT;
|
| 130 |
+Einval:
|
| 131 |
+ err = -EINVAL;
|
| 132 |
+Efault:
|
| 133 |
+ if (kcmsg_base != (struct cmsghdr *)stackbuf)
|
| 134 |
+ sock_kfree_s(sk, kcmsg_base, kcmlen);
|
| 135 |
+ return err;
|
| 136 |
}
|
| 137 |
|
| 138 |
int put_cmsg_compat(struct msghdr *kmsg, int level, int type, int len, void *data)
|
| 139 |
Index: linux-2.6.13.y/net/socket.c
|
| 140 |
===================================================================
|
| 141 |
--- linux-2.6.13.y.orig/net/socket.c
|
| 142 |
+++ linux-2.6.13.y/net/socket.c
|
| 143 |
@@ -1739,10 +1739,11 @@ asmlinkage long sys_sendmsg(int fd, stru
|
| 144 |
goto out_freeiov;
|
| 145 |
ctl_len = msg_sys.msg_controllen;
|
| 146 |
if ((MSG_CMSG_COMPAT & flags) && ctl_len) {
|
| 147 |
- err = cmsghdr_from_user_compat_to_kern(&msg_sys, ctl, sizeof(ctl));
|
| 148 |
+ err = cmsghdr_from_user_compat_to_kern(&msg_sys, sock->sk, ctl, sizeof(ctl));
|
| 149 |
if (err)
|
| 150 |
goto out_freeiov;
|
| 151 |
ctl_buf = msg_sys.msg_control;
|
| 152 |
+ ctl_len = msg_sys.msg_controllen;
|
| 153 |
} else if (ctl_len) {
|
| 154 |
if (ctl_len > sizeof(ctl))
|
| 155 |
{
|