WIP: sysctl: Respect max length when treat a variable string as a constant string

[zlei]

Summary

D48511 has too many comments to review.

Test Plan

See test plan in D48511 .

[zlei]

sys/kern/kern_sysctl.c

1903

I'd keep arg2 assigned, in case consumer want to write to such sysctl knob,

static char buff[] = "some long long content";
SYSCTL_STRING(_sysctl, OID_AUTO, buff, CTLFLAG_RW, buff, 0,
    "Read-write string buff");

I have not searched the above usage, but just in case.

[zlei]

sys/kern/kern_sysctl.c

1903

Emm, CTLFLAG_RW is wong usage. Consider this one.

--- SYSCTL_STRING(_sysctl, OID_AUTO, buff, CTLFLAG_RW, buff, 0,
+++ SYSCTL_STRING(_sysctl, OID_AUTO, buff, CTLFLAG_RDTUN, buff, 0,
    "Read-write string buff");

[zlei]

@olce You can click "Hide All Inlines" to help reviewing.

[avg]

Still looks good to me.

[olce]

This generally looks good (and much better than D48511) except for what I stated in an inline comment (don't mess with arg2, I insist on that).

Although not blocking because it doesn't affect the correctness of your change and current consumer expectations, my remark in D48511 that there isn't much sense in introducing the -1/+1 computation if you don't go all the way to ensure that a terminating NUL will be output in all cases still stands (the case where arg1 has len arg2 without being NUL terminated is not handled properly; in theory, it shouldn't happen, so a simple KASSERT() is probably enough, but that requires an audit of all uses of sysctl_handle_string(); else, just establish an alternative for the first SYSCTL_OUT() when outlen equals arg2 and arg1[arg2 - 1] == '\0' and force a NUL at tmparg[arg2 - 1] after the copy, and then replacing the strnlen() with a mere strlen()).

sys/kern/kern_sysctl.c

1903

I'd keep arg2 assigned, in case consumer want to write to such sysctl knob, (snip)

Please don't, this is in contradiction with current usage, the herald comment and probably common sense. Consumers can already cater to the scenario you described by just using sizeof(buff) in place of 0:

SYSCTL_STRING(_sysctl, OID_AUTO, buff, CTLFLAG_RW, buff, sizeof(buff), "Read-write string buff");

Emm, CTLFLAG_RW is wong usage.

Actually it is CTLFLAG_RDTUN that seems wrong. The generic sysctl machinery won't let you write into such a knob anyway. CTLFLAG_WRTUN must be used instead.

So I think you're trying to cater to a non-existent case here, or am I missing something?

arg2 shouldn't be modified, as explained in D48511.