Skip to contentSkip to conversation

linux-user: fix mq_getsetattr implementation

PATCH Superseded Max Filippov 2 participants 3 messages Mar 31, 2018

v1 of 3 Superseded Latest is v3 →

lore
Share & tools
lore

Max Filippov

author
#0 lore raw

PATCH 1/1 linux-user: fix mq_getsetattr implementation

mq_getsetattr implementation does not set errno correctly in case of error. Also in the presence of both 2nd and 3rd arguments it calls both mq_getattr and mq_setattr, whereas only the latter call would suffice.

Don't call mq_getattr in the presence of the 3rd argument. Use get_errno to set errno value.

This fixes test rt/tst-mqueue2 from the glibc testsuite.

Cc Lionel Landwerlin <[email protected]>
Cc Kirill A. Shutemov <[email protected]>
Cc Riku Voipio <[email protected]>
Cc Aurelien Jarno <[email protected]>
Cc Laurent Vivier <[email protected]>
Signed-off-by Max Filippov <[email protected]>
---
 linux-user/syscall.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
linux-user/syscall.c +7 -6
index 18ea79140f16..8b364551c311 100644
@@ -12092,15 +12092,16 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         {
             struct mq_attr posix_mq_attr_in, posix_mq_attr_out;
             ret = 0;
-            if (arg3 != 0) {
-                ret = mq_getattr(arg1, &posix_mq_attr_out);
-                copy_to_user_mq_attr(arg3, &posix_mq_attr_out);
-            }
             if (arg2 != 0) {
                 copy_from_user_mq_attr(&posix_mq_attr_in, arg2);
-                ret |= mq_setattr(arg1, &posix_mq_attr_in, &posix_mq_attr_out);
+                ret = mq_setattr(arg1, &posix_mq_attr_in, &posix_mq_attr_out);
+            } else if (arg3 != 0) {
+                ret = mq_getattr(arg1, &posix_mq_attr_out);
             }
-
+            if (arg3 != 0) {
+                copy_to_user_mq_attr(arg3, &posix_mq_attr_out);
+            }
+            ret = get_errno(ret);
         }
         break;
 #endif
Signature · 1 line
2.11.0
#1 lore raw

Le 31/03/2018 à 18:23, Max Filippov a écrit :

Quoted 24 lines

mq_getsetattr implementation does not set errno correctly in case of error. Also in the presence of both 2nd and 3rd arguments it calls both mq_getattr and mq_setattr, whereas only the latter call would suffice.

Don't call mq_getattr in the presence of the 3rd argument. Use get_errno to set errno value.

This fixes test rt/tst-mqueue2 from the glibc testsuite.

Cc Lionel Landwerlin <[email protected]>
Cc Kirill A. Shutemov <[email protected]>
Cc Riku Voipio <[email protected]>
Cc Aurelien Jarno <[email protected]>
Cc Laurent Vivier <[email protected]>
Signed-off-by Max Filippov <[email protected]>
---
 linux-user/syscall.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
linux-user/syscall.c +0 -0
index 18ea79140f16..8b364551c311 100644

linux-user/syscall.c abi_long do_syscall(void *cpu_env, int num, abi_long arg1, · line 12094

Quoted 3 lines
linux-user/syscall.c +0 -0
@@ -12092,15 +12092,16 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         {
             struct mq_attr posix_mq_attr_in, posix_mq_attr_out;
             ret = 0;

you can remove this line

linux-user/syscall.c abi_long do_syscall(void *cpu_env, int num, abi_long arg1, · line 12097

Quoted 8 lines
linux-user/syscall.c +1 -5
-            if (arg3 != 0) {
-                ret = mq_getattr(arg1, &posix_mq_attr_out);
-                copy_to_user_mq_attr(arg3, &posix_mq_attr_out);
-            }
             if (arg2 != 0) {
                 copy_from_user_mq_attr(&posix_mq_attr_in, arg2);
-                ret |= mq_setattr(arg1, &posix_mq_attr_in, &posix_mq_attr_out);
+                ret = mq_setattr(arg1, &posix_mq_attr_in, &posix_mq_attr_out);

you can call get_errno(mq_setattr(..)) here...

linux-user/syscall.c abi_long do_syscall(void *cpu_env, int num, abi_long arg1, · line 12099

Quoted 2 lines
linux-user/syscall.c +2 -0
+            } else if (arg3 != 0) {
+                ret = mq_getattr(arg1, &posix_mq_attr_out);

and get_errno(q_getattr(..)) here.

linux-user/syscall.c abi_long do_syscall(void *cpu_env, int num, abi_long arg1, · line 12101

Quoted 3 lines
linux-user/syscall.c +1 -1
             }
-
+            if (arg3 != 0) {

you should also check ret < 0 before copying to arg3

linux-user/syscall.c abi_long do_syscall(void *cpu_env, int num, abi_long arg1, · line 12104

Quoted 3 lines
linux-user/syscall.c +3 -0
+                copy_to_user_mq_attr(arg3, &posix_mq_attr_out);
+            }
+            ret = get_errno(ret);

you can remove this line if you call get_errno() directly before.

linux-user/syscall.c abi_long do_syscall(void *cpu_env, int num, abi_long arg1, · line 12107

Quoted 4 lines
linux-user/syscall.c +0 -0
         }
         break;
 #endif

Thanks,
Laurent

#2 lore raw

Le 31/03/2018 à 18:57, Laurent Vivier a écrit :

Quoted 20 lines

Le 31/03/2018 à 18:23, Max Filippov a écrit :

Quoted 19 lines

mq_getsetattr implementation does not set errno correctly in case of error. Also in the presence of both 2nd and 3rd arguments it calls both mq_getattr and mq_setattr, whereas only the latter call would suffice.

Don't call mq_getattr in the presence of the 3rd argument. Use get_errno to set errno value.

This fixes test rt/tst-mqueue2 from the glibc testsuite.

Cc Lionel Landwerlin <[email protected]>
Cc Kirill A. Shutemov <[email protected]>
Cc Riku Voipio <[email protected]>
Cc Aurelien Jarno <[email protected]>
Cc Laurent Vivier <[email protected]>
Signed-off-by Max Filippov <[email protected]>
---
 linux-user/syscall.c | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)
Quoted 10 lines

linux-user/syscall.c abi_long do_syscall(void *cpu_env, int num, abi_long arg1, · line 12094

Quoted 8 lines
linux-user/syscall.c +0 -0
index 18ea79140f16..8b364551c311 100644
@@ -12092,15 +12092,16 @@ abi_long do_syscall(void *cpu_env, int num, abi_long arg1,
         {
             struct mq_attr posix_mq_attr_in, posix_mq_attr_out;
             ret = 0;

you can remove this line

in fact, you must keep this line because ret is not update if arg2 == NULL and arg3 == NULL.

Thanks,
Laurent