From: Petr Mladek <pmladek@suse.com>
To: Rik van Riel <riel@surriel.com>
Cc: Josh Poimboeuf <jpoimboe@kernel.org>,
linux-kernel@vger.kernel.org, live-patching@vger.kernel.org,
kernel-team@fb.com, Jiri Kosina <jikos@kernel.org>,
Miroslav Benes <mbenes@suse.cz>,
Joe Lawrence <joe.lawrence@redhat.com>,
Breno Leitao <leitao@debian.org>
Subject: Re: [PATCH v4] livepatch: fix race between fork and KLP transition
Date: Thu, 4 Aug 2022 15:06:21 +0200 [thread overview]
Message-ID: <YuvETeDi56Uv22nS@alley> (raw)
In-Reply-To: <c04f97fc29c4618f137b27ce6537800b53f1d95f.camel@surriel.com>
On Tue 2022-08-02 16:07:08, Rik van Riel wrote:
> On Thu, 2022-07-28 at 17:37 +0200, Petr Mladek wrote:
> > On Wed 2022-07-27 10:24:37, Rik van Riel wrote:
> > > v4: address changelog comments by Josh (thank you)
> > >
> > > ---8<---
> > > When a KLP fails to apply, klp_reverse_transition will clear the
> > > TIF_PATCH_PENDING flag on all tasks, except for newly created tasks
> > > which are not on the task list yet.
> >
> > It actually is not true. klp_reverse_transtion() clears
> > TIF_PATCH_FLAG only
> > temporary when it waits until all processes leave the ftrace
> > handler. It sets TIF_PATCH_FLAG once again for all tasks by calling
> > klp_start_transition().
> >
> > The difference is important. The WARN_ON_ONCE() in
> > klp_complete_transition() will be printed when fork() copied
> > TIF_PATCH_FLAG before it was set again.
> >
> > Anyway, the important thing is that TIF_PATCH_FLAG and task-
> > >patch_state
> > might be incompatible because fork() copies them at different times.
> >
> > klp_copy_process() must make sure that they are in sync. And
> > it must be done under tasklist_lock when the child is added
> > to the global task list.
>
> Hmmm, how should this be addressed in the changelog?
>
> Should I just remove most of that paragraph and leave it
> at "there can be a race"?
It would be nice to somehow summarize what I wrote. I mean to explain
why the problem is easier to see with revert and not with forward
transition.
It is because TIF_PATCH_FLAG might stay cleared in the child even
when it was set again in the parent by the klp_revert_transtion().
As a result, the child will never get transition back to the reverted
state.
The problem is hard to hit during the forward transition because
child might have TIF_PATCH_FLAG still set even when
it might later copy an already migrated task->patch_state
when parent gets migrated in the race window. In this case,
the TIF_PATCH_FLAG will get cleared when the child returns
from fork and all will be good.
In each case, the inconsistent state is there even during
the forward transition. But it would be caught only when
the entire transition is finished during the rather small
race window.
The patch should fix the race in any direction.
I could provide even better description after I am back
from vacation on Aug 22.
Best Regards,
Petr
prev parent reply other threads:[~2022-08-04 13:06 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-07-20 16:10 [PATCH,RFC] livepatch: fix race between fork and klp_reverse_transition Rik van Riel
2022-07-21 23:23 ` Song Liu
2022-07-22 15:30 ` Petr Mladek
2022-07-22 19:01 ` [PATCH v2] " Rik van Riel
2022-07-25 13:32 ` Petr Mladek
2022-07-25 13:49 ` [PATCH v3] " Rik van Riel
2022-07-27 0:10 ` Josh Poimboeuf
2022-07-27 0:26 ` Rik van Riel
2022-07-28 15:20 ` Petr Mladek
2022-07-27 14:24 ` [PATCH v4] livepatch: fix race between fork and KLP transition Rik van Riel
2022-07-28 15:37 ` Petr Mladek
2022-08-02 20:07 ` Rik van Riel
2022-08-04 13:06 ` Petr Mladek [this message]
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=YuvETeDi56Uv22nS@alley \
--to=pmladek@suse.com \
--cc=jikos@kernel.org \
--cc=joe.lawrence@redhat.com \
--cc=jpoimboe@kernel.org \
--cc=kernel-team@fb.com \
--cc=leitao@debian.org \
--cc=linux-kernel@vger.kernel.org \
--cc=live-patching@vger.kernel.org \
--cc=mbenes@suse.cz \
--cc=riel@surriel.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 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 read-only IMAP folder(s) and NNTP newsgroup(s).