axgbe: bound the sysctl parser writes in get_val()

[ojanerif_amd.com]

Summary

get_val() copied each option and value token into fixed 32-byte buffers
and indexed a 32-entry pointer array without bounding the token length
or the option count, overflowing on long input. Reject input that
doesn't fit, fix the terminating NUL offset, and have the handlers
check parse_generic_sysctl()'s return value.

[kbowling]

sys/dev/axgbe/xgbe-sysctl.c

274–278

would mallocarray clean this up a bit?

[ojanerif_amd.com]

Thanks. The only allocation here is in alloc_sysctl_buffer(), and both multiplications are compile-time constants (AXGBE_SYSCTL_NOPS/AXGBE_SYSCTL_OPLEN = 32 times sizeof(char *)/sizeof(char)), so there's no overflow risk to guard against. get_val() itself doesn't allocate. Switched both to mallocarray() anyway as a cleaner idiom match, no functional change. Uploading v2.

[kbowling]

sys/dev/axgbe/xgbe-sysctl.c

284

When a token reaches 31 characters, the loop stops mid-token. The surrounding code then advances i as though it encountered a space. This silently truncates values and can interpret their remaining characters as another option.

I would make get_val() report an error when a token exceeds the limit, before applying settings. Propagate that error through the handlers too: they currently ignore parse_generic_sysctl()’s return value.

[ojanerif_amd.com]

Done. get_val() now returns -EINVAL when a token or the option count
doesn't fit, and the handlers return parse_generic_sysctl()'s error
before touching the hardware. Checking that return value exposed a
missing break in the SYSL_U8 case, which made every duplex value fail,
so that's fixed too.

[kbowling]

The use of -EINVAL is not correct for FreeBSD userspace; in some drivers like intel we tend to "sanitize" these with a function that maps them correctly to FreeBSD positive values (see i.e. ice(4)). In this case I'd probably just rewrite all the values though unless there is some shared OS code we are syncing against. It doesn't need to be done in this commit but probably should be a quick followup.

sys/dev/axgbe/xgbe-sysctl.c

933

This is correct but exposes yet another existing error that will now be fatal. We need to handle SYSL_U32 in the parser.