Check that access is OK before doing put_user_*

Signed-off-by: Taylor Simpson <tsimpson@quicinc.com>
---
 target/hexagon/op_helper.c | 23 ++++++++++++++---------
 1 file changed, 14 insertions(+), 9 deletions(-)

diff --git a/target/hexagon/op_helper.c b/target/hexagon/op_helper.c
index 4595559..d7f53a2 100644
--- a/target/hexagon/op_helper.c
+++ b/target/hexagon/op_helper.c
@@ -140,22 +140,27 @@ void HELPER(debug_check_store_width)(CPUHexagonState *env, int slot, int check)
 
 void HELPER(commit_store)(CPUHexagonState *env, int slot_num)
 {
-    switch (env->mem_log_stores[slot_num].width) {
+    uint8_t width = env->mem_log_stores[slot_num].width;
+    target_ulong va = env->mem_log_stores[slot_num].va;
+
+#ifdef CONFIG_USER_ONLY
+    g_assert(width == 1 || width == 2 || width == 4 || width == 8);
+    /* We perform this check elsewhere in system mode */
+    probe_write(env, va, width, MMU_USER_IDX, 0);
+#endif
+
+    switch (width) {
     case 1:
-        put_user_u8(env->mem_log_stores[slot_num].data32,
-                    env->mem_log_stores[slot_num].va);
+        put_user_u8(env->mem_log_stores[slot_num].data32, va);
         break;
     case 2:
-        put_user_u16(env->mem_log_stores[slot_num].data32,
-                     env->mem_log_stores[slot_num].va);
+        put_user_u16(env->mem_log_stores[slot_num].data32, va);
         break;
     case 4:
-        put_user_u32(env->mem_log_stores[slot_num].data32,
-                     env->mem_log_stores[slot_num].va);
+        put_user_u32(env->mem_log_stores[slot_num].data32, va);
         break;
     case 8:
-        put_user_u64(env->mem_log_stores[slot_num].data64,
-                     env->mem_log_stores[slot_num].va);
+        put_user_u64(env->mem_log_stores[slot_num].data64, va);
         break;
     default:
         g_assert_not_reached();
-- 
2.7.4

On 7/13/21 12:46 PM, Taylor Simpson wrote:
>   void HELPER(commit_store)(CPUHexagonState *env, int slot_num)
>   {
> -    switch (env->mem_log_stores[slot_num].width) {
> +    uint8_t width = env->mem_log_stores[slot_num].width;
> +    target_ulong va = env->mem_log_stores[slot_num].va;
> +
> +#ifdef CONFIG_USER_ONLY
> +    g_assert(width == 1 || width == 2 || width == 4 || width == 8);
> +    /* We perform this check elsewhere in system mode */
> +    probe_write(env, va, width, MMU_USER_IDX, 0);
> +#endif
> +
> +    switch (width) {
>       case 1:
> -        put_user_u8(env->mem_log_stores[slot_num].data32,
> -                    env->mem_log_stores[slot_num].va);
> +        put_user_u8(env->mem_log_stores[slot_num].data32, va);

The primary problem here is that put_user_* is the wrong set of functions to use.  You 
should have been using exec/cpu_ldst.h, in particular cpu_ld*_data_ra and cpu_st*_data_ra.


r~