LinuxKPI: add proper support for charp module parameters

[val_packett.cool]

Summary

sysctl_handle_string does not actually work on a double pointer.
Implement a handler that manages a dynamically allocated string instead of a static one.

This is required for hw.amdgpu.virtual_display.

Theoretically this code should even support read-write sysctls, but we only have boot-time-only tunables in amdgpu.

Related to: https://github.com/freebsd/drm-kmod/issues/134

---

BTW, can someone please document what exactly are the "indexes" (oldidx/newidx) in the sysctl req struct? sysctl_handle_string fiddles with them but I don't understand why.

Test Plan

I have only tested with amdgpu's virtual_display tunable.

[val_packett.cool]

add missing + 1 for strlen

[• hselasky]

sys/compat/linuxkpi/common/include/linux/moduleparam.h

143–145

Style:
Function prototype should go after the macros:

extern int lkpi_sysctl_handle_charp(SYSCTL_HANDLER_ARGS);

I also like to use extern, though some see it redundant.

sys/compat/linuxkpi/common/src/linux_compat.c

121

Just use the Linux allocator, kalloc(GFP_KERNEL) ?

The function is very small and leaks are impossible!

[wulf]

sys/compat/linuxkpi/common/src/linux_compat.c

2740

Take this lock exclusively right here. Otherwise SYSCTL_IN and SYSCTL_OUT gets a great chance to see different versions of *charp when try_upgrade is failed

2743

SYSCTL_OUT can sleep. You should call sysctl_wire_old_buffer() before taking lkpi_charp_lock

2765

You cannot do malloc(M_WAITOK) with non-sleepable lock held.

2769

How these operations on *charp are synchronized with drm-kmod? Are they protected by external lock?

[wulf]

It looks that sx lock suits our needs better than rw here.

[val_packett.cool]

Use the linuxkpi allocator

[val_packett.cool]

Switch to sx lock

[val_packett.cool]

sys/compat/linuxkpi/common/src/linux_compat.c

2769

In the kmods we have, charp parameters are only used for boot-only tunables. I'm not sure if they even make sense with things that can change at runtime. There is no synchronization, the module just expects the kernel to modify the pointer on load and reads it when initializing the hardware.

[wulf]

sys/compat/linuxkpi/common/include/linux/moduleparam.h

111

var must be freed on module unload

sys/compat/linuxkpi/common/src/linux_compat.c

2752

IMO charp should be freed only after successfull SYSCTL_IN(). I think *charp should not be reseted after e.g. EFAULT

2769

Ok. It looks that drm-kmod is awared of absence of synchronization and does strdup() before parsing of data referenced by charp.

[val_packett.cool]

sys/compat/linuxkpi/common/src/linux_compat.c

2752

hmm well, I put it here because if (req->newlen == 0) I replace it with NULL without doing SYSCTL_IN

[val_packett.cool]

• rebase
• add SYSUNINIT for deallocating the strings
• remove useless null check (kfree supports null)

[• hselasky]

sys/compat/linuxkpi/common/include/linux/moduleparam.h

108

This kfree() looks incorrect. Should be kfree(var); ???