[PATCH v2 0/4] sysctl: Disallow partial updates of miss-formatted sysctl vectors

Joel Granados posted 4 patches 1 month, 2 weeks ago
kernel/sysctl.c                          | 237 ++++++++++++++++++++-----------
tools/testing/selftests/sysctl/sysctl.sh |  60 ++++++++
2 files changed, 217 insertions(+), 80 deletions(-)
[PATCH v2 0/4] sysctl: Disallow partial updates of miss-formatted sysctl vectors
Posted by Joel Granados 1 month, 2 weeks ago
What?
=====

Stage table->data when writing INT & ULONG ctl_tables. Commit staged
data to kernel variable only when all the conversions have succeeded.
This is applicable only to variables that represent a vector; paths
pertaining to scalers are left unchanged. Notice that partial updates
can still happen if less than the size of the vector are passed and
correctly formatted.

This is the behavior we are protecting against:

  # echo "4 4 1 7" > /proc/sys/kernel/printk
  # echo "1 x" > /proc/sys/kernel/printk
  -bash: echo: write error: Invalid argument
  # cat /proc/sys/kernel/printk
  1 4 1 7    <- the write failed, the first element changed anyway

Why?
====
Allowing a partial change to a vector after returning from an erroneous
proc_handler is just plain wrong. This should be handled within sysctl
to avoid users having to do it for themselves [1]. 

Behavioral Changes
==================
1. A failed write will not update the vector
2. Vector writes can now fail with -ENOMEM

Testing
=======
This went through regular sysctl kunit and self test. Also is posted to
0-day.

I always find it difficult to know who wants to receive this. Please let
me know if you want to be removed from the Cc/To.

Best

[1] https://lore.kernel.org/all/tencent_A860C873956A52E26AD8D309A308A241BA08@qq.com/

Signed-off-by: Joel Granados <joel.granados@kernel.org>
---
Changes in v2:
- Avoid doc compilation warnings related to apply_conv_on_vec function
- Initialize rc in testing to avoid a false positive when executing 0013
  individually
- Link to v1: https://lore.kernel.org/r/20260813-lklm-partial_ctlvec-v1-0-df9e51c13704@kernel.org

---
Joel Granados (4):
      sysctl: Split data conversion and file position handling
      sysctl: Reject uint arrays before calling the general proc_vec
      sysctl: Disallow partial updates for erroneous sysctl vectors
      sysctl: Add 0013 to test partially updated vectors

 kernel/sysctl.c                          | 237 ++++++++++++++++++++-----------
 tools/testing/selftests/sysctl/sysctl.sh |  60 ++++++++
 2 files changed, 217 insertions(+), 80 deletions(-)
---
base-commit: 4c12287001da60f3c022bb25932aa7a5590fc0b1
change-id: 20260813-lklm-partial_ctlvec-bd8867b70d5c

Best regards,
-- 
Joel Granados <joel.granados@kernel.org>
Re: [PATCH v2 0/4] sysctl: Disallow partial updates of miss-formatted sysctl vectors
Posted by Joel Granados 1 month, 1 week ago
On Fri, Aug 14, 2026 at 12:41:11PM +0200, Joel Granados wrote:
> What?
> =====
> 
> Stage table->data when writing INT & ULONG ctl_tables. Commit staged
> data to kernel variable only when all the conversions have succeeded.
> This is applicable only to variables that represent a vector; paths
> pertaining to scalers are left unchanged. Notice that partial updates
> can still happen if less than the size of the vector are passed and
> correctly formatted.
... snip ...
>       sysctl: Split data conversion and file position handling
>       sysctl: Reject uint arrays before calling the general proc_vec
>       sysctl: Disallow partial updates for erroneous sysctl vectors
>       sysctl: Add 0013 to test partially updated vectors
> 
>  kernel/sysctl.c                          | 237 ++++++++++++++++++++-----------
>  tools/testing/selftests/sysctl/sysctl.sh |  60 ++++++++
>  2 files changed, 217 insertions(+), 80 deletions(-)
> ---
> base-commit: 4c12287001da60f3c022bb25932aa7a5590fc0b1
> change-id: 20260813-lklm-partial_ctlvec-bd8867b70d5c
> 
> Best regards,
> -- 
> Joel Granados <joel.granados@kernel.org>
> 
> 

