[PATCH 1/1] set wo_stat to an init value in do_wait function

[PATCH 1/1] set wo_stat to an init value in do_wait function

[hank]

>From 23323ae46453f506df6647715042483548ea149e Mon Sep 17 00:00:00 2001
From: hank <pyu@redhat.com>
Date: Wed, 2 Nov 2011 15:28:58 +0800
Subject: [PATCH 1/1] set wo_stat to an init value in do_wait function

When all of the below conditions become true:
1 parent fork a child
2 parent ignore SIGCHLD signal
3 parent call waitpid function
do_wait function won't touch the wo->stat variable.

Below is a test program can reproduce this problem:
========================================================

int main(int argc, char *argv[])
{
        int pid, child;
        int status;
        int *p;

        signal(SIGCHLD, SIG_IGN);

        child = fork();
        if (child == 0) {
                sleep(1);
                exit(0);
        } else if (child < 0) {
                perror("fork");
                exit(1);
        } else {
                status = 0xa5a5;
                p = &status;
                printf("status addr: %p\n", p);
                pid = waitpid(-1, &status, WUNTRACED);
                printf("pid=%d status=0x%x\n", pid, status);
                exit(0);
        }
        return 0;
}
========================================================

After run this program, we can see the value of status is still
0xa5a5,so kernel do not touch this value.
It may be dangerous. Because lots of programs such as 'su' don't set
an init value for the variable 'status' when it call waitpid function,
and after the waitpid function return, the program may check the value
of 'status' to see the state of child. If kernel don't set a value to
'status', it may be a random value.
Of course, it only happens when the father program ignore SIGCHLD
signal, and the father should not ignore this signal if it want to
check the state of its child. But maybe some other programs let the
father program ignore the SIGCHLD signal. For example, the grandfather
program ignore SIGCHLD signal and it fork the father program, so the
father program ignore SIGCHLD signal...

Signed-off-by: hank <pyu@redhat.com>
---
 kernel/exit.c |    6 ++++++
 1 files changed, 6 insertions(+), 0 deletions(-)

diff --git a/kernel/exit.c b/kernel/exit.c
index d0b7d98..972f5ae 100644
--- a/kernel/exit.c
+++ b/kernel/exit.c
@@ -1683,6 +1683,12 @@ static long do_wait(struct wait_opts *wo)
 
 	trace_sched_process_wait(wo->wo_pid);
 
+	if (wo->wo_stat) {
+		retval = put_user(0, wo->wo_stat);
+		if (unlikely(retval))
+			return retval;
+	}
+
 	init_waitqueue_func_entry(&wo->child_wait, child_wait_callback);
 	wo->child_wait.private = current;
 	add_wait_queue(&current->signal->wait_chldexit, &wo->child_wait);
-- 
1.7.4.4

Re: [PATCH 1/1] set wo_stat to an init value in do_wait function

[Oleg Nesterov]

On 11/02, hank wrote:
>
> When all of the below conditions become true:
> 1 parent fork a child
> 2 parent ignore SIGCHLD signal
> 3 parent call waitpid function
> do_wait function won't touch the wo->stat variable.

Of course it doesn't, do_wait() fails and does nothing.

The parent ignores SIGCHLD. In this case waitpid(&status) acts as
if there are no children. Except it sleeps.

IOW,

> int main(int argc, char *argv[])
> {
>         int pid, child;
>         int status;
>         int *p;
> 
>         signal(SIGCHLD, SIG_IGN);
> 
>         child = fork();
>         if (child == 0) {
>                 sleep(1);
>                 exit(0);
>         } else if (child < 0) {
>                 perror("fork");
>                 exit(1);
>         } else {
>                 status = 0xa5a5;
>                 p = &status;
>                 printf("status addr: %p\n", p);
>                 pid = waitpid(-1, &status, WUNTRACED);
>                 printf("pid=%d status=0x%x\n", pid, status);
>                 exit(0);
>         }
>         return 0;
> }
> ========================================================
>
> After run this program, we can see the value of status is still
> 0xa5a5,so kernel do not touch this value.

Sure, this is correct.

> It may be dangerous. Because lots of programs such as 'su' don't set
> an init value for the variable 'status' when it call waitpid function,
> and after the waitpid function return, the program may check the value
> of 'status' to see the state of child.

Then this program is buggy. Once again, waitpid() fails. The program
shouldn't look at status at all.

Oleg.