mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
To: Bradley Morgan <brads@mainlining.org>
Cc: boqun@kernel.org, dave@stgolabs.net, frederic@kernel.org,
	jiangshanlai@gmail.com, joelagnelf@nvidia.com,
	josh@joshtriplett.org, kernel-janitors@vger.kernel.org,
	linux-kernel@vger.kernel.org, mathieu.desnoyers@efficios.com,
	neeraj.upadhyay@kernel.org, paulmck@kernel.org,
	qiang.zhang@linux.dev, rcu@vger.kernel.org, rostedt@goodmis.org,
	urezki@gmail.com
Subject: Re: [PATCH] rcuscale: Constify struct rcu_scale_ops
Date: Sun, 8 Nov 2026 21:21:18 +0100	[thread overview]
Message-ID: <afaf8aa9-f7e3-4fc7-9e4e-7b9e9c73ca5f@wanadoo.fr> (raw)
In-Reply-To: <B33D74A6-E196-45F4-AA77-32B4FDCD0817@mainlining.org>

Le 08/11/2026 à 00:25, Bradley Morgan a écrit :
> On 7 November 2026 21:02:25 GMT, Christophe JAILLET
> <christophe.jaillet@wanadoo.fr> wrote:
>> 'struct rcu_scale_ops' is not modified in this driver.
>>
>> Constifying these structures moves some data to a read-only section, so
>> increases overall security, especially when the structure holds some
>> function pointers.
>>
>> On a x86_64, with allmodconfig:
>> Before:
>> ======
>>    text	   data	    bss	    dec	    hex	filename
>>   45585	  24946	   2816	  73347	  11e83	kernel/rcu/rcuscale.o
>>
>> After:
>> =====
>>    text	   data	    bss	    dec	    hex	filename
>>   46545	  23986	   2816	  73347	  11e83	kernel/rcu/rcuscale.o
>>
> 
> Well, thanks. so you just consted rcu_scale_ops()? Uhh, well, this makes
> enough sense to be feasible
> 
> Reviewed-by: Bradley Morgan
> <brads@mainlining.org>
> 
> But how is this worth it? Why doesn't this work now? how will this make it
> better? Those are my questions.

Nut sure to fully understand your point.

- how is this worth it?
The patch is simple and easy to review.
The struct rcu_scale_ops is local to this file, so no side effect is 
expected. It is compile tested.
So the additional security it brings (some function pointers that are 
now read-only) is a win to nearly no cost.

- Why doesn't this work now?
It does work now. The patch is not a fix, just an improvement.

-how will this make it better?
The function pointers that are now read-only can't be modified. So the 
code is safer.
A modification could be done by a bug that writes memory at an 
unexpected location or by an attacker that would manage to overwrite 
these function pointers to change the behavior of the code.
In general, RO data is preferred to RW when possible.

Hoping this answers your question.

CJ

