From owner-freebsd-current@freebsd.org Sat Feb 15 14:40:39 2020 Return-Path: Delivered-To: freebsd-current@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id B029A23D379 for ; Sat, 15 Feb 2020 14:40:39 +0000 (UTC) (envelope-from se@freebsd.org) Received: from smtp.freebsd.org (smtp.freebsd.org [IPv6:2610:1c1:1:606c::24b:4]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "smtp.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 48KXwl4G7lz4KP2; Sat, 15 Feb 2020 14:40:39 +0000 (UTC) (envelope-from se@freebsd.org) Received: from Stefans-MBP-449.fritz.box (p200300CD5F0C3500D4E12A243610A3C6.dip0.t-ipconnect.de [IPv6:2003:cd:5f0c:3500:d4e1:2a24:3610:a3c6]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client did not present a certificate) (Authenticated sender: se/mail) by smtp.freebsd.org (Postfix) with ESMTPSA id 20ECF17EFD; Sat, 15 Feb 2020 14:40:39 +0000 (UTC) (envelope-from se@freebsd.org) Subject: Re: option KDTRACE_HOOKS non-optional after r357912? To: Mateusz Guzik Cc: freebsd-current@freebsd.org References: From: =?UTF-8?Q?Stefan_E=c3=9fer?= Message-ID: <57662d12-5c9f-c947-70ca-d1032b82fb3b@freebsd.org> Date: Sat, 15 Feb 2020 15:40:35 +0100 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:68.0) Gecko/20100101 Thunderbird/68.5.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: de-DE Content-Transfer-Encoding: 8bit X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 15 Feb 2020 14:40:39 -0000 Am 15.02.20 um 14:47 schrieb Mateusz Guzik: > On 2/15/20, Stefan Eßer 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. 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