[Qemu-devel] [PATCH v2] linux-user/syscall.c: fix copy_to_user_fdset for fds over 30

[Nickolai Zeldovich <nickolai@csail.mit.edu>]

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 <nickolai@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