> 
> Well, I'm shrugging, but my tag would stand here, because it won't break
> anything in particular.
> 
>> Signed-off-by: Christophe JAILLET <christophe.jaillet@wanadoo.fr>
>> ---
>> Compile tested only.
>> ---
>> kernel/rcu/rcuscale.c | 18 +++++++++---------
>> 1 file changed, 9 insertions(+), 9 deletions(-)
>>
>> diff --git a/kernel/rcu/rcuscale.c b/kernel/rcu/rcuscale.c
>> index 1097ec15879c..522f7bc2e693 100644
>> --- a/kernel/rcu/rcuscale.c
>> +++ b/kernel/rcu/rcuscale.c
>> @@ -159,7 +159,7 @@ struct rcu_scale_ops {
>> 	const char *name;
>> };
>>
>> -static struct rcu_scale_ops *cur_ops;
>> +static const struct rcu_scale_ops *cur_ops;
>>
>> /*
>>   * Definitions for rcu scalability testing.
>> @@ -185,7 +185,7 @@ static void rcu_sync_scale_init(void)
>> {
>> }
>>
>> -static struct rcu_scale_ops rcu_ops = {
>> +static const struct rcu_scale_ops rcu_ops = {
>> 	.ptype		= RCU_FLAVOR,
>> 	.init		= rcu_sync_scale_init,
>> 	.readlock	= rcu_scale_read_lock,
>> @@ -247,7 +247,7 @@ static void srcu_scale_synchronize_expedited(void)
>> 	synchronize_srcu_expedited(srcu_ctlp);
>> }
>>
>> -static struct rcu_scale_ops srcu_ops = {
>> +static const struct rcu_scale_ops srcu_ops = {
>> 	.ptype		= SRCU_FLAVOR,
>> 	.init		= rcu_sync_scale_init,
>> 	.readlock	= srcu_scale_read_lock,
>> @@ -276,7 +276,7 @@ static void srcu_sync_scale_cleanup(void)
>> 	cleanup_srcu_struct(srcu_ctlp);
>> }
>>
>> -static struct rcu_scale_ops srcud_ops = {
>> +static const struct rcu_scale_ops srcud_ops = {
>> 	.ptype		= SRCU_FLAVOR,
>> 	.init		= srcu_sync_scale_init,
>> 	.cleanup	= srcu_sync_scale_cleanup,
>> @@ -313,7 +313,7 @@ static void rcu_tasks_scale_stats(void)
>> 	rcu_tasks_torture_stats_print(scale_type, SCALE_FLAG);
>> }
>>
>> -static struct rcu_scale_ops tasks_ops = {
>> +static const struct rcu_scale_ops tasks_ops = {
>> 	.ptype		= RCU_TASKS_FLAVOR,
>> 	.init		= rcu_sync_scale_init,
>> 	.readlock	= tasks_scale_read_lock,
>> @@ -357,7 +357,7 @@ static void rcu_tasks_rude_scale_stats(void)
>> 	rcu_tasks_rude_torture_stats_print(scale_type, SCALE_FLAG);
>> }
>>
>> -static struct rcu_scale_ops tasks_rude_ops = {
>> +static const struct rcu_scale_ops tasks_rude_ops = {
>> 	.ptype		= RCU_TASKS_RUDE_FLAVOR,
>> 	.init		= rcu_sync_scale_init,
>> 	.readlock	= tasks_rude_scale_read_lock,
>> @@ -396,7 +396,7 @@ static void tasks_trace_scale_read_unlock(int idx)
>> 	rcu_read_unlock_trace();
>> }
>>
>> -static struct rcu_scale_ops tasks_tracing_ops = {
>> +static const struct rcu_scale_ops tasks_tracing_ops = {
>> 	.ptype		= RCU_TASKS_FLAVOR,
>> 	.init		= rcu_sync_scale_init,
>> 	.readlock	= tasks_trace_scale_read_lock,
>> @@ -693,7 +693,7 @@ rcu_scale_writer(void *arg)
>> }
>>
>> static void
>> -rcu_scale_print_module_parms(struct rcu_scale_ops *cur_ops, const char *tag)
>> +rcu_scale_print_module_parms(const struct rcu_scale_ops *cur_ops, const char *tag)
>> {
>> 	pr_alert("%s" SCALE_FLAG
>> 		 "--- %s: gp_async=%d gp_async_max=%d gp_exp=%d holdoff=%d minruntime=%d nreaders=%d nwriters=%d nexp=%d exp_interval=%d writer_holdoff=%d writer_holdoff_jiffies=%d verbose=%d shutdown_secs=%d\n",
>> @@ -1109,7 +1109,7 @@ rcu_scale_init(void)
>> 	int firsterr = 0;
>> 	long i;
>> 	long j;
>> -	static struct rcu_scale_ops *scale_ops[] = {
>> +	static const struct rcu_scale_ops *scale_ops[] = {
>> 		&rcu_ops, &srcu_ops, &srcud_ops, TASKS_OPS TASKS_RUDE_OPS TASKS_TRACING_OPS
>> 	};
>>
>>
> 
> --- Thanks!


  reply	other threads:[~2026-11-08 20:21 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-11-07 21:02 Christophe JAILLET
2026-11-07 23:25 ` Bradley Morgan
2026-11-08 20:21   ` Christophe JAILLET [this message]
2026-11-08 23:06 ` Joel Fernandes

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=afaf8aa9-f7e3-4fc7-9e4e-7b9e9c73ca5f@wanadoo.fr \
    --to=christophe.jaillet@wanadoo.fr \
    --cc=boqun@kernel.org \
    --cc=brads@mainlining.org \
    --cc=dave@stgolabs.net \
    --cc=frederic@kernel.org \
    --cc=jiangshanlai@gmail.com \
    --cc=joelagnelf@nvidia.com \
    --cc=josh@joshtriplett.org \
    --cc=kernel-janitors@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathieu.desnoyers@efficios.com \
    --cc=neeraj.upadhyay@kernel.org \
    --cc=paulmck@kernel.org \
    --cc=qiang.zhang@linux.dev \
    --cc=rcu@vger.kernel.org \
    --cc=rostedt@goodmis.org \
    --cc=urezki@gmail.com \
    /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

all inboxes | Powered by JetHome®