pf: Limit ioctl to a reasonable and tuneable number of elements

[kp]

Summary

pf ioctls frequently take a variable number of elements as argument. This can potentially allow users to request very large allocations.
These will fail, but even a failing M_NOWAIT might tie up resources and result in concurrent M_WAITOK allocations entering vm_wait and inducing reclamation of caches.

Limit these ioctls to what should be a reasonable value, but allow users to tune it should they need to.

[cem]

Looks good in general. It seems that some cases use WOULD_OVERFLOW as a secondary check and some don't. Is there any reason for that?

Maybe the tuned value could be clamped to a limit that avoids WOULD_OVERFLOW problems (SIZE_MAX / <largest item size>?), and WOULD_OVERFLOW could be dropped from each conditional.

sys/netpfil/pf/pf_ioctl.c

219 ↗

(On Diff #41282)

mismatch between extern linkage here and static in definition.

[kp]

Add missing WOULD_OVERFLOW checks.

[kp]

Fix static / extern mismatch

[kp]

In D15018#316088, @cem wrote:

Looks good in general. It seems that some cases use WOULD_OVERFLOW as a secondary check and some don't. Is there any reason for that?

Good catch, I missed it in the first two cases because that used to be a fixed 65k value. Now that it's user configurable it needs the check too.

Maybe the tuned value could be clamped to a limit that avoids WOULD_OVERFLOW problems (SIZE_MAX / <largest item size>?), and WOULD_OVERFLOW could be dropped from each conditional.

That would be possible too, but I'm a bit worried that this'll get extended to other ioctl commands and we'll forget to update the maximum value.

[cem]

In D15018#316121, @kristof wrote:

In D15018#316088, @cem wrote:

Looks good in general. It seems that some cases use WOULD_OVERFLOW as a secondary check and some don't. Is there any reason for that?

Good catch, I missed it in the first two cases because that used to be a fixed 65k value. Now that it's user configurable it needs the check too.

Thanks :-)

Maybe the tuned value could be clamped to a limit that avoids WOULD_OVERFLOW problems (SIZE_MAX / <largest item size>?), and WOULD_OVERFLOW could be dropped from each conditional.

That would be possible too, but I'm a bit worried that this'll get extended to other ioctl commands and we'll forget to update the maximum value.

Yeah, makes sense.

Patch looks good to me.