[Qemu-devel] [PATCH] readline: avoid memcpy() of overlapping regions
memcpy() for overlapping regions is undefined behavior; use memmove()
instead in readline_hist_add().
Signed-off-by: Nickolai Zeldovich nicko...@csail.mit.edu
---
readline.c |4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/readline.c b/readline.c
index 5fc9643..aeccc7b 100644
--- a/readline.c
+++ b/readline.c
@@ -248,8 +248,8 @@ static void readline_hist_add(ReadLineState *rs, const char
*cmdline)
if (idx == READLINE_MAX_CMDS) {
/* Need to get one free slot */
free(rs-history[0]);
- memcpy(rs-history, rs-history[1],
- (READLINE_MAX_CMDS - 1) * sizeof(char *));
+memmove(rs-history, rs-history[1],
+(READLINE_MAX_CMDS - 1) * sizeof(char *));
rs-history[READLINE_MAX_CMDS - 1] = NULL;
idx = READLINE_MAX_CMDS - 1;
}
--
1.7.10.4
[Qemu-devel] [PATCH v2] linux-user/syscall.c: fix copy_to_user_fdset for fds over 30
On a 64-bit system (e.g., x86_64), copy_to_user_fdset populates the
bitmask returned to the user-space program by left-shifting the value
(FD_ISSET(k, fds) != 0), which is of type int, by k bits (0 through 63).
According to the C standard, left-shifting an int by 31 bits is undefined
behavior because it shifts a 1 into the sign bit, and shifting an int
by 32 bits or more is UB because it's equal to or greater than the
type's width.
The resulting behavior depends on the specific compiler, but with gcc
4.7.2 on an x86_64 host (as well as guest), select calls that were
supposed to set fd 31 on return would actually set fds 31 through 63,
and select calls that were supposed to set an fd above 31 (e.g., 48)
would set that fd mod 32 (e.g., 16).
This patch fixes the problem by casting the value (FD_ISSET(..) != 0)
to a suitably long and unsigned type before doing the left-shift, and
fixes select for fds above 32 on x86_64.
Signed-off-by: Nickolai Zeldovich nicko...@csail.mit.edu
---
v2 of this patch removes unnecessary parentheses, as suggested by Richard
Henderson -- thanks!
linux-user/syscall.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 5a81d9f..f6e9d47 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -912,7 +912,7 @@ static inline abi_long copy_to_user_fdset(abi_ulong
target_fds_addr,
for (i = 0; i nw; i++) {
v = 0;
for (j = 0; j TARGET_ABI_BITS; j++) {
-v |= ((FD_ISSET(k, fds) != 0) j);
+v |= (abi_ulong) (FD_ISSET(k, fds) != 0) j;
k++;
}
__put_user(v, target_fds[i]);
--
1.7.10.4
[Qemu-devel] [PATCH] linux-user/syscall.c: fix select on x86_64
Use the correct argument passing convention for select on x86_64.
Previously, select worked for i386 but was broken for x86_64 (always
returning EINVAL). With this change, select works on both i386 and
x86_64. (Other targets untested but should be unaffected.)
Signed-off-by: Nickolai Zeldovich nicko...@csail.mit.edu
---
linux-user/syscall.c |8 +---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index e99adab..5a81d9f 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -6213,7 +6213,8 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
ret = get_errno(settimeofday(tv, NULL));
}
break;
-#if defined(TARGET_NR_select) !defined(TARGET_S390X)
!defined(TARGET_S390)
+#if defined(TARGET_NR_select) !defined(TARGET_S390X) \
+ !defined(TARGET_S390) !defined(TARGET_X86_64)
case TARGET_NR_select:
{
struct target_sel_arg_struct *sel;
@@ -7153,8 +7154,9 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
}
break;
#endif /* TARGET_NR_getdents64 */
-#if defined(TARGET_NR__newselect) || defined(TARGET_S390X)
-#ifdef TARGET_S390X
+#if defined(TARGET_NR__newselect) || defined(TARGET_S390X) \
+|| defined(TARGET_X86_64)
+#if defined(TARGET_S390X) || defined(TARGET_X86_64)
case TARGET_NR_select:
#else
case TARGET_NR__newselect:
--
1.7.10.4
[Qemu-devel] [PATCH] linux-user/syscall.c: fix copy_to_user_fdset for fds over 30
On a 64-bit system (e.g., x86_64), copy_to_user_fdset populates the
bitmask returned to the user-space program by left-shifting the value
(FD_ISSET(k, fds) != 0), which is of type int, by k bits (0 through 63).
According to the C standard, left-shifting an int by 31 bits is undefined
behavior because it shifts a 1 into the sign bit, and shifting an int
by 32 bits or more is UB because it's equal to or greater than the
type's width.
The resulting behavior depends on the specific compiler, but with gcc
4.7.2 on an x86_64 host (as well as guest), select calls that were
supposed to set fd 31 on return would actually set fds 31 through 63,
and select calls that were supposed to set an fd above 31 (e.g., 48)
would set that fd mod 32 (e.g., 16).
This patch fixes the problem by casting the value (FD_ISSET(..) != 0)
to a suitably long and unsigned type before doing the left-shift, and
fixes select for fds above 32 on x86_64.
Signed-off-by: Nickolai Zeldovich nicko...@csail.mit.edu
---
linux-user/syscall.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 5a81d9f..17c3dd6 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -912,7 +912,7 @@ static inline abi_long copy_to_user_fdset(abi_ulong
target_fds_addr,
for (i = 0; i nw; i++) {
v = 0;
for (j = 0; j TARGET_ABI_BITS; j++) {
-v |= ((FD_ISSET(k, fds) != 0) j);
+v |= (((abi_ulong) (FD_ISSET(k, fds) != 0)) j);
k++;
}
__put_user(v, target_fds[i]);
--
1.7.10.4
Re: [Qemu-devel] [PATCH] slirp: Don't crash on packets from 0.0.0.0/8.
On Mon, Nov 12, 2012 at 4:37 AM, Jan Kiszka jan.kis...@siemens.com wrote: On 2012-11-12 01:59, Nickolai Zeldovich wrote: LWIP can generate packets with a source of 0.0.0.0, which triggers an assertion failure in arp_table_add(). Instead of crashing, simply return to avoid adding an invalid ARP table entry. I would prefer to filter out such invalid packets at a different level. Did you analyzed which path it takes through the stack? The particular packet that crashed qemu for me was a gratuitous ARP, though it looks like all three calls to arp_table_add() in arp_input() can trigger this. Popping up one level, I'm not sure why arp_table_add() and arp_table_search() need a special case for 0.0.0.0/8 in the first place. I couldn't find any other code that assumes the ARP table cannot contain 0.0.0.0/8 entries. Would anything break if the check for 0.0.0.0/8 was removed from arp_table_add() and arp_table_search() altogether? Nickolai.
[Qemu-devel] [PATCH v2] slirp: Don't crash on packets from 0.0.0.0/8.
LWIP can generate packets with a source of 0.0.0.0, which triggers an
assertion failure in arp_table_add(). Instead of crashing, simply return
to avoid adding an invalid ARP table entry.
Signed-off-by: Nickolai Zeldovich nicko...@csail.mit.edu
---
slirp/arp_table.c |4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
Change from v1: adhere to qemu's code style (put braces around all
indentation blocks).
diff --git a/slirp/arp_table.c b/slirp/arp_table.c
index 5d7b8ac..bf698c1 100644
--- a/slirp/arp_table.c
+++ b/slirp/arp_table.c
@@ -38,7 +38,9 @@ void arp_table_add(Slirp *slirp, uint32_t ip_addr, uint8_t
ethaddr[ETH_ALEN])
ethaddr[3], ethaddr[4], ethaddr[5]));
/* Check 0.0.0.0/8 invalid source-only addresses */
-assert((ip_addr htonl(~(0xf 28))) != 0);
+if ((ip_addr htonl(~(0xf 28))) == 0) {
+return;
+}
if (ip_addr == 0x || ip_addr == broadcast_addr) {
/* Do not register broadcast addresses */
--
1.7.10.4
[Qemu-devel] [PATCH] slirp: Don't crash on packets from 0.0.0.0/8.
LWIP can generate packets with a source of 0.0.0.0, which triggers an
assertion failure in arp_table_add(). Instead of crashing, simply return
to avoid adding an invalid ARP table entry.
Signed-off-by: Nickolai Zeldovich nicko...@csail.mit.edu
---
slirp/arp_table.c |3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/slirp/arp_table.c b/slirp/arp_table.c
index 5d7b8ac..3318ce9 100644
--- a/slirp/arp_table.c
+++ b/slirp/arp_table.c
@@ -38,7 +38,8 @@ void arp_table_add(Slirp *slirp, uint32_t ip_addr, uint8_t
ethaddr[ETH_ALEN])
ethaddr[3], ethaddr[4], ethaddr[5]));
/* Check 0.0.0.0/8 invalid source-only addresses */
-assert((ip_addr htonl(~(0xf 28))) != 0);
+if ((ip_addr htonl(~(0xf 28))) == 0)
+return;
if (ip_addr == 0x || ip_addr == broadcast_addr) {
/* Do not register broadcast addresses */
--
1.7.10.4
[Qemu-devel] [PATCH] fix exception precision for cmpxchg8b
In qemu-0.9.0, an exception in cmpxchg8b (e.g. page fault due to a missing TLB entry) causes the wrong eip value to be pushed onto the exception stack -- it seems to be the eip of the last exception or the start of the translation block, whichever happened last. This makes it impossible to resume execution after such an exception. The simple patch below fixes it, by explicitly saving the current eip before invoking the cmpxchg8b helper; the same approach appears to be taken in many other instructions before generating code that could raise an exception. Apologies for the non-tab-clean patch, but it's simple enough to apply by hand. I can't quite understand what's generating the equivalent piece of code (to save pc_start into eip) for the cmpxchgl instruction (defined right above cmpxchg8b in translate.c). I'd be thankful if someone could explain to me where it's getting saved. Nickolai. --- qemu-0.9.0/target-i386/translate.c 2007-02-05 15:01:54.0 -0800 +++ /home/nickolai/build/qemu-0.9.0/target-i386/translate.c 2007-04-24 19:33:47.0 -0700 @@ -3800,6 +3800,7 @@ if (s-cc_op != CC_OP_DYNAMIC) gen_op_set_cc_op(s-cc_op); gen_lea_modrm(s, modrm, reg_addr, offset_addr); +gen_jmp_im(pc_start - s-cs_base); gen_op_cmpxchg8b(); s-cc_op = CC_OP_EFLAGS; break;
Re: [Qemu-devel] [PATCH] fix exception precision for cmpxchg8b
Thanks. Looks like inline-generated instructions use cpu_restore_state() to invert the translated PC into the simulated PC. Nickolai.
