May 3, 2024 19:55:37 Kees Cook <keescook@xxxxxxxxxxxx>:
> On Fri, May 03, 2024 at 03:44:09PM +0200, Thomas Weißschuh wrote:
>> Sysctl handlers are not supposed to modify the ctl_table passed to them.
>> Adapt the logic to work with a temporary
>> variable, similar to how it is done in other parts of the kernel.
>>
>> This is also a prerequisite to enforce the immutability of the argument
>> through the callbacks prototy.
>>
>> Fixes: 964c9dff0091 ("stackleak: Allow runtime disabling of kernel stack erasing")
>> Cc: stable@xxxxxxxxxxxxxxx
>
> I realize I've already Acked, but does this actually need to be CCed
> to stable?
You acked it without the Cc stable.
I shouldn't have kept your Ack, sorry.
Feel free to drop the Cc, it shouldn't be critical.
I suspect the bots will pick it up anyways.
>> Acked-by: Kees Cook <keescook@xxxxxxxxxxxx>
>> Reviewed-by: Luis Chamberlain <mcgrof@xxxxxxxxxx>
>> Signed-off-by: Thomas Weißschuh <linux@xxxxxxxxxxxxxx>
>> ---
>> This was split out of my sysctl-const-handler series [0].
>>
>> As that series will take some more time, submit the patch on its own,
>> as it is a generic bugfix that is valuable on its own.
>> And I can get it out of my books.
>>
>> Changelog in contrast to the patch in the series:
>> * Reword commit message to remove strong relation to the constification
>> * Cc stable
>>
>> [0] https://lore.kernel.org/lkml/20240423-sysctl-const-handler-v3-0-e0beccb836e2@xxxxxxxxxxxxxx/
>>
>> Cc: Joel Granados <j.granados@xxxxxxxxxxx>
>> ---
>> kernel/stackleak.c | 7 ++++---
>> 1 file changed, 4 insertions(+), 3 deletions(-)
>>
>> diff --git a/kernel/stackleak.c b/kernel/stackleak.c
>> index 34c9d81eea94..b292e5ca0b7d 100644
>> --- a/kernel/stackleak.c
>> +++ b/kernel/stackleak.c
>> @@ -27,10 +27,11 @@ static int stack_erasing_sysctl(struct ctl_table *table, int write,
>> int ret = 0;
>> int state = !static_branch_unlikely(&stack_erasing_bypass);
>> int prev_state = state;
>> + struct ctl_table tmp = *table;
>>
>> - table->data = &state;
>> - table->maxlen = sizeof(int);
>> - ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
>> + tmp.data = &state;
>> + tmp.maxlen = sizeof(int);
>> + ret = proc_dointvec_minmax(&tmp, write, buffer, lenp, ppos);
>> state = !!state;
>> if (ret || !write || state == prev_state)
>> return ret;
>
> I can pick this up; thanks!
Thanks!
