All of lore.kernel.org
 help / color / mirror / Atom feed
From: Schspa Shi <schspa@gmail.com>
To: mcgrof@kernel.org
Cc: linux-kernel@vger.kernel.org, Schspa Shi <schspa@gmail.com>,
	syzbot+10d19d528d9755d9af22@syzkaller.appspotmail.com,
	syzbot+70d5d5d83d03db2c813d@syzkaller.appspotmail.com,
	syzbot+83cb0411d0fcf0a30fc1@syzkaller.appspotmail.com,
	syzbot+c92c6a251d49ceceb625@syzkaller.appspotmail.com
Subject: [PATCH v2] umh: fix out of scope usage when the process is being killed
Date: Wed, 14 Dec 2022 21:46:56 +0800	[thread overview]
Message-ID: <20221214134656.21228-1-schspa@gmail.com> (raw)

When the process is killed, wait_for_completion_state will return with
-ERESTARTSYS, and the completion variable in the stack will be unavailable,
even freed. If the user-mode thread is complete at the same time, there
will be a race to use a unavailable variable.

Please refer to the following scenarios.
            T1                                  T2
------------------------------------------------------------------
call_usermodehelper_exec
                                   call_usermodehelper_exec_async
                                   << do something >>
                                   umh_complete(sub_info);
                                   comp = xchg(&sub_info->complete, NULL);
                                   /* we got the completion */
                                   << context switch >>

    << Being killed >>
	retval = wait_for_completion_state(sub_info->complete, state);
	if (!retval)
		goto wait_done;

	if (wait & UMH_KILLABLE) {
		/* umh_complete() will see NULL and free sub_info */
		if (xchg(&sub_info->complete, NULL))
			goto unlock;
        << we can't got the completion, because T2 take it already >>
	}
	....
	return retval;
}

/**
 * the completion variable in stack is end of life cycle.
 * and maybe freed due to process is recycled.
 */
                                   -------- BUG here----------
                                   if (comp)
                                       complete(comp);

To fix it, we can add an additional wait_for_completion to ensure the
completion object is completely unused. And this is what
kthread_create_on_node does to handle this race.

Reported-by: syzbot+10d19d528d9755d9af22@syzkaller.appspotmail.com
Reported-by: syzbot+70d5d5d83d03db2c813d@syzkaller.appspotmail.com
Reported-by: syzbot+83cb0411d0fcf0a30fc1@syzkaller.appspotmail.com
Reported-by: syzbot+c92c6a251d49ceceb625@syzkaller.appspotmail.com
Signed-off-by: Schspa Shi <schspa@gmail.com>
---

v1->v2:
  - Use a new way to fix the race as kthread_create_on_node do.
  - Optimize comments and use more accurate words to describe the problem.

 kernel/umh.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/kernel/umh.c b/kernel/umh.c
index 850631518665..d8350a195c7f 100644
--- a/kernel/umh.c
+++ b/kernel/umh.c
@@ -452,6 +452,10 @@ int call_usermodehelper_exec(struct subprocess_info *sub_info, int wait)
 		/* umh_complete() will see NULL and free sub_info */
 		if (xchg(&sub_info->complete, NULL))
 			goto unlock;
+		/*
+		 * umh_complete will call complete() shortly.
+		 */
+		wait_for_completion(&done);
 	}
 
 wait_done:
-- 
2.37.3


             reply	other threads:[~2022-12-14 13:47 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-14 13:46 Schspa Shi [this message]
2022-12-14 19:59 ` [PATCH v2] umh: fix out of scope usage when the process is being killed Luis Chamberlain
2022-12-15  5:11   ` Schspa Shi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20221214134656.21228-1-schspa@gmail.com \
    --to=schspa@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=syzbot+10d19d528d9755d9af22@syzkaller.appspotmail.com \
    --cc=syzbot+70d5d5d83d03db2c813d@syzkaller.appspotmail.com \
    --cc=syzbot+83cb0411d0fcf0a30fc1@syzkaller.appspotmail.com \
    --cc=syzbot+c92c6a251d49ceceb625@syzkaller.appspotmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.