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!
next prev parent 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®