Re: option KDTRACE_HOOKS non-optional after r357912?

On Sat, Feb 15, 2020 at 03:58:06PM +0100, Stefan Eßer wrote:
> 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 ...
> 
Try https://reviews.freebsd.org/D23699

> > 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"
> > 
> _______________________________________________
> 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"