mbox series

[0/2] SA_IMMUTABLE fixes

Message ID 87h7c9qg7p.fsf_-_@email.froward.int.ebiederm.org (mailing list archive)
Headers show
Series SA_IMMUTABLE fixes | expand

Message

Eric W. Biederman Nov. 18, 2021, 9:58 p.m. UTC
SA_IMMUTABLE fixed issues with force_sig_seccomp and the introduction
for force_sig_fatal where the exit previously could not be interrupted
but now it can.  Unfortunately it added that behavior to all force_sig
functions under the right conditions which debuggers usage of SIG_TRAP
and debuggers handling of SIGSEGV.

Solve that by limiting SA_IMMUTABLE to just the cases that historically
debuggers have not been able to intercept.

The first patch changes force_sig_info_to_task to take a flag
that requests which behavior is desired.

The second patch adds force_exit_sig which replaces force_fatal_sig
in the cases where historically userspace would only find out about
the ``signal'' after the process has exited.

The first one with the hunk changing force_fatal_sig removed should be
suitable for backporting to v5.15. v5.15 does not implement
force_fatal_sig.

This should be enough to fix the regressions.

Kyle if you can double check me that I have properly fixed these issues
that would be appreciated.

Any other review or suggestions to improve the names would be
appreciated.  I think I have named things reasonably well but I am very
close to the code so it is easy for me to miss things.

Eric W. Biederman (2):
      signal: Don't always set SA_IMMUTABLE for forced signals
      signal: Replace force_fatal_sig with force_exit_sig when in doubt

 arch/m68k/kernel/traps.c              |  2 +-
 arch/powerpc/kernel/signal_32.c       |  2 +-
 arch/powerpc/kernel/signal_64.c       |  4 ++--
 arch/s390/kernel/traps.c              |  2 +-
 arch/sparc/kernel/signal_32.c         |  4 ++--
 arch/sparc/kernel/windows.c           |  2 +-
 arch/x86/entry/vsyscall/vsyscall_64.c |  2 +-
 arch/x86/kernel/vm86_32.c             |  2 +-
 include/linux/sched/signal.h          |  1 +
 kernel/entry/syscall_user_dispatch.c  |  4 ++--
 kernel/signal.c                       | 36 ++++++++++++++++++++++++++++-------
 11 files changed, 42 insertions(+), 19 deletions(-)

Eric

Comments

Kyle Huey Nov. 19, 2021, 1:12 a.m. UTC | #1
On Thu, Nov 18, 2021 at 1:58 PM Eric W. Biederman <ebiederm@xmission.com> wrote:
>
>
> SA_IMMUTABLE fixed issues with force_sig_seccomp and the introduction
> for force_sig_fatal where the exit previously could not be interrupted
> but now it can.  Unfortunately it added that behavior to all force_sig
> functions under the right conditions which debuggers usage of SIG_TRAP
> and debuggers handling of SIGSEGV.
>
> Solve that by limiting SA_IMMUTABLE to just the cases that historically
> debuggers have not been able to intercept.
>
> The first patch changes force_sig_info_to_task to take a flag
> that requests which behavior is desired.
>
> The second patch adds force_exit_sig which replaces force_fatal_sig
> in the cases where historically userspace would only find out about
> the ``signal'' after the process has exited.
>
> The first one with the hunk changing force_fatal_sig removed should be
> suitable for backporting to v5.15. v5.15 does not implement
> force_fatal_sig.
>
> This should be enough to fix the regressions.
>
> Kyle if you can double check me that I have properly fixed these issues
> that would be appreciated.
>
> Any other review or suggestions to improve the names would be
> appreciated.  I think I have named things reasonably well but I am very
> close to the code so it is easy for me to miss things.
>
> Eric W. Biederman (2):
>       signal: Don't always set SA_IMMUTABLE for forced signals
>       signal: Replace force_fatal_sig with force_exit_sig when in doubt
>
>  arch/m68k/kernel/traps.c              |  2 +-
>  arch/powerpc/kernel/signal_32.c       |  2 +-
>  arch/powerpc/kernel/signal_64.c       |  4 ++--
>  arch/s390/kernel/traps.c              |  2 +-
>  arch/sparc/kernel/signal_32.c         |  4 ++--
>  arch/sparc/kernel/windows.c           |  2 +-
>  arch/x86/entry/vsyscall/vsyscall_64.c |  2 +-
>  arch/x86/kernel/vm86_32.c             |  2 +-
>  include/linux/sched/signal.h          |  1 +
>  kernel/entry/syscall_user_dispatch.c  |  4 ++--
>  kernel/signal.c                       | 36 ++++++++++++++++++++++++++++-------
>  11 files changed, 42 insertions(+), 19 deletions(-)
>
> Eric

rr's test suite passes with both diffs applied

Tested-by: Kyle Huey <khuey@kylehuey.com>

- Kyle
Eric W. Biederman Nov. 19, 2021, 3:41 p.m. UTC | #2
Linus,

Please pull the SA_IMMUTABLE-fixes-for-v5.16-rc2 branch from the git tree:

  git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git SA_IMMUTABLE-fixes-for-v5.16-rc2

  HEAD: fcb116bc43c8c37c052530ead79872f8b2615711 signal: Replace force_fatal_sig with force_exit_sig when in doubt

This is just a small set of changes where debuggers were no longer able
to intercept synchronous SIGTRAP and SIGSEGV.  This is essentially the
change you suggested with all of i's dotted and the t's crossed so that
ptrace can intercept all of the cases it has been able to intercept the
past, and all of the cases that made it to exit without giving ptrace a
chance still don't give ptrace a chance.

This change[1] has been tested by both Kyle and Kees.

Eric W. Biederman (2):
      signal: Don't always set SA_IMMUTABLE for forced signals
      signal: Replace force_fatal_sig with force_exit_sig when in doubt

 arch/m68k/kernel/traps.c              |  2 +-
 arch/powerpc/kernel/signal_32.c       |  2 +-
 arch/powerpc/kernel/signal_64.c       |  4 ++--
 arch/s390/kernel/traps.c              |  2 +-
 arch/sparc/kernel/signal_32.c         |  4 ++--
 arch/sparc/kernel/windows.c           |  2 +-
 arch/x86/entry/vsyscall/vsyscall_64.c |  2 +-
 arch/x86/kernel/vm86_32.c             |  2 +-
 include/linux/sched/signal.h          |  1 +
 kernel/entry/syscall_user_dispatch.c  |  4 ++--
 kernel/signal.c                       | 36 ++++++++++++++++++++++++++++-------
 11 files changed, 42 insertions(+), 19 deletions(-)

[1]: https://lkml.kernel.org/r/87h7c9qg7p.fsf_-_@email.froward.int.ebiederm.org

Eric
pr-tracker-bot@kernel.org Nov. 19, 2021, 7:46 p.m. UTC | #3
The pull request you sent on Fri, 19 Nov 2021 09:41:49 -0600:

> git://git.kernel.org/pub/scm/linux/kernel/git/ebiederm/user-namespace.git SA_IMMUTABLE-fixes-for-v5.16-rc2

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/7af959b5d5c8497b423e802e2b0ad847cb29b3d3

Thank you!