Re: [PATCH v3 1/2] linux-user: Fix unaligned memory access in prlimit64 syscall

2023-03-07 Thread Laurent Vivier

Le 24/02/2023 à 01:39, Ilya Leoshkevich a écrit :

target_rlimit64 contains uint64_t fields, so it's 8-byte aligned on
some hosts, while some guests may align their respective type on a
4-byte boundary. This may lead to an unaligned access, which is an UB.

Fix by defining the fields as abi_ullong. This makes the host alignment
match that of the guest, and lets the compiler know that it should emit
code that can deal with the guest alignment.

While at it, also use __get_user() and __put_user() instead of
tswap64().

Fixes: 163a05a8398b ("linux-user: Implement prlimit64 syscall")
Reported-by: Richard Henderson 
Signed-off-by: Ilya Leoshkevich 
---
  linux-user/generic/target_resource.h | 4 ++--
  linux-user/syscall.c | 8 
  2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/linux-user/generic/target_resource.h 
b/linux-user/generic/target_resource.h
index 539d8c46772..37d3eb09b3b 100644
--- a/linux-user/generic/target_resource.h
+++ b/linux-user/generic/target_resource.h
@@ -12,8 +12,8 @@ struct target_rlimit {
  };
  
  struct target_rlimit64 {

-uint64_t rlim_cur;
-uint64_t rlim_max;
+abi_ullong rlim_cur;
+abi_ullong rlim_max;
  };
  
  #define TARGET_RLIM_INFINITY((abi_ulong)-1)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index a6c426d73cf..73082531ffc 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -12886,8 +12886,8 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int 
num, abi_long arg1,
  if (!lock_user_struct(VERIFY_READ, target_rnew, arg3, 1)) {
  return -TARGET_EFAULT;
  }
-rnew.rlim_cur = tswap64(target_rnew->rlim_cur);
-rnew.rlim_max = tswap64(target_rnew->rlim_max);
+__get_user(rnew.rlim_cur, _rnew->rlim_cur);
+__get_user(rnew.rlim_max, _rnew->rlim_max);
  unlock_user_struct(target_rnew, arg3, 0);
  rnewp = 
  }
@@ -12897,8 +12897,8 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int 
num, abi_long arg1,
  if (!lock_user_struct(VERIFY_WRITE, target_rold, arg4, 1)) {
  return -TARGET_EFAULT;
  }
-target_rold->rlim_cur = tswap64(rold.rlim_cur);
-target_rold->rlim_max = tswap64(rold.rlim_max);
+__put_user(rold.rlim_cur, _rold->rlim_cur);
+__put_user(rold.rlim_max, _rold->rlim_max);
  unlock_user_struct(target_rold, arg4, 1);
  }
  return ret;


Applied to my linux-user-for-8.0 branch.

Thanks,
Laurent




Re: [PATCH v3 1/2] linux-user: Fix unaligned memory access in prlimit64 syscall

2023-03-06 Thread Laurent Vivier

Le 24/02/2023 à 01:39, Ilya Leoshkevich a écrit :

target_rlimit64 contains uint64_t fields, so it's 8-byte aligned on
some hosts, while some guests may align their respective type on a
4-byte boundary. This may lead to an unaligned access, which is an UB.

Fix by defining the fields as abi_ullong. This makes the host alignment
match that of the guest, and lets the compiler know that it should emit
code that can deal with the guest alignment.

While at it, also use __get_user() and __put_user() instead of
tswap64().

Fixes: 163a05a8398b ("linux-user: Implement prlimit64 syscall")
Reported-by: Richard Henderson 
Signed-off-by: Ilya Leoshkevich 
---
  linux-user/generic/target_resource.h | 4 ++--
  linux-user/syscall.c | 8 
  2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/linux-user/generic/target_resource.h 
b/linux-user/generic/target_resource.h
index 539d8c46772..37d3eb09b3b 100644
--- a/linux-user/generic/target_resource.h
+++ b/linux-user/generic/target_resource.h
@@ -12,8 +12,8 @@ struct target_rlimit {
  };
  
  struct target_rlimit64 {

-uint64_t rlim_cur;
-uint64_t rlim_max;
+abi_ullong rlim_cur;
+abi_ullong rlim_max;
  };
  
  #define TARGET_RLIM_INFINITY((abi_ulong)-1)

diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index a6c426d73cf..73082531ffc 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -12886,8 +12886,8 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int 
num, abi_long arg1,
  if (!lock_user_struct(VERIFY_READ, target_rnew, arg3, 1)) {
  return -TARGET_EFAULT;
  }
