Re: option KDTRACE_HOOKS non-optional after r357912?

Am 15.02.20 um 15:40 schrieb Stefan Eßer:
> Am 15.02.20 um 14:47 schrieb Mateusz Guzik:
>> On 2/15/20, Stefan Eßer <st.esser_at_yahoo.de> wrote:
>>> Hi Mateusz,
>>>
>>> your optimization of systrace checks has made KDTRACE_HOOKS mandatory,
>>> since there are unprotected assignments to systrace_enabled (which is
>>> defined as constant 0 in kernels without KDTRACE_HOOKS due to your
>>> change):
>>>
>>> /sys/cddl/dev/systrace/systrace.c:322:20: error: expression is not
>>> assignable
>>>                 systrace_enabled = true;
>>>                 ~~~~~~~~~~~~~~~~ ^
>>> /sys/cddl/dev/systrace/systrace.c:334:20: error: expression is not
>>> assignable
>>>                 systrace_enabled = false;
>>>                 ~~~~~~~~~~~~~~~~ ^
>>> 2 errors generated.
>>> *** [systrace.o] Error code 1
>>>
>>> The easy work-around is of course to add KDTRACE_HOOKS to the stripped
>>> down kernel configuration. But I think there should be stab functions
>>> in systrace.c to cover the case that this option is not active.
>>>
>>> Or is the overhead and other impact of KDTRACE_HOOKS considered to be
>>> so insignificant that it should be included in every kernel?
>>
>> Well tinderbox built for me.
> 
> Yes, no surprise, KDTRACE_HOOKS is defined in all the GENERIC kernels.
> 
>> Note that the module strongly depends on KDTRACE_HOOKS to work in the
>> first place -- even prior to my patch support in the syscall path was gated by
>> this define. In other words, the module should not be being built if the option
>> is not enabled. Thus if anything the change adds an unintended improvement
>> of catching the lack of dependency checking here. I may take a closer look
>> later but preferably someone familiar with the build system would take
>> care of it.
> 
> If KDTRACE_HOOKS is meant to be kept optional, the hooks should not be
> compiled in and the functions to enable that feature should return
> failure, IMHO.

A clarification: The above was of course meant for the case that the
option has not been specified. It has been activated, I do not expect
the enable function to return failure ...

> It is obviously not useful to load a module that depends on an optional
> feature, if that feature has been disabled. And I do not do this.
> 
> I just want to build a stripped down kernel, but have not restricted the
> modules that get built. I just do not load those that I do not need.
> 
> But the current situation makes buildkernel fail (unless KDTRACE_HOOKS
> is defined or the systrace module explicitly disabled), and that is at
> least a violation of POLA.
> 
>> It comes with some overhead of course since there is no hot patching, but
>> it is unlikely you will be able to measure it because of other factors
> 
> Yes, but in a stripped down production kernel where these hooks will
> never be legitimately used, they do not only add overhead but also
> attack surface, and so I do not want to include them.
> 
> Regards, STefan
> _______________________________________________
> freebsd-current_at_freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/freebsd-current
> To unsubscribe, send any mail to "freebsd-current-unsubscribe_at_freebsd.org"
>