I feel the superficial changes mentioned in the review (up until now) do
not merit me sending a V3. The modified tree is here [1]. 

Best

[1] https://git.kernel.org/pub/scm/linux/kernel/git/joel.granados/linux.git/log/?h=jag/partial_ctlvec

Re: [PATCH v2 0/4] sysctl: Disallow partial updates of miss-formatted sysctl vectors
Posted by Bradley Morgan 1 month, 2 weeks ago
Copy it, only write back once every value converted. That's the
right way to do this. Went through the whole series against the
tree, notes in the patch replies.

Some nits on the series:
- "miss-formatted" in the subject. mis-formatted, or just malformed.
- akpm's scalars nit still applies.
- Sashiko saw something on this one (akpm linked it).

Otherwise fine by me, the split is clean.


Reviewed-by: Bradley Morgan <include@grrlz.net>
also

Tested-by: Bradley Morgan <include@grrlz.net> for the test


Thanks!
Re: [PATCH v2 0/4] sysctl: Disallow partial updates of miss-formatted sysctl vectors
Posted by Joel Granados 1 month, 1 week ago
On Sat, Aug 15, 2026 at 05:09:30PM +0100, Bradley Morgan wrote:
> 
> Copy it, only write back once every value converted. That's the
> right way to do this. Went through the whole series against the
> tree, notes in the patch replies.
Your feedback was useful, but it was riddled with a *lot* of hand
waving. Please be succinct and to the point. The previous paragraph is
not needed.

> 
> Some nits on the series:
> - "miss-formatted" in the subject. mis-formatted, or just malformed.
Note that this comment is missing the why. It is good to know why you
want the change

> - akpm's scalars nit still applies.
> - Sashiko saw something on this one (akpm linked it).
In my opinion this is just noise. If you see that I have missed a
comment, call me out on it. If you have additional information, answer
to reviewers mail. 

> 
> Otherwise fine by me, the split is clean.
> 
> 
> Reviewed-by: Bradley Morgan <include@grrlz.net>
> also
> 
> Tested-by: Bradley Morgan <include@grrlz.net> for the test
How did you test? And what where your results?

Best
Re: [PATCH v2 0/4] sysctl: Disallow partial updates of miss-formatted sysctl vectors
Posted by Bradley Morgan 1 month, 1 week ago
On 18 August 2026 13:46:58 BST, Joel Granados <joel.granados@kernel.org>
wrote:
>On Sat, Aug 15, 2026 at 05:09:30PM +0100, Bradley Morgan wrote:
>> 
>> Copy it, only write back once every value converted. That's the
>> right way to do this. Went through the whole series against the
>> tree, notes in the patch replies.
>Your feedback was useful, but it was riddled with a *lot* of hand
>waving. Please be succinct and to the point. The previous paragraph is
>not needed.
>
>> 
>> Some nits on the series:
>> - "miss-formatted" in the subject. mis-formatted, or just malformed.
>Note that this comment is missing the why. It is good to know why you
>want the change
>
>> - akpm's scalars nit still applies.
>> - Sashiko saw something on this one (akpm linked it).
>In my opinion this is just noise. If you see that I have missed a
>comment, call me out on it. If you have additional information, answer
>to reviewers mail. 
>
>> 
>> Otherwise fine by me, the split is clean.
>> 
>> 
>> Reviewed-by: Bradley Morgan <include@grrlz.net>
>> also
>> 
>> Tested-by: Bradley Morgan <include@grrlz.net> for the test
>How did you test? And what where your results?
>

Built and booted the latest kernel. Its the summer holidays, I can do all
this, it's magical

>Best
>

