On Tue, Jun 14, 2016 at 3:17 PM, Emese Revfy <re.emese@xxxxxxxxx> wrote: > I would like to introduce the latent_entropy gcc plugin. This plugin mitigates > the problem of the kernel having too little entropy during and after boot > for generating crypto keys. > > This plugin mixes random values into the latent_entropy global variable > in functions marked by the __latent_entropy attribute. > The value of this global variable is added to the kernel entropy pool > to increase the entropy. > > It is a CII project supported by the Linux Foundation. > > The latent_entropy plugin was ported from grsecurity/PaX originally written by > the PaX Team. You can find more about the plugin here: > https://grsecurity.net/pipermail/grsecurity/2012-July/001093.html > > The plugin supports all gcc version from 4.5 to 6.0. > > I do some changes above the PaX version. The important one is mixing > the stack pointer into the global variable too. > You can find more about the changes here: > https://github.com/ephox-gcc-plugins/latent_entropy > > This patch set is based on the "Introduce GCC plugin infrastructure" patch set (v9 next-20160520). > > Emese Revfy (4): > Add support for passing gcc plugin arguments > Add the latent_entropy gcc plugin > Mark functions with the latent_entropy attribute > Add the extra_latent_entropy kernel parameter > > > Changes from v2: > * Moved the passing of gcc plugin arguments into a separate patch > (Suggested-by: Kees Cook <keescook@xxxxxxxxxxxx>) > * Mix the global entropy variable with the stack pointer (latent_entropy_plugin.c) > * Handle tail calls (latent_entropy_plugin.c) > * Fix some indentation related warnings suggested by checkpatch.pl (latent_entropy_plugin.c) > * Commented some latent_entropy plugin code > (Suggested-by: Kees Cook <keescook@xxxxxxxxxxxx>) This continues to look great to me, thanks for all the added comments and improvements. One thing I'd still really like to see is that each patch passes script/checkpatch.pl with as few issues as possible. The >80 character lines are a problem, and I think they impede readability. Quoting from Documentation/CodingStyle: The limit on the length of lines is 80 columns and this is a strongly preferred limit. Statements longer than 80 columns will be broken into sensible chunks, unless exceeding 80 columns significantly increases readability and does not hide information. Descendants are always substantially shorter than the parent and are placed substantially to the right. The same applies to function headers with a long argument list. However, never break user-visible strings such as printk messages, because that breaks the ability to grep for them. Personally, I'm okay with exceptions where they make sense, but they should be exceptions, as opposed to being common. Right now in this series, there are a lot of cases of long lines even in places where it isn't code, like in commit messages and Kconfig text. Here's the current output from checkpatch.pl: 0002-Add-the-latent_entropy-gcc-plugin.patch total: 5 errors, 54 warnings, 702 lines checked 0003-Mark-functions-with-the-latent_entropy-attribute.patch total: 2 errors, 8 warnings, 0 checks, 197 lines checked 0004-Add-the-extra_latent_entropy-kernel-parameter.patch total: 0 errors, 3 warnings, 65 lines checked The 54 warnings in the plugin should get vastly reduced. You can ignore things that aren't correct, like: ERROR: Macros with complex values should be enclosed in parentheses #148: FILE: include/linux/init.h:42: +#define __init __section(.init.text) __cold notrace __latent_entropy Obviously __init isn't a "regular" macro, etc. And other misparsings can be ignored, like: ERROR: spaces required around that '>' (ctx:VxW) #310: FILE: scripts/gcc-plugins/latent_entropy_plugin.c:127: + vec<constructor_elt, va_gc> *vals; ^ since checkpatch doesn't know about such things. But things like this should be fixed: WARNING: line over 80 characters #157: FILE: include/linux/init.h:95: +#define __meminit __section(.meminit.text) __cold notrace __latent_entropy I'd expect it to look like: #define __meminit __section(.meminit.text) __cold notrace \ __latent_entropy And for function declarations: WARNING: line over 80 characters #185: FILE: kernel/fork.c:409: +static __latent_entropy int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm) static __latent_entropy int dup_mmap(struct mm_struct *mm, struct mm_struct *oldmm) and for long code lines: WARNING: line over 80 characters #774: FILE: scripts/gcc-plugins/latent_entropy_plugin.c:591: + register_callback(plugin_name, PLUGIN_REGISTER_GGC_ROOTS, NULL, (void *)>_ggc_r_gt_latent_entropy); register_callback(plugin_name, PLUGIN_REGISTER_GGC_ROOTS, NULL, (void *)>_ggc_r_gt_latent_entropy); Cases of long text lines are an exception, though: WARNING: line over 80 characters #322: FILE: scripts/gcc-plugins/latent_entropy_plugin.c:139: + error("variable %qD with %qE attribute must not be initialized", *node, name); This should leave the text unbroken, but wrap the next part: error("variable %qD with %qE attribute must not be initialized", *node, name); Thanks! -Kees -- Kees Cook Chrome OS & Brillo Security -- To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html