From: Eric W. Biederman <hidden> Date: 2020-03-25 15:13:12
Bernd Edlinger [off-list ref] writes:
This is an infrastructure change that makes way for fixing this issue.
Each patch was already posted previously so this is just a cleanup of
the original mailing list thread(s) which got out of control by now.
Everything started here:
https://lore.kernel.org/lkml/AM6PR03MB5170B06F3A2B75EFB98D071AE4E60@AM6PR03MB5170.eurprd03.prod.outlook.com/
I added reviewed-by tags from the mailing list threads, except when
withdrawn.
It took a lot longer than expected to collect everything from the
mailinglist threads, since several commit messages have been infected
with typos, and they got fixed without a new patch version.
- Correct the point of no return.
- Add two new mutexes to replace cred_guard_mutex.
- Fix each use of cred_guard_mutex.
- Update documentation.
- Add a test case.
Bernd Edlinger (11):
exec: Fix a deadlock in strace
selftests/ptrace: add test cases for dead-locks
mm: docs: Fix a comment in process_vm_rw_core
kernel: doc: remove outdated comment cred.c
kernel/kcmp.c: Use new infrastructure to fix deadlocks in execve
proc: Use new infrastructure to fix deadlocks in execve
proc: io_accounting: Use new infrastructure to fix deadlocks in execve
perf: Use new infrastructure to fix deadlocks in execve
pidfd: Use new infrastructure to fix deadlocks in execve
exec: Fix dead-lock in de_thread with ptrace_attach
doc: Update documentation of ->exec_*_mutex
Eric W. Biederman (5):
exec: Only compute current once in flush_old_exec
exec: Factor unshare_sighand out of de_thread and call it separately
exec: Move cleanup of posix timers on exec out of de_thread
exec: Move exec_mmap right after de_thread in flush_old_exec
exec: Add exec_update_mutex to replace cred_guard_mutex
Documentation/security/credentials.rst | 29 +++++--
fs/exec.c | 122 ++++++++++++++++++++++--------
fs/proc/base.c | 23 +++---
include/linux/binfmts.h | 8 +-
include/linux/sched/signal.h | 17 ++++-
init/init_task.c | 3 +-
kernel/cred.c | 4 +-
kernel/events/core.c | 12 +--
kernel/fork.c | 7 +-
kernel/kcmp.c | 8 +-
kernel/pid.c | 4 +-
kernel/ptrace.c | 20 ++++-
kernel/seccomp.c | 15 ++--
mm/process_vm_access.c | 2 +-
tools/testing/selftests/ptrace/Makefile | 4 +-
tools/testing/selftests/ptrace/vmaccess.c | 86 +++++++++++++++++++++
16 files changed, 278 insertions(+), 86 deletions(-)
create mode 100644 tools/testing/selftests/ptrace/vmaccess.c
Two small nits.
- You reposted my patches with adding your signed-off-by
- You reposted my patches and did not include a "From:"
in the body so "git am" listed you as the author.
I have fixed those up and will be merging this code to linux-next,
unless you object.
Eric
This is an infrastructure change that makes way for fixing this issue.
Each patch was already posted previously so this is just a cleanup of
the original mailing list thread(s) which got out of control by now.
Everything started here:
https://lore.kernel.org/lkml/AM6PR03MB5170B06F3A2B75EFB98D071AE4E60@AM6PR03MB5170.eurprd03.prod.outlook.com/
I added reviewed-by tags from the mailing list threads, except when
withdrawn.
It took a lot longer than expected to collect everything from the
mailinglist threads, since several commit messages have been infected
with typos, and they got fixed without a new patch version.
- Correct the point of no return.
- Add two new mutexes to replace cred_guard_mutex.
- Fix each use of cred_guard_mutex.
- Update documentation.
- Add a test case.
Bernd Edlinger (11):
exec: Fix a deadlock in strace
selftests/ptrace: add test cases for dead-locks
mm: docs: Fix a comment in process_vm_rw_core
kernel: doc: remove outdated comment cred.c
kernel/kcmp.c: Use new infrastructure to fix deadlocks in execve
proc: Use new infrastructure to fix deadlocks in execve
proc: io_accounting: Use new infrastructure to fix deadlocks in execve
perf: Use new infrastructure to fix deadlocks in execve
pidfd: Use new infrastructure to fix deadlocks in execve
exec: Fix dead-lock in de_thread with ptrace_attach
doc: Update documentation of ->exec_*_mutex
Eric W. Biederman (5):
exec: Only compute current once in flush_old_exec
exec: Factor unshare_sighand out of de_thread and call it separately
exec: Move cleanup of posix timers on exec out of de_thread
exec: Move exec_mmap right after de_thread in flush_old_exec
exec: Add exec_update_mutex to replace cred_guard_mutex
Documentation/security/credentials.rst | 29 +++++--
fs/exec.c | 122 ++++++++++++++++++++++--------
fs/proc/base.c | 23 +++---
include/linux/binfmts.h | 8 +-
include/linux/sched/signal.h | 17 ++++-
init/init_task.c | 3 +-
kernel/cred.c | 4 +-
kernel/events/core.c | 12 +--
kernel/fork.c | 7 +-
kernel/kcmp.c | 8 +-
kernel/pid.c | 4 +-
kernel/ptrace.c | 20 ++++-
kernel/seccomp.c | 15 ++--
mm/process_vm_access.c | 2 +-
tools/testing/selftests/ptrace/Makefile | 4 +-
tools/testing/selftests/ptrace/vmaccess.c | 86 +++++++++++++++++++++
16 files changed, 278 insertions(+), 86 deletions(-)
create mode 100644 tools/testing/selftests/ptrace/vmaccess.c
Two small nits.
- You reposted my patches with adding your signed-off-by
- You reposted my patches and did not include a "From:"
in the body so "git am" listed you as the author.
I have fixed those up and will be merging this code to linux-next,
unless you object.
Thanks, I have not expected that a From: which names a different domain
than hotmail.de would be forwarded by the SMTP servers I have to use.
Actually I was not even aware of the problem at all.
The Patch "exec: Add exec_update_mutex to replace cred_guard_mutex"
was initially reviewed-by: bernd.edlinger@hotmail.de but it turned out
to be faulty, so I update your patch faithfully, and did a small change
to fix the patch, therefore it is actually 99% your and 1% my patch,
therefore I figured I should be in a signed-off-by: together with you.
BTW: I saw a Reviewed-by: Kirill Tkhai [off-list ref] on the mailing list,
you should add that as well.
Thanks
Bernd.
This is an infrastructure change that makes way for fixing this issue.
Each patch was already posted previously so this is just a cleanup of
the original mailing list thread(s) which got out of control by now.
Everything started here:
https://lore.kernel.org/lkml/AM6PR03MB5170B06F3A2B75EFB98D071AE4E60@AM6PR03MB5170.eurprd03.prod.outlook.com/
I added reviewed-by tags from the mailing list threads, except when
withdrawn.
It took a lot longer than expected to collect everything from the
mailinglist threads, since several commit messages have been infected
with typos, and they got fixed without a new patch version.
- Correct the point of no return.
- Add two new mutexes to replace cred_guard_mutex.
- Fix each use of cred_guard_mutex.
- Update documentation.
- Add a test case.
Bernd Edlinger (11):
exec: Fix a deadlock in strace
selftests/ptrace: add test cases for dead-locks
mm: docs: Fix a comment in process_vm_rw_core
kernel: doc: remove outdated comment cred.c
kernel/kcmp.c: Use new infrastructure to fix deadlocks in execve
proc: Use new infrastructure to fix deadlocks in execve
proc: io_accounting: Use new infrastructure to fix deadlocks in execve
perf: Use new infrastructure to fix deadlocks in execve
pidfd: Use new infrastructure to fix deadlocks in execve
exec: Fix dead-lock in de_thread with ptrace_attach
doc: Update documentation of ->exec_*_mutex
Eric W. Biederman (5):
exec: Only compute current once in flush_old_exec
exec: Factor unshare_sighand out of de_thread and call it separately
exec: Move cleanup of posix timers on exec out of de_thread
exec: Move exec_mmap right after de_thread in flush_old_exec
exec: Add exec_update_mutex to replace cred_guard_mutex
Documentation/security/credentials.rst | 29 +++++--
fs/exec.c | 122 ++++++++++++++++++++++--------
fs/proc/base.c | 23 +++---
include/linux/binfmts.h | 8 +-
include/linux/sched/signal.h | 17 ++++-
init/init_task.c | 3 +-
kernel/cred.c | 4 +-
kernel/events/core.c | 12 +--
kernel/fork.c | 7 +-
kernel/kcmp.c | 8 +-
kernel/pid.c | 4 +-
kernel/ptrace.c | 20 ++++-
kernel/seccomp.c | 15 ++--
mm/process_vm_access.c | 2 +-
tools/testing/selftests/ptrace/Makefile | 4 +-
tools/testing/selftests/ptrace/vmaccess.c | 86 +++++++++++++++++++++
16 files changed, 278 insertions(+), 86 deletions(-)
create mode 100644 tools/testing/selftests/ptrace/vmaccess.c
Two small nits.
- You reposted my patches with adding your signed-off-by
- You reposted my patches and did not include a "From:"
in the body so "git am" listed you as the author.
Oh, do I understand you right, that I can add a From: in the
*body* of the mail, and then the From: in the MIME header part
which I cannot change is ignored, so I can make you the author?
Thanks
Bernd.
I have fixed those up and will be merging this code to linux-next,
unless you object.
Eric
On Sat, Mar 28, 2020 at 11:32:35PM +0100, Bernd Edlinger wrote:
Oh, do I understand you right, that I can add a From: in the
*body* of the mail, and then the From: in the MIME header part
which I cannot change is ignored, so I can make you the author?
Correct. (If you use "git send-email" it'll do this automatically.)
e.g., trimmed from my workflow:
git format-patch -n --to "$to" --cover-letter -o outgoing/ \
--subject-prefix "PATCH v$version" "$SHA"
edit outgoing/0000-*
git send-email --transfer-encoding=8bit --8bit-encoding=UTF-8 \
--from="$ME" --to="$to" --cc="$ME" --cc="...more..." outgoing/*
--
Kees Cook
On Sat, Mar 28, 2020 at 11:32:35PM +0100, Bernd Edlinger wrote:
quoted
Oh, do I understand you right, that I can add a From: in the
*body* of the mail, and then the From: in the MIME header part
which I cannot change is ignored, so I can make you the author?
Correct. (If you use "git send-email" it'll do this automatically.)
e.g., trimmed from my workflow:
git format-patch -n --to "$to" --cover-letter -o outgoing/ \
--subject-prefix "PATCH v$version" "$SHA"
edit outgoing/0000-*
git send-email --transfer-encoding=8bit --8bit-encoding=UTF-8 \
--from="$ME" --to="$to" --cc="$ME" --cc="...more..." outgoing/*
Okay, thanks, I see that is very helpful information for me, and in
this case I had also fixed a small bug in one of Eric's patches, which
was initially overlooked (aquiring mutexes in wrong order,
releasing an unlocked mutex in some error paths).
I am completely unexperienced, and something that complex was not
expected to happen :-) so this is just to make sure I can handle it
correctly if something like this happens again.
In the case of PATCH v6 05/16 I removed the Reviewd-by: Bernd Edlinger
since it is now somehow two authors and reviewing own code is obviously
not ok, instead I added a Signed-off-by: Bernd Edlinger (and posted the
whole series on Eric's behalf (after asking Eric's permissing per off-list
e-mail, which probably ended in his spam folder)
Is this having two Signed-off-by: for mutliple authors the
correct way to handle a shared authorship?
Thanks
Bernd.
From: Matthew Wilcox <willy@infradead.org> Date: 2020-03-30 20:15:48
On Mon, Mar 30, 2020 at 10:12:02PM +0200, Bernd Edlinger wrote:
On 3/29/20 5:44 AM, Kees Cook wrote:
quoted
On Sat, Mar 28, 2020 at 11:32:35PM +0100, Bernd Edlinger wrote:
quoted
Oh, do I understand you right, that I can add a From: in the
*body* of the mail, and then the From: in the MIME header part
which I cannot change is ignored, so I can make you the author?
Correct. (If you use "git send-email" it'll do this automatically.)
e.g., trimmed from my workflow:
git format-patch -n --to "$to" --cover-letter -o outgoing/ \
--subject-prefix "PATCH v$version" "$SHA"
edit outgoing/0000-*
git send-email --transfer-encoding=8bit --8bit-encoding=UTF-8 \
--from="$ME" --to="$to" --cc="$ME" --cc="...more..." outgoing/*
Okay, thanks, I see that is very helpful information for me, and in
this case I had also fixed a small bug in one of Eric's patches, which
was initially overlooked (aquiring mutexes in wrong order,
releasing an unlocked mutex in some error paths).
I am completely unexperienced, and something that complex was not
expected to happen :-) so this is just to make sure I can handle it
correctly if something like this happens again.
In the case of PATCH v6 05/16 I removed the Reviewd-by: Bernd Edlinger
since it is now somehow two authors and reviewing own code is obviously
not ok, instead I added a Signed-off-by: Bernd Edlinger (and posted the
whole series on Eric's behalf (after asking Eric's permissing per off-list
e-mail, which probably ended in his spam folder)
Is this having two Signed-off-by: for mutliple authors the
correct way to handle a shared authorship?
If the patch comes through you, then Reviewed-by: is inappropriate.
Instead, you should use Signed-off-by: in the second sense of
Documentation/process/submitting-patches.rst
This also documents how to handle "minor changes" that you make.
On Mon, Mar 30, 2020 at 01:14:59PM -0700, Matthew Wilcox wrote:
On Mon, Mar 30, 2020 at 10:12:02PM +0200, Bernd Edlinger wrote:
quoted
On 3/29/20 5:44 AM, Kees Cook wrote:
quoted
On Sat, Mar 28, 2020 at 11:32:35PM +0100, Bernd Edlinger wrote:
quoted
Oh, do I understand you right, that I can add a From: in the
*body* of the mail, and then the From: in the MIME header part
which I cannot change is ignored, so I can make you the author?
Correct. (If you use "git send-email" it'll do this automatically.)
e.g., trimmed from my workflow:
git format-patch -n --to "$to" --cover-letter -o outgoing/ \
--subject-prefix "PATCH v$version" "$SHA"
edit outgoing/0000-*
git send-email --transfer-encoding=8bit --8bit-encoding=UTF-8 \
--from="$ME" --to="$to" --cc="$ME" --cc="...more..." outgoing/*
Okay, thanks, I see that is very helpful information for me, and in
this case I had also fixed a small bug in one of Eric's patches, which
was initially overlooked (aquiring mutexes in wrong order,
releasing an unlocked mutex in some error paths).
I am completely unexperienced, and something that complex was not
expected to happen :-) so this is just to make sure I can handle it
correctly if something like this happens again.
In the case of PATCH v6 05/16 I removed the Reviewd-by: Bernd Edlinger
since it is now somehow two authors and reviewing own code is obviously
not ok, instead I added a Signed-off-by: Bernd Edlinger (and posted the
whole series on Eric's behalf (after asking Eric's permissing per off-list
e-mail, which probably ended in his spam folder)
Is this having two Signed-off-by: for mutliple authors the
correct way to handle a shared authorship?
If the patch comes through you, then Reviewed-by: is inappropriate.
Instead, you should use Signed-off-by: in the second sense of
Documentation/process/submitting-patches.rst
This also documents how to handle "minor changes" that you make.
On Mon, Mar 30, 2020 at 01:14:59PM -0700, Matthew Wilcox wrote:
quoted
On Mon, Mar 30, 2020 at 10:12:02PM +0200, Bernd Edlinger wrote:
quoted
On 3/29/20 5:44 AM, Kees Cook wrote:
quoted
On Sat, Mar 28, 2020 at 11:32:35PM +0100, Bernd Edlinger wrote:
quoted
Oh, do I understand you right, that I can add a From: in the
*body* of the mail, and then the From: in the MIME header part
which I cannot change is ignored, so I can make you the author?
Correct. (If you use "git send-email" it'll do this automatically.)
e.g., trimmed from my workflow:
git format-patch -n --to "$to" --cover-letter -o outgoing/ \
--subject-prefix "PATCH v$version" "$SHA"
edit outgoing/0000-*
git send-email --transfer-encoding=8bit --8bit-encoding=UTF-8 \
--from="$ME" --to="$to" --cc="$ME" --cc="...more..." outgoing/*
Okay, thanks, I see that is very helpful information for me, and in
this case I had also fixed a small bug in one of Eric's patches, which
was initially overlooked (aquiring mutexes in wrong order,
releasing an unlocked mutex in some error paths).
I am completely unexperienced, and something that complex was not
expected to happen :-) so this is just to make sure I can handle it
correctly if something like this happens again.
In the case of PATCH v6 05/16 I removed the Reviewd-by: Bernd Edlinger
since it is now somehow two authors and reviewing own code is obviously
not ok, instead I added a Signed-off-by: Bernd Edlinger (and posted the
whole series on Eric's behalf (after asking Eric's permissing per off-list
e-mail, which probably ended in his spam folder)
Is this having two Signed-off-by: for mutliple authors the
correct way to handle a shared authorship?
If the patch comes through you, then Reviewed-by: is inappropriate.
Instead, you should use Signed-off-by: in the second sense of
Documentation/process/submitting-patches.rst
This also documents how to handle "minor changes" that you make.