Patchwork linux-user/syscall.c: fix copy_to_user_fdset for fds over 30

login
register
mail settings
Submitter Nickolai Zeldovich
Date Jan. 3, 2013, 10:02 a.m.
Message ID <1357207368-21116-1-git-send-email-nickolai@csail.mit.edu>
Download mbox | patch
Permalink /patch/209204/
State New
Headers show

Comments

Nickolai Zeldovich - Jan. 3, 2013, 10:02 a.m.
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>
---
 linux-user/syscall.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
Richard Henderson - Jan. 4, 2013, 6:57 p.m.
On 01/03/2013 02:02 AM, Nickolai Zeldovich wrote:
> -            v |= ((FD_ISSET(k, fds) != 0) << j);
> +            v |= (((abi_ulong) (FD_ISSET(k, fds) != 0)) << j);

It would be easier to read if you dropped the unnecessary parens.

	v |= (abi_ulong)(FD_ISSET(k, fds) != 0) << j;

Otherwise the change looks correct.


r~

Patch

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]);