-rnew.rlim_cur = tswap64(target_rnew->rlim_cur);
-rnew.rlim_max = tswap64(target_rnew->rlim_max);
+__get_user(rnew.rlim_cur, _rnew->rlim_cur);
+__get_user(rnew.rlim_max, _rnew->rlim_max);
  unlock_user_struct(target_rnew, arg3, 0);
  rnewp = 
  }
@@ -12897,8 +12897,8 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int 
num, abi_long arg1,
  if (!lock_user_struct(VERIFY_WRITE, target_rold, arg4, 1)) {
  return -TARGET_EFAULT;
  }
-target_rold->rlim_cur = tswap64(rold.rlim_cur);
-target_rold->rlim_max = tswap64(rold.rlim_max);
+__put_user(rold.rlim_cur, _rold->rlim_cur);
+__put_user(rold.rlim_max, _rold->rlim_max);
  unlock_user_struct(target_rold, arg4, 1);
  }
  return ret;


Reviewed-by: Laurent Vivier 



Re: [PATCH v3 1/2] linux-user: Fix unaligned memory access in prlimit64 syscall

2023-02-23 Thread Philippe Mathieu-Daudé

On 24/2/23 01:39, Ilya Leoshkevich wrote:

target_rlimit64 contains uint64_t fields, so it's 8-byte aligned on
some hosts, while some guests may align their respective type on a
4-byte boundary. This may lead to an unaligned access, which is an UB.

Fix by defining the fields as abi_ullong. This makes the host alignment
match that of the guest, and lets the compiler know that it should emit
code that can deal with the guest alignment.

While at it, also use __get_user() and __put_user() instead of
tswap64().

Fixes: 163a05a8398b ("linux-user: Implement prlimit64 syscall")
Reported-by: Richard Henderson 
Signed-off-by: Ilya Leoshkevich 
---
  linux-user/generic/target_resource.h | 4 ++--
  linux-user/syscall.c | 8 
  2 files changed, 6 insertions(+), 6 deletions(-)


Reviewed-by: Philippe Mathieu-Daudé 




[PATCH v3 1/2] linux-user: Fix unaligned memory access in prlimit64 syscall

2023-02-23 Thread Ilya Leoshkevich
target_rlimit64 contains uint64_t fields, so it's 8-byte aligned on
some hosts, while some guests may align their respective type on a
4-byte boundary. This may lead to an unaligned access, which is an UB.

Fix by defining the fields as abi_ullong. This makes the host alignment
match that of the guest, and lets the compiler know that it should emit
code that can deal with the guest alignment.

While at it, also use __get_user() and __put_user() instead of
tswap64().

Fixes: 163a05a8398b ("linux-user: Implement prlimit64 syscall")
Reported-by: Richard Henderson 
Signed-off-by: Ilya Leoshkevich 
---
 linux-user/generic/target_resource.h | 4 ++--
 linux-user/syscall.c | 8 
 2 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/linux-user/generic/target_resource.h 
b/linux-user/generic/target_resource.h
index 539d8c46772..37d3eb09b3b 100644
--- a/linux-user/generic/target_resource.h
+++ b/linux-user/generic/target_resource.h
@@ -12,8 +12,8 @@ struct target_rlimit {
 };
 
 struct target_rlimit64 {
-uint64_t rlim_cur;
-uint64_t rlim_max;
+abi_ullong rlim_cur;
+abi_ullong rlim_max;
 };
 
 #define TARGET_RLIM_INFINITY((abi_ulong)-1)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index a6c426d73cf..73082531ffc 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -12886,8 +12886,8 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int 
num, abi_long arg1,
 if (!lock_user_struct(VERIFY_READ, target_rnew, arg3, 1)) {
 return -TARGET_EFAULT;
 }
-rnew.rlim_cur = tswap64(target_rnew->rlim_cur);
-rnew.rlim_max = tswap64(target_rnew->rlim_max);
+__get_user(rnew.rlim_cur, _rnew->rlim_cur);
+__get_user(rnew.rlim_max, _rnew->rlim_max);
 unlock_user_struct(target_rnew, arg3, 0);
 rnewp = 
 }
@@ -12897,8 +12897,8 @@ static abi_long do_syscall1(CPUArchState *cpu_env, int 
num, abi_long arg1,
 if (!lock_user_struct(VERIFY_WRITE, target_rold, arg4, 1)) {
 return -TARGET_EFAULT;
 }
-target_rold->rlim_cur = tswap64(rold.rlim_cur);
-target_rold->rlim_max = tswap64(rold.rlim_max);
+__put_user(rold.rlim_cur, _rold->rlim_cur);
+__put_user(rold.rlim_max, _rold->rlim_max);
 unlock_user_struct(target_rold, arg4, 1);
 }
 return ret;
-- 
2.39.1