added patches for e1000e Kaby Lake support #46
Conversation
zenmonkeykstop
force-pushed
the
e1000e-patch-build
branch
from
72e892c to
4da9736
Compare
There was a problem hiding this comment.
I think that having the patches checked into this repository makes sense and the changes proposed look good to me as-is.
I've tested the functionality of these changes as per the following test plan:
- Confirmed patches are the ones discussed in Add latest version of e1000e module, built for grsec kernel securedrop#4024 (comment)
- Confirmed patches applied correctly and build successful
- Confirm resulting kernel image boots and functions correctly, with the same configuration as previously shipped 4.4.x series kernels
- Confirm e1000e driver works (Unfortunately I do not have the hardware to test this)
Built a 4.4.177 kernel on this branch and everything looks good!
Resulting kernel built on this branch has zero changes from current config, so not worth updating:
diff --git a/files/config-securedrop-4.4 b/files/config-securedrop-4.4
index 131e8b5..d027c7f 100644
--- a/files/config-securedrop-4.4
+++ b/files/config-securedrop-4.4
@@ -1,6 +1,6 @@
#
# Automatically generated file; DO NOT EDIT.
-# Linux/x86 4.4.167 Kernel Configuration
+# Linux/x86 4.4.177 Kernel Configuration
|
@zenmonkeykstop Would you mind re-submitting this PR via a local branch on this repo? You should have write access already. @emkll's right that we have CI disabled on forks, and it'd be great to have an automated check of this new logic on top of the manual verification we've been doing. |
I take that back: the CI run doesn't include the scenario that this logic touches, so we wouldn't learn anything new. I'm going to override the wait and merge as-is, given that we've already verified the logic by building locally. Next step will be re-verifying support on hardware, which can happen over in the SecureDrop repo. |
|
One more thing: we may not want these patches applied to all scenarios. Worth testing against 4.14 to see if we need to bother making these patches conditional right now, or whether we can defer and follow up. @emkll has already kicked off a 4.14 build on this branch, if it's clean, fine to merge as -is. |
There was a problem hiding this comment.
Bad news, the patches don't apply cleanly on 4.14:
TASK [ansible-role-grsecurity-build : Apply e1000e i219lm patch.] **************
An exception occurred during task execution. To see the full traceback, use -vvv. The error was: 1 out of 1 hunk FAILED
fatal: [grsecurity-build-stable3-stretch]: FAILED! => {"changed": false, "msg": "1 out of 1 hunk FAILED\n1 out of 1 hunk FAILED\n1 out of 1 hunk FAILED\n"}
PLAY RECAP *********************************************************************
grsecurity-build-stable3-stretch : ok=27 changed=12 unreachable=0 failed=1
ERROR:
While we don't need to address this immediately for the purposes of shipping e1000e enabled kernels to SecureDrop instances, I don't think we should merge until conditional patching logic is applied (maybe limiting to grsecurity_build_patch_type=stable2?)
It might be best to
|
Roger that, @emkll, thanks for the quick feedback. I'll take on a commit with some conditional logic, off by default, suitable for test-driving on the 4.14 path. |
The e1000e patches are intended to provide greater hardware support, specifically for SecureDrop recommended hardware. The patches won't apply to 4.14 kernels, though, so by default let's make applying the patches conditional on patch type. The list of patches can still be overridden at the site level to any list of custom filenames. Using the `basename` filter to support fullpaths to reference local patches.
The order in which the custom patches are applied is crucial. I wasn't careful when tweaking @zenmonkeykstop's custom patch logic, and the wrong order caused patching to fail, which broke the build. Fixed that issue, and tacked on a comment stating that order is important.
There was a problem hiding this comment.
87bd5eb does indeed resolve the patch order, and the conditional logic works as expected:
molecule converge -s workstationwhich uses the stable3 patches (4.14 series) does not have the e1000e patches applied, and the playbook does not bail prior to the building stepmolecule converge -s securedrop-dockerwhich uses the stable2 patches (4.4 series) apply the patches cleanly and the build process is kicked off
|
We have CI configured not to run on forks here. That's something we should perhaps change. For now, however, I'm merging via override, given the substantial local testing performed by @emkll and myself. |
These patches were already included in the SD kernel build process as of freedomofpress/ansible-role-grsecurity-build#46
These patches were already included in the SD kernel build process as of freedomofpress/ansible-role-grsecurity-build#46
These patches were already included in the SD kernel build process as of freedomofpress/ansible-role-grsecurity-build#46
Includes patches for SecureDrop server hw support. These patches were already included in the SD kernel build process as of freedomofpress/ansible-role-grsecurity-build#46
Includes patches for SecureDrop server hw support. These patches were already included in the SD kernel build process as of freedomofpress/ansible-role-grsecurity-build#46 Creates separate targets for SecureDrop kernels on 4.14.x vs 5.4.x. For reference, the "patch_type" from Grsecurity maps to kernel versions as: stable3 -> 4.14.x stable4 -> 5.4.x Includes script tweaks so that patches & configs are optional.
Includes custom configs already in use for SecureDrop contexts, both servers and Workstation. Does not include outdated e1000e patches [0]. The patches pulled in from the legacy build logic were only relevant to kernel versions 4.4 and below, i.e. "stable2" in grsec nomenclature. We're using at least 4.14 across the board now, so those patches are moot. Creates separate targets for SecureDrop kernels on 4.14.x vs 5.4.x. For reference, the "patch_type" from Grsecurity maps to kernel versions as: stable3 -> 4.14.x stable4 -> 5.4.x Includes script tweaks so that patches & configs are optional. [0] freedomofpress/ansible-role-grsecurity-build#46
(Work in progress)
Adds two kernel patches for e1000e driver support on 7th-gen Intel NUCs.
This could almost certainly be done more cleanly, and the patches are checked in rather than being pulled in and verified at build time, hence "work in progress".
Tested against NUC7i5BNH hardware, not yet tested against existing hardware recommendations.