Re: [Qemu-devel] generating EFAULT in linux-user
Here is an updated patch that show more of the changes that are needed
to detect EFAULT situations in linux-user. The areas I focused on are
the socket, semaphore, and message queue family of syscalls as they all
make interesting use of pointers to structures. This patch does sit on
top of the other patches I sent to this list this week for semaphores
and message queues, so it can't be 'test driven' without them.
With this patch, more of the Linux Test Project test suite is able to
PASS, and qemu SEGVs less frequently.
Still to be done is to actually fold this into lock_user(), and finish
applying it in the other applicable syscalls.
Comments would be appreciated.
Stuart
Stuart R. Anderson [EMAIL PROTECTED]
Network Software Engineering http://www.netsweng.com/
1024D/37A79149: 0791 D3B8 9A4C 2CDC A31F
BD03 0A62 E534 37A7 9149Index: qemu/exec.c
===
--- qemu.orig/exec.c 2007-03-23 09:05:45.0 -0400
+++ qemu/exec.c 2007-03-23 09:06:23.0 -0400
@@ -1785,6 +1785,29 @@
spin_unlock(tb_lock);
}
+int page_check_range(target_ulong start, target_ulong len, int flags)
+{
+PageDesc *p;
+target_ulong end;
+target_ulong addr;
+
+end = TARGET_PAGE_ALIGN(start+len); /* must do before we loose bits in the next step */
+start = start TARGET_PAGE_MASK;
+
+if( end start ) return EFAULT; /* we've wrapped around */
+for(addr = start; addr end; addr += TARGET_PAGE_SIZE) {
+p = page_find(addr TARGET_PAGE_BITS);
+ if( !p ) return EFAULT;
+ if( !(p-flags PAGE_VALID) ) return EFAULT;
+
+if (!(p-flags PAGE_READ)
+(flags PAGE_READ) ) return EFAULT;
+if (!(p-flags PAGE_WRITE)
+(flags PAGE_WRITE) ) return EFAULT;
+}
+return 0;
+}
+
/* called from signal handler: invalidate the code and unprotect the
page. Return TRUE if the fault was succesfully handled. */
int page_unprotect(target_ulong address, unsigned long pc, void *puc)
Index: qemu/cpu-all.h
===
--- qemu.orig/cpu-all.h 2007-03-23 09:05:45.0 -0400
+++ qemu/cpu-all.h 2007-03-23 10:52:16.0 -0400
@@ -689,6 +689,7 @@
int page_get_flags(target_ulong address);
void page_set_flags(target_ulong start, target_ulong end, int flags);
void page_unprotect_range(target_ulong data, target_ulong data_size);
+int page_check_range(target_ulong start, target_ulong len, int flags);
#define SINGLE_CPU_DEFINES
#ifdef SINGLE_CPU_DEFINES
Index: qemu/linux-user/syscall.c
===
--- qemu.orig/linux-user/syscall.c 2007-03-23 09:06:16.0 -0400
+++ qemu/linux-user/syscall.c 2007-03-23 10:55:01.0 -0400
@@ -406,28 +406,36 @@
return ret;
}
-static inline void target_to_host_sockaddr(struct sockaddr *addr,
+static inline long target_to_host_sockaddr(struct sockaddr *addr,
target_ulong target_addr,
- socklen_t len)
+ socklen_t len,
+ int pg_access)
{
+long ret = 0;
struct target_sockaddr *target_saddr;
target_saddr = lock_user(target_addr, len, 1);
+if( ret=page_check_range(target_saddr,len,pg_access) ) return ret;
memcpy(addr, target_saddr, len);
addr-sa_family = tswap16(target_saddr-sa_family);
unlock_user(target_saddr, target_addr, 0);
+return ret;
}
-static inline void host_to_target_sockaddr(target_ulong target_addr,
+static inline long host_to_target_sockaddr(target_ulong target_addr,
struct sockaddr *addr,
- socklen_t len)
+ socklen_t len,
+ int pg_access)
{
+long ret = 0;
struct target_sockaddr *target_saddr;
target_saddr = lock_user(target_addr, len, 0);
+if( ret=page_check_range(target_saddr,len,pg_access) ) return ret;
memcpy(target_saddr, addr, len);
target_saddr-sa_family = tswap16(addr-sa_family);
unlock_user(target_saddr, target_addr, len);
+return ret;
}
/* ??? Should this also swap msgh-name? */
@@ -788,18 +796,20 @@
static long do_bind(int sockfd, target_ulong target_addr,
socklen_t addrlen)
{
+long ret = 0;
void *addr = alloca(addrlen);
-target_to_host_sockaddr(addr, target_addr, addrlen);
+if( ret=target_to_host_sockaddr(addr, target_addr, addrlen,PAGE_READ) ) return -ret;
return get_errno(bind(sockfd, addr, addrlen));
}
static long do_connect(int sockfd, target_ulong target_addr,
[Qemu-devel] generating EFAULT in linux-user
With a little help from Paul yesterday, I was able to come up with a
scheme for detecting bad pointers passed to system calls in linux-user
mode. This is used to return EFAULT as would be done on a real kernel.
The attached patch is very preliminary, but shows how it can be done.
I'm sending it now to solicit comments.
The patch currently just add a seperate call to validate the address. Per
yesterdays discussion, the checking should be folded into lock_user(),
but it's not a trivial drop in as lock_user() and lock_user_struct() are
used in different ways in different places, and none of them are actually
checking a return value. I'm still thinking on how best to accomplish
this part.
The end result, is that the tests in LTPs msg* tests that try to
generate EFAULT can now do so (and thus PASS).
Stuart
Stuart R. Anderson [EMAIL PROTECTED]
Network Software Engineering http://www.netsweng.com/
1024D/37A79149: 0791 D3B8 9A4C 2CDC A31F
BD03 0A62 E534 37A7 9149Index: qemu/exec.c
===
--- qemu.orig/exec.c 2007-03-22 17:18:32.0 -0400
+++ qemu/exec.c 2007-03-22 17:42:30.0 -0400
@@ -1785,6 +1785,29 @@
spin_unlock(tb_lock);
}
+int page_check_range(target_ulong start, target_ulong len, int flags)
+{
+PageDesc *p;
+target_ulong end;
+target_ulong addr;
+
+end = TARGET_PAGE_ALIGN(start+len); /* must do before we loose bits in the next step */
+start = start TARGET_PAGE_MASK;
+
+if( end start ) return EFAULT; /* we've wrapped around */
+for(addr = start; addr end; addr += TARGET_PAGE_SIZE) {
+p = page_find(addr TARGET_PAGE_BITS);
+ if( !p ) return EFAULT;
+ if( !(p-flags PAGE_VALID) ) return EFAULT;
+
+if (!(p-flags PAGE_READ)
+(flags PAGE_READ) ) return EFAULT;
+if (!(p-flags PAGE_WRITE)
+(flags PAGE_WRITE) ) return EFAULT;
+}
+return 0;
+}
+
/* called from signal handler: invalidate the code and unprotect the
page. Return TRUE if the fault was succesfully handled. */
int page_unprotect(target_ulong address, unsigned long pc, void *puc)
Index: qemu/cpu-all.h
===
--- qemu.orig/cpu-all.h 2007-03-22 17:18:32.0 -0400
+++ qemu/cpu-all.h 2007-03-22 17:19:10.0 -0400
@@ -689,6 +689,7 @@
int page_get_flags(target_ulong address);
void page_set_flags(target_ulong start, target_ulong end, int flags);
void page_unprotect_range(target_ulong data, target_ulong data_size);
+int page_check_range(target_ulong start, target_ulong len, int flags);
#define SINGLE_CPU_DEFINES
#ifdef SINGLE_CPU_DEFINES
Index: qemu/linux-user/syscall.c
===
--- qemu.orig/linux-user/syscall.c 2007-03-22 17:19:00.0 -0400
+++ qemu/linux-user/syscall.c 2007-03-22 17:26:17.0 -0400
@@ -1287,12 +1287,15 @@
target_ulong __unused5;
};
-static inline void target_to_host_msqid_ds(struct msqid_ds *host_md,
- target_ulong target_addr)
+/* static inline */ long target_to_host_msqid_ds(struct msqid_ds *host_md,
+ target_ulong target_addr,
+ int pg_access)
{
+long ret = 0;
struct target_msqid_ds *target_md;
lock_user_struct(target_md, target_addr, 1);
+if( ret=page_check_range(target_md,sizeof(*target_md),pg_access) ) return -ret;
target_to_host_ipc_perm((host_md-msg_perm),target_addr);
host_md-msg_stime = tswapl(target_md-msg_stime);
host_md-msg_rtime = tswapl(target_md-msg_rtime);
@@ -1303,9 +1306,10 @@
host_md-msg_lspid = tswapl(target_md-msg_lspid);
host_md-msg_lrpid = tswapl(target_md-msg_lrpid);
unlock_user_struct(target_md, target_addr, 0);
+return ret;
}
-static inline void host_to_target_msqid_ds(target_ulong target_addr,
+/* static inline */ void host_to_target_msqid_ds(target_ulong target_addr,
struct msqid_ds *host_md)
{
struct target_msqid_ds *target_md;
@@ -1323,17 +1327,22 @@
unlock_user_struct(target_md, target_addr, 1);
}
-static inline long do_msgctl(long first, long second, long ptr)
+/* static inline */ long do_msgctl(long first, long second, long ptr)
{
struct msqid_ds dsarg;
int cmd = second0xff;
long ret = 0;
switch( cmd ) {
case IPC_STAT:
+if( ret=target_to_host_msqid_ds(dsarg,ptr,PAGE_WRITE) ) return -ret;
+ret = get_errno(msgctl(first, cmd, dsarg));
+host_to_target_msqid_ds(ptr,dsarg);
+ break;
case IPC_SET:
-target_to_host_msqid_ds(dsarg,ptr);
+if( ret=target_to_host_msqid_ds(dsarg,ptr,PAGE_READ) ) return -ret;