Thanks!
Re: [PATCH v2 0/4] sysctl: Disallow partial updates of miss-formatted sysctl vectors
Posted by Andrew Morton 1 month, 2 weeks ago
On Fri, 14 Aug 2026 12:41:11 +0200 Joel Granados <joel.granados@kernel.org> wrote:

> What?
> =====
> 
> Stage table->data when writing INT & ULONG ctl_tables. Commit staged
> data to kernel variable only when all the conversions have succeeded.
> This is applicable only to variables that represent a vector; paths
> pertaining to scalers are left unchanged. Notice that partial updates

"scalars"

> can still happen if less than the size of the vector are passed and
> correctly formatted.
> 
> This is the behavior we are protecting against:
> 
>   # echo "4 4 1 7" > /proc/sys/kernel/printk
>   # echo "1 x" > /proc/sys/kernel/printk
>   -bash: echo: write error: Invalid argument
>   # cat /proc/sys/kernel/printk
>   1 4 1 7    <- the write failed, the first element changed anyway

I like the change.

> Why?
> ====
> Allowing a partial change to a vector after returning from an erroneous
> proc_handler is just plain wrong. This should be handled within sysctl
> to avoid users having to do it for themselves [1]. 
> 
> Behavioral Changes
> ==================
> 1. A failed write will not update the vector
> 2. Vector writes can now fail with -ENOMEM

Yeah, this improves Linux.

> Testing
> =======
> This went through regular sysctl kunit and self test. Also is posted to
> 0-day.
> 
> I always find it difficult to know who wants to receive this. Please let
> me know if you want to be removed from the Cc/To.

Sashiko said one thing:
	https://sashiko.dev/#/patchset/20260814-lklm-partial_ctlvec-v2-0-9df50d26e477@kernel.org
Re: [PATCH v2 0/4] sysctl: Disallow partial updates of miss-formatted sysctl vectors
Posted by Joel Granados 1 month, 1 week ago
On Fri, Aug 14, 2026 at 12:36:02PM -0700, Andrew Morton wrote:
> On Fri, 14 Aug 2026 12:41:11 +0200 Joel Granados <joel.granados@kernel.org> wrote:
> 
> > What?
> > =====
> > 
> > Stage table->data when writing INT & ULONG ctl_tables. Commit staged
> > data to kernel variable only when all the conversions have succeeded.
> > This is applicable only to variables that represent a vector; paths
> > pertaining to scalers are left unchanged. Notice that partial updates
> 
> "scalars"

Will change in the cover letter (If/when I send the next version). It
wont actually show up in upstream as this is just in the cover letter.

> 
> > can still happen if less than the size of the vector are passed and
> > correctly formatted.
> > 
> > This is the behavior we are protecting against:
> > 
> >   # echo "4 4 1 7" > /proc/sys/kernel/printk
> >   # echo "1 x" > /proc/sys/kernel/printk
> >   -bash: echo: write error: Invalid argument
> >   # cat /proc/sys/kernel/printk
> >   1 4 1 7    <- the write failed, the first element changed anyway
> 
> I like the change.
> 
> > Why?
> > ====
> > Allowing a partial change to a vector after returning from an erroneous
> > proc_handler is just plain wrong. This should be handled within sysctl
> > to avoid users having to do it for themselves [1]. 
> > 
> > Behavioral Changes
> > ==================
> > 1. A failed write will not update the vector
> > 2. Vector writes can now fail with -ENOMEM
> 
> Yeah, this improves Linux.
> 
> > Testing
> > =======
> > This went through regular sysctl kunit and self test. Also is posted to
> > 0-day.
> > 
> > I always find it difficult to know who wants to receive this. Please let
> > me know if you want to be removed from the Cc/To.
> 
> Sashiko said one thing:
> 	https://sashiko.dev/#/patchset/20260814-lklm-partial_ctlvec-v2-0-9df50d26e477@kernel.org

The tests that keep on giving :).

I'm already working on a replacement for the sysctl selftests and prefer
to leave these as they are and address them in the new version if they
persist.


Thx for the feedback

Best

Joel