Thread (29 messages) 29 messages, 6 authors, 2024-05-13

Re: [PATCH v3 00/11] sysctl: treewide: constify ctl_table argument of sysctl handlers

From: Joel Granados <hidden>
Date: 2024-05-08 11:46:40
Also in: bpf, bridge, kexec, linux-arm-kernel, linux-fsdevel, linux-hardening, linux-mm, linux-nfs, linux-perf-users, linux-rdma, linux-riscv, linux-s390, linux-sctp, linux-security-module, linux-trace-kernel, linux-xfs, lkml, lvs-devel, netdev, netfilter-devel

Kees

Could you comment on the feasibility of this alternative from the
Control Flow Integrity perspective. My proposal is to change the
proc_handler to void* and back in the same release. So there would not
be a kernel released with a void* proc_handler.
quoted
However, there is an alternative way to do this that allows chunking. We
first define the proc_handler as a void pointer (casting it where it is
being used) [1]. Then we could do the constification by subsystem (like
Jakub proposes). Finally we can "revert the void pointer change so we
don't have one size fit all pointer as our proc_handler [2].

Here are some comments about the alternative:
1. We would need to make the first argument const in all the derived
   proc_handlers [3] 
2. There would be no undefined behavior for two reasons:
   2.1. There is no case where we change the first argument. We know
        this because there are no compile errors after we make it const.
   2.2. We would always go from non-const to const. This is the case
        because all the stuff that is unchanged in non-const.
3. If the idea sticks, it should go into mainline as one patchset. I
   would not like to have a void* proc_handler in a kernel release.
4. I think this is a "win/win" solution were the constification goes
   through and it is divided in such a way that it is reviewable.

I would really like to hear what ppl think about this "heretic"
alternative. @Thomas, @Luis, @Kees @Jakub?
Thanks for that alternative, I'm not a big fan though.

Besides the wonky syntax, Control Flow Integrity should trap on
this construct. Functions are called through different pointers than
their actual types which is exactly what CFI is meant to prevent.

Maybe people find it easier to review when using
"--word-diff" and/or "-U0" with git diff/show.
There is really nothing going an besides adding a few "const"s.

But if the consensus prefers this solution, I'll be happy to adopt it.
quoted
[1] https://git.kernel.org/pub/scm/linux/kernel/git/joel.granados/linux.git/commit/?h=jag/constfy_treewide_alternative&id=4a383503b1ea650d4e12c1f5838974e879f5aa6f
[2] https://git.kernel.org/pub/scm/linux/kernel/git/joel.granados/linux.git/commit/?h=jag/constfy_treewide_alternative&id=a3be65973d27ec2933b9e81e1bec60be3a9b460d
[3] proc_dostring, proc_dobool, proc_dointvec....

Thomas
Best
-- 

Joel Granados

Attachments

Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help