linux-kernel - Re: [PATCH v4 20/22] rv: Add rtapp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250425092309.nuWnAPa3@linutronix.de>
Date: Fri, 25 Apr 2025 11:23:09 +0200
From: Nam Cao <namcao@...utronix.de>
To: John Ogness <john.ogness@...utronix.de>
Cc: Gabriele Monaco <gmonaco@...hat.com>,
	Steven Rostedt <rostedt@...dmis.org>,
	linux-trace-kernel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 20/22] rv: Add rtapp_sleep monitor

On Fri, Apr 25, 2025 at 09:54:27AM +0206, John Ogness wrote:
> On 2025-04-25, John Ogness <john.ogness@...utronix.de> wrote:
> > If I understand this correctly, trace_sched_switch() is reporting
> > accurate state transition information, but by the time it is reported
> > that state may have already changed (in which case another
> > trace_sched_switch() occurs later).
> >
> > So in this example, the task did go to sleep. Why do you think it was
> > preempted instead?

You are right, the task did go to sleep. Scratch what I said earlier.

The monitor checks that if an RT task going to sleep, it will be woken by a
"RT-friendly" source. The problem is that trace_sched_switch() may appear
after trace_sched_waking(). The monitor sees the task sleeps, and waits
until the task is woken and checks the waker. But the monitor doesn't see
the task being woken, because it has already happened before the task
sleeps.

For correct ordering, we could:

  - Use trace_sched_wakeup() instead of trace_sched_waking(). This would
    have correct order, but information about the waker is gone at
    trace_sched_wakeup(), so it doesn't work.

  - Use trace_set_current_state() instead of trace_sched_switch() to
    determine task going to sleep.

The latter option works, but then "sleep" would be defined as task doing
set_current_state(TASK_INTERRUPTIBLE). This new definition is probably not
precise? But for the monitor, it's fine.

Btw, while testing this, I discovered another bug. Real-time thread may
"legally" sleep by a 'restart' syscall after 'nanosleep'. The monitor
doesn't recognize this syscall as valid sleep reason and flags it :(

> On 2025-04-25, Gabriele Monaco <gmonaco@...hat.com> wrote:
> > Peter's fix [1] landed on next recently, I guess in a couple of days
> > you'll get it on the upstream tree and you may not see the problem.

This patch fixes stale prev_state due to signaling. It doesn't fix this
case.

Best regards,
Nam