* [PATCH] Fix stack corruption when handling PR_GETDEATHSIG
@ 2020-05-06 23:47 Stephen Long
2020-05-07 6:29 ` Philippe Mathieu-Daudé
2020-05-07 8:23 ` Laurent Vivier
0 siblings, 2 replies; 3+ messages in thread
From: Stephen Long @ 2020-05-06 23:47 UTC (permalink / raw)
To: qemu-devel; +Cc: pzheng, riku.voipio, efriedma, laurent, apazos
From: Ana Pazos <apazos@quicinc.com>
Signed-off-by: Ana Pazos <apazos@quicinc.com>
---
Submitting this patch on behalf of Ana Pazos. The bug was triggered by
the following c file on aarch64-linux-user.
> #include <signal.h>
> #include <sys/prctl.h>
>
> int main() {
> int PDeachSig = 0;
> if (prctl(PR_GET_PDEATHSIG, &PDeachSig) == 0 && PDeachSig == SIGKILL)
> prctl(PR_SET_PDEATHSIG, 0);
> return (PDeachSig == SIGKILL);
> }
linux-user/syscall.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/linux-user/syscall.c b/linux-user/syscall.c
index 05f03919ff..4eac567f97 100644
--- a/linux-user/syscall.c
+++ b/linux-user/syscall.c
@@ -10253,10 +10253,10 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
switch (arg1) {
case PR_GET_PDEATHSIG:
{
- int deathsig;
+ uint32_t deathsig;
ret = get_errno(prctl(arg1, &deathsig, arg3, arg4, arg5));
if (!is_error(ret) && arg2
- && put_user_ual(deathsig, arg2)) {
+ && put_user_u32(deathsig, arg2)) {
return -TARGET_EFAULT;
}
return ret;
--
2.17.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix stack corruption when handling PR_GETDEATHSIG
2020-05-06 23:47 [PATCH] Fix stack corruption when handling PR_GETDEATHSIG Stephen Long
@ 2020-05-07 6:29 ` Philippe Mathieu-Daudé
2020-05-07 8:23 ` Laurent Vivier
1 sibling, 0 replies; 3+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-05-07 6:29 UTC (permalink / raw)
To: Stephen Long, qemu-devel; +Cc: riku.voipio, efriedma, laurent, apazos, pzheng
Hi Stephen,
On 5/7/20 1:47 AM, Stephen Long wrote:
> From: Ana Pazos <apazos@quicinc.com>
>
> Signed-off-by: Ana Pazos <apazos@quicinc.com>
> ---
> Submitting this patch on behalf of Ana Pazos. The bug was triggered by
> the following c file on aarch64-linux-user.
This is fine, but you have to add your own S-o-b tag too.
See the link from
https://wiki.qemu.org/Contribute/SubmitAPatch#Patch_emails_must_include_a_Signed-off-by:_line
By making a contribution to this project, I certify that:
(c) The contribution was provided directly to me by some other
person who certified (a), (b) or (c) and I have not modified
it.
>
>> #include <signal.h>
>> #include <sys/prctl.h>
>>
>> int main() {
>> int PDeachSig = 0;
>> if (prctl(PR_GET_PDEATHSIG, &PDeachSig) == 0 && PDeachSig == SIGKILL)
>> prctl(PR_SET_PDEATHSIG, 0);
>> return (PDeachSig == SIGKILL);
>> }
>
> linux-user/syscall.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 05f03919ff..4eac567f97 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -10253,10 +10253,10 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
> switch (arg1) {
> case PR_GET_PDEATHSIG:
> {
> - int deathsig;
> + uint32_t deathsig;
I think the bug here is deathsig should be abi_long, the put_user_ual()
call seems fine. But I don't know well user-mode, so don't rely on my
comment :)
> ret = get_errno(prctl(arg1, &deathsig, arg3, arg4, arg5));
> if (!is_error(ret) && arg2
> - && put_user_ual(deathsig, arg2)) {
> + && put_user_u32(deathsig, arg2)) {
> return -TARGET_EFAULT;
> }
> return ret;
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fix stack corruption when handling PR_GETDEATHSIG
2020-05-06 23:47 [PATCH] Fix stack corruption when handling PR_GETDEATHSIG Stephen Long
2020-05-07 6:29 ` Philippe Mathieu-Daudé
@ 2020-05-07 8:23 ` Laurent Vivier
1 sibling, 0 replies; 3+ messages in thread
From: Laurent Vivier @ 2020-05-07 8:23 UTC (permalink / raw)
To: Stephen Long, qemu-devel; +Cc: riku.voipio, efriedma, apazos, pzheng
Le 07/05/2020 à 01:47, Stephen Long a écrit :
> From: Ana Pazos <apazos@quicinc.com>
>
> Signed-off-by: Ana Pazos <apazos@quicinc.com>
> ---
> Submitting this patch on behalf of Ana Pazos. The bug was triggered by
> the following c file on aarch64-linux-user.
>
>> #include <signal.h>
>> #include <sys/prctl.h>
>>
>> int main() {
>> int PDeachSig = 0;
>> if (prctl(PR_GET_PDEATHSIG, &PDeachSig) == 0 && PDeachSig == SIGKILL)
>> prctl(PR_SET_PDEATHSIG, 0);
>> return (PDeachSig == SIGKILL);
>> }
Put the example in the description of the patch, it will help to
understand the fix by being kept in the log.
> linux-user/syscall.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/linux-user/syscall.c b/linux-user/syscall.c
> index 05f03919ff..4eac567f97 100644
> --- a/linux-user/syscall.c
> +++ b/linux-user/syscall.c
> @@ -10253,10 +10253,10 @@ static abi_long do_syscall1(void *cpu_env, int num, abi_long arg1,
> switch (arg1) {
> case PR_GET_PDEATHSIG:
> {
> - int deathsig;
> + uint32_t deathsig;
> ret = get_errno(prctl(arg1, &deathsig, arg3, arg4, arg5));
> if (!is_error(ret) && arg2
> - && put_user_ual(deathsig, arg2)) {
> + && put_user_u32(deathsig, arg2)) {
> return -TARGET_EFAULT;
> }
> return ret;
>
I think you could keep the "int" and only replace put_user_ual() by
put_user_s32() (to stay in line with the man page that mentions "(int
*)". "int" is always a signed 32bit..
And add your "Signed-off-by".
Thanks,
Laurent
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-05-07 8:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-06 23:47 [PATCH] Fix stack corruption when handling PR_GETDEATHSIG Stephen Long
2020-05-07 6:29 ` Philippe Mathieu-Daudé
2020-05-07 8:23 ` Laurent Vivier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).