On 7 January 2013 20:30, Laurent Vivier <laur...@vivier.eu> wrote:
> According to man reboot(2), the 4th argument is only used with
> LINUX_REBOOT_CMD_RESTART2. In other cases, trying to convert
> the value can generate EFAULT.
>
> Signed-off-by: Laurent Vivier <laur...@vivier.eu>
> ---
> linux-user/syscall.c | 14 ++++++++++----
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 3167a87..730e428 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -101,6 +101,7 @@ int __clone2(int (*fn)(void *), void *child_stack_base,
> #include <linux/fb.h>
> #include <linux/vt.h>
> #include <linux/dm-ioctl.h>
> +#include <linux/reboot.h>
> #include "linux_loop.h"
> #include "cpu-uname.h"
>
> @@ -6415,10 +6416,15 @@ abi_long do_syscall(void *cpu_env, int num, abi_long
> arg1,
> break;
> #endif
> case TARGET_NR_reboot:
> - if (!(p = lock_user_string(arg4)))
> - goto efault;
> - ret = reboot(arg1, arg2, arg3, p);
> - unlock_user(p, arg4, 0);
> + if (arg3 == LINUX_REBOOT_CMD_RESTART2) {
> + /* arg4 must be ignored in all other cases */
> + if (!(p = lock_user_string(arg4)))
> + goto efault;
Coding style requires braces; please use checkpatch.pl.
> + ret = reboot(arg1, arg2, arg3, p);
> + unlock_user(p, arg4, 0);
> + } else {
> + ret = reboot(arg1, arg2, arg3, (void*)(unsigned long)arg4);
I don't think we should pass arg4 in this case. It's a pointer, so it's
definitely wrong to pass a pointer we haven't converted somehow.
Just passing NULL would be better, I think; that will be safe and
make it reasonably obvious we need to fix something if the kernel
ever for some reason adds a new command that takes an argument.
thanks
-- PMM
