lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Mon, 6 Nov 2017 17:41:29 +0000 From: Dave Martin <Dave.Martin@....com> To: Arnd Bergmann <arnd@...db.de> Cc: Catalin Marinas <Catalin.Marinas@....com>, Will Deacon <Will.Deacon@....com>, "Luis R. Rodriguez" <mcgrof@...nel.org>, Kees Cook <keescook@...omium.org>, Alex Bennée <alex.bennee@...aro.org>, "Eric W. Biederman" <ebiederm@...ssion.com>, Andrew Morton <akpm@...ux-foundation.org>, Thomas Gleixner <tglx@...utronix.de>, "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>, "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org> Subject: Re: [PATCH] sysctl: add register_sysctl() dummy helper On Mon, Nov 06, 2017 at 05:12:03PM +0000, Arnd Bergmann wrote: > On Mon, Nov 6, 2017 at 5:50 PM, Dave Martin <Dave.Martin@....com> wrote: > > On Mon, Nov 06, 2017 at 01:36:40PM +0000, Arnd Bergmann wrote: > >> The register_sysctl() function has been around for five years with commit > >> fea478d4101a ("sysctl: Add register_sysctl for normal sysctl users") > >> but now that arm64 started using it, I ran into a compile error: > >> > >> arch/arm64/kernel/armv8_deprecated.c: In function 'register_insn_emulation_sysctl': > >> arch/arm64/kernel/armv8_deprecated.c:257:2: error: implicit declaration of function 'register_sysctl' > > > > Hmmm, looks like I missed this combination in testing. > > > > I wonder whether ARMV8_DEPRECATED without SYSCTL is really a good idea > > though: in that config, we build a lot of dead code and leak some > > memory today. The default emulation is still potentially useful, but > > all the support for runtime twiddling of the emulation modes becomes > > useless. > > > > For parallel reasons, the SVE sysctl stuff is protected by #ifdef > > CONFIG_SYSCTL, which is why I didn't get a similar splat there. > > > > > > So, although this patch is superficially sensible, it may tend to hide > > bugs: code that calls register_sysctl() when CONFIG_SYSCTL=n is > > suspicious and probably needs review... no? > > I think your analysis for this code is correct, we waste a lot of memory > if we do it like this. However in the general case where we register a > statically allocated 'struct ctl_table', it wouldn't be an issue, because > gcc could then eliminate all the dead code. > > Adding a CONFIG_SYSCTL #ifdef or Kconfig dependency would > probably be reasonable for armv8_deprecated.c, for the rest of > the kernel, having that wrapper is probably better. > > I don't really care how this gets fixed, as long as some solution gets > merged. Well, since your patch makes things more consistent, I'm happy to Reviewed-by: Dave Martin <Dave.Martin@....com> The armv8_deprecated code looks suspicious in any case, so I'll propose a fix for that separately -- adding a Kconfig dependency seems simplest. Cheers ---Dave
Powered by blists - more mailing lists