Linux Trace Kernel
 help / color / mirror / Atom feed
From: Wen Yang <wen.yang@linux.dev>
To: Gabriele Monaco <gmonaco@redhat.com>,
	linux-kernel@vger.kernel.org,
	Steven Rostedt <rostedt@goodmis.org>,
	Nam Cao <namcao@linutronix.de>,
	linux-trace-kernel@vger.kernel.org
Subject: Re: [PATCH 3/9] rv: Reset per-task DA monitors before releasing the slot
Date: Sun, 17 May 2026 16:55:41 +0800	[thread overview]
Message-ID: <a871022a-baf2-426f-b3dc-36149e928a26@linux.dev> (raw)
In-Reply-To: <20260512140250.262190-4-gmonaco@redhat.com>


The fix is correct: task_mon_slot = RV_PER_TASK_MONITOR_INIT
equals CONFIG_RV_PER_TASK_MONITORS, which is one past the end of rv[],
so calling da_monitor_reset_all() after rv_put_task_monitor_slot()
would write into whatever memory follows task_struct.rv[] — which is
randomised and can get quite nasty, as you noted in the review thread.

Overlap note: . 
https://lore.kernel.org/all/f654a17c671469fd8fc9ea438daf2266d05068d4.camel@redhat.com/

We will coordinate to avoid redundancy;
we are happy to defer to your version here.


Reviewed-by: Wen Yang <wen.yang@linux.dev>



On 5/12/26 22:02, Gabriele Monaco wrote:
> Per-task monitors use task_mon_slot to determine which slot in the array
> to use for the monitor. During destruction, this slot is returned but
> this is done before resetting the monitor. As a result, the monitor's
> reset is in fact resetting a slot that is outside of the array
> (RV_PER_TASK_MONITOR_INIT).
> 
> Release the slot only after the reset to avoid out-of-bound memory
> access.
> 
> Fixes: 30984ccf31b7f ("rv: Refactor da_monitor to minimise macros")
> Fixes: 792575348ff70 ("rv/include: Add deterministic automata monitor definition via C macros")
> Signed-off-by: Gabriele Monaco <gmonaco@redhat.com>
> ---
>   include/rv/da_monitor.h | 5 +++--
>   1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/include/rv/da_monitor.h b/include/rv/da_monitor.h
> index 250888812125..0b7028df08fb 100644
> --- a/include/rv/da_monitor.h
> +++ b/include/rv/da_monitor.h
> @@ -309,10 +309,11 @@ static inline void da_monitor_destroy(void)
>   		WARN_ONCE(1, "Disabling a disabled monitor: " __stringify(MONITOR_NAME));
>   		return;
>   	}
> -	rv_put_task_monitor_slot(task_mon_slot);
> -	task_mon_slot = RV_PER_TASK_MONITOR_INIT;
>   
>   	da_monitor_reset_all();
> +
> +	rv_put_task_monitor_slot(task_mon_slot);
> +	task_mon_slot = RV_PER_TASK_MONITOR_INIT;
>   }
>   
>   #elif RV_MON_TYPE == RV_MON_PER_OBJ

  reply	other threads:[~2026-05-17  8:56 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-12 14:02 [PATCH 0/9] rv: Fixes on Deterministic and Hybrid Automata Gabriele Monaco
2026-05-12 14:02 ` [PATCH 1/9] rv: Fix __user specifier usage in extract_params() Gabriele Monaco
2026-05-17  8:48   ` Wen Yang
2026-05-12 14:02 ` [PATCH 2/9] rv: Fix read_lock scope in per-task DA cleanup Gabriele Monaco
2026-05-17  8:51   ` Wen Yang
2026-05-12 14:02 ` [PATCH 3/9] rv: Reset per-task DA monitors before releasing the slot Gabriele Monaco
2026-05-17  8:55   ` Wen Yang [this message]
2026-05-12 14:02 ` [PATCH 4/9] rv: Prevent task migration while handling per-CPU events Gabriele Monaco
2026-05-17  8:57   ` Wen Yang
2026-05-12 14:02 ` [PATCH 5/9] rv: Ensure all pending probes terminate on per-obj monitor destroy Gabriele Monaco
2026-05-17  9:01   ` Wen Yang
2026-05-12 14:02 ` [PATCH 6/9] rv: Ensure synchronous cleanup for HA monitors Gabriele Monaco
2026-05-17  9:12   ` Wen Yang
2026-05-18 11:54     ` Gabriele Monaco
2026-05-19  9:31       ` Gabriele Monaco
2026-05-12 14:02 ` [PATCH 7/9] rv: Do not rely on clean monitor when initialising HA Gabriele Monaco
2026-05-17  9:15   ` Wen Yang
2026-05-12 14:02 ` [PATCH 8/9] rv: Add automatic cleanup handlers for per-task HA monitors Gabriele Monaco
2026-05-17  9:40   ` Wen Yang
2026-05-18 12:18     ` Gabriele Monaco
2026-05-12 14:02 ` [PATCH 9/9] rv: Mandate deallocation for per-obj monitors Gabriele Monaco
2026-05-17  9:52   ` Wen Yang
2026-05-18  6:36     ` Gabriele Monaco
2026-05-18 15:40       ` Wen Yang

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=a871022a-baf2-426f-b3dc-36149e928a26@linux.dev \
    --to=wen.yang@linux.dev \
    --cc=gmonaco@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-trace-kernel@vger.kernel.org \
    --cc=namcao@linutronix.de \
    --cc=rostedt@goodmis.org \
    /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