| Message ID | 87bd9251-5d6a-11f5-9a28-78224a776742@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | target/hppa: lock both words of function descriptor | expand |
On 9/16/23 15:52, Mikulas Patocka wrote: > The code in setup_rt_frame reads two words at haddr, but locks only one. > This patch fixes it to lock both. > > Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> > > --- > linux-user/hppa/signal.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > Index: qemu/linux-user/hppa/signal.c > =================================================================== > --- qemu.orig/linux-user/hppa/signal.c > +++ qemu/linux-user/hppa/signal.c > @@ -149,12 +149,11 @@ void setup_rt_frame(int sig, struct targ > target_ulong *fdesc, dest; > > haddr &= -4; > - if (!lock_user_struct(VERIFY_READ, fdesc, haddr, 1)) { > + if (!(fdesc = lock_user(VERIFY_READ, haddr, 2 * sizeof(target_ulong), 1))) > goto give_sigsegv; > - } Patch is Ok, but I think the qemu coding style is to keep the { } braces, even if they are unnecessary (as in this case). Acked-by: Helge Deller <deller@gmx.de> > __get_user(dest, fdesc); > __get_user(env->gr[19], fdesc + 1); > - unlock_user_struct(fdesc, haddr, 1); > + unlock_user(fdesc, haddr, 0); > haddr = dest; > } > env->iaoq_f = haddr; >
On 9/16/23 09:18, Helge Deller wrote: > On 9/16/23 15:52, Mikulas Patocka wrote: >> The code in setup_rt_frame reads two words at haddr, but locks only one. >> This patch fixes it to lock both. >> >> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> >> >> --- >> linux-user/hppa/signal.c | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) >> >> Index: qemu/linux-user/hppa/signal.c >> =================================================================== >> --- qemu.orig/linux-user/hppa/signal.c >> +++ qemu/linux-user/hppa/signal.c >> @@ -149,12 +149,11 @@ void setup_rt_frame(int sig, struct targ >> target_ulong *fdesc, dest; >> >> haddr &= -4; >> - if (!lock_user_struct(VERIFY_READ, fdesc, haddr, 1)) { >> + if (!(fdesc = lock_user(VERIFY_READ, haddr, 2 * sizeof(target_ulong), 1))) >> goto give_sigsegv; >> - } > > Patch is Ok, but I think the qemu coding style is to keep the { } braces, even > if they are unnecessary (as in this case). Coding style also separates the assignment from the if condition. r~ > > Acked-by: Helge Deller <deller@gmx.de> > >> __get_user(dest, fdesc); >> __get_user(env->gr[19], fdesc + 1); >> - unlock_user_struct(fdesc, haddr, 1); >> + unlock_user(fdesc, haddr, 0); >> haddr = dest; >> } >> env->iaoq_f = haddr; >> >
On 9/16/23 19:17, Richard Henderson wrote: > On 9/16/23 09:18, Helge Deller wrote: >> On 9/16/23 15:52, Mikulas Patocka wrote: >>> The code in setup_rt_frame reads two words at haddr, but locks only one. >>> This patch fixes it to lock both. >>> >>> Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> >>> >>> --- >>> linux-user/hppa/signal.c | 5 ++--- >>> 1 file changed, 2 insertions(+), 3 deletions(-) >>> >>> Index: qemu/linux-user/hppa/signal.c >>> =================================================================== >>> --- qemu.orig/linux-user/hppa/signal.c >>> +++ qemu/linux-user/hppa/signal.c >>> @@ -149,12 +149,11 @@ void setup_rt_frame(int sig, struct targ >>> target_ulong *fdesc, dest; >>> >>> haddr &= -4; >>> - if (!lock_user_struct(VERIFY_READ, fdesc, haddr, 1)) { >>> + if (!(fdesc = lock_user(VERIFY_READ, haddr, 2 * sizeof(target_ulong), 1))) >>> goto give_sigsegv; >>> - } >> >> Patch is Ok, but I think the qemu coding style is to keep the { } braces, even >> if they are unnecessary (as in this case). > > Coding style also separates the assignment from the if condition. I'll fix it up. Thanks! Helge > > r~ > >> >> Acked-by: Helge Deller <deller@gmx.de> >> >>> __get_user(dest, fdesc); >>> __get_user(env->gr[19], fdesc + 1); >>> - unlock_user_struct(fdesc, haddr, 1); >>> + unlock_user(fdesc, haddr, 0); >>> haddr = dest; >>> } >>> env->iaoq_f = haddr; >>> >> > >
Index: qemu/linux-user/hppa/signal.c =================================================================== --- qemu.orig/linux-user/hppa/signal.c +++ qemu/linux-user/hppa/signal.c @@ -149,12 +149,11 @@ void setup_rt_frame(int sig, struct targ target_ulong *fdesc, dest; haddr &= -4; - if (!lock_user_struct(VERIFY_READ, fdesc, haddr, 1)) { + if (!(fdesc = lock_user(VERIFY_READ, haddr, 2 * sizeof(target_ulong), 1))) goto give_sigsegv; - } __get_user(dest, fdesc); __get_user(env->gr[19], fdesc + 1); - unlock_user_struct(fdesc, haddr, 1); + unlock_user(fdesc, haddr, 0); haddr = dest; } env->iaoq_f = haddr;
The code in setup_rt_frame reads two words at haddr, but locks only one. This patch fixes it to lock both. Signed-off-by: Mikulas Patocka <mpatocka@redhat.com> --- linux-user/hppa/signal.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)