Bug 231445 - [patch] sleepq_catch_signals will still enter sleep after a ptrace attach event
Summary: [patch] sleepq_catch_signals will still enter sleep after a ptrace attach event
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: kern (show other bugs)
Version: CURRENT
Hardware: Any Any
: --- Affects Some People
Assignee: freebsd-bugs (Nobody)
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2018-09-18 10:56 UTC by Efi Weiss
Modified: 2019-07-26 10:45 UTC (History)
3 users (show)

See Also:


Attachments
patch (1.37 KB, text/plain)
2018-09-18 10:56 UTC, Efi Weiss
no flags Details
For thread on sleepqueue, leave cleaning TDB_FSTP to the sleepq code. (1.38 KB, patch)
2019-05-21 21:10 UTC, Konstantin Belousov
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Efi Weiss 2018-09-18 10:56:06 UTC
Created attachment 197188 [details]
patch

If a ptrace attach SIGSTOP is queued to a process while that process is on a sleep queue but has not yet entered sleep, the signal will not abort the sleep.

This behavior which contradicts the expected behavior that happens when the process is interrupted mid sleep - it aborts the sleep and will continue from a user mode boundary when continued.

In the current condition after the process is restarted, it will immediately enter the sleep as if no signal was received (this is due to issignal deleting signals handled by ptracestop and returning 0, where sleepq_catch_signals uses the return value in the pending signals check prior to entering sleep).

A proposed patch is attached.
I reproduced this issue on a FreeBSD12-CURRENT amd64 machine running on QEMU with multiple cores.
Comment 1 Konstantin Belousov freebsd_committer freebsd_triage 2019-05-21 21:09:10 UTC
Can you provide a demonstrating example or test ?
It took me some time to convince myself that what you propose is reasonable
behavior, and I want to see the real code that depends on this.

If anything, the patch I will attach in a minute should be a proper solution,
but I did not tested it.
Comment 2 Konstantin Belousov freebsd_committer freebsd_triage 2019-05-21 21:10:04 UTC
Created attachment 204522 [details]
For thread on sleepqueue, leave cleaning TDB_FSTP to the sleepq code.
Comment 3 Konstantin Belousov freebsd_committer freebsd_triage 2019-05-23 15:39:29 UTC
https://reviews.freebsd.org/D20381
Comment 4 commit-hook freebsd_committer freebsd_triage 2019-05-29 14:06:09 UTC
A commit references this bug:

Author: kib
Date: Wed May 29 14:05:28 UTC 2019
New revision: 348360
URL: https://svnweb.freebsd.org/changeset/base/348360

Log:
  Do not go into sleep in sleepq_catch_signals() when SIGSTOP from
  PT_ATTACH was consumed.

  In particular, do not clear TDP_FSTP in ptracestop() if td_wchan is
  non-NULL. Leave it to sleepq_catch_signal() to clear and convert zero
  return code to EINTR.

  Otherwise, per submitter report, if the PT_ATTACH SIGSTOP was
  delivered right after the thread was added to the sleepqueue but not
  yet really sleep, and cursig() caused debugger attach, the thread
  sleeps instead of returning to the userspace boundary with EINTR.

  PR: 231445
  Reported by:	Efi Weiss <valmarelox@gmail.com>
  Reviewed by:	markj
  Sponsored by:	The FreeBSD Foundation
  MFC after:	2 weeks
  Differential revision:	https://reviews.freebsd.org/D20381

Changes:
  head/sys/kern/kern_sig.c
  head/sys/kern/subr_sleepqueue.c
Comment 5 commit-hook freebsd_committer freebsd_triage 2019-06-12 11:10:24 UTC
A commit references this bug:

Author: kib
Date: Wed Jun 12 11:09:34 UTC 2019
New revision: 348988
URL: https://svnweb.freebsd.org/changeset/base/348988

Log:
  MFC r348360:
  Do not go into sleep in sleepq_catch_signals() when SIGSTOP from
  PT_ATTACH was consumed.

  PR:	231445

Changes:
_U  stable/12/
  stable/12/sys/kern/kern_sig.c
  stable/12/sys/kern/subr_sleepqueue.c
Comment 6 commit-hook freebsd_committer freebsd_triage 2019-07-26 10:43:26 UTC
A commit references this bug:

Author: kib
Date: Fri Jul 26 10:43:07 UTC 2019
New revision: 350357
URL: https://svnweb.freebsd.org/changeset/base/350357

Log:
  MFC r348360:
  Do not go into sleep in sleepq_catch_signals() when SIGSTOP from
  PT_ATTACH was consumed.

  PR:	231445

Changes:
_U  stable/11/
  stable/11/sys/kern/kern_sig.c
  stable/11/sys/kern/subr_sleepqueue.c