| Message ID | 20200615055017.229846-1-Jason@zx2c4.com |
|---|---|
| State | New |
| Headers | show |
| Series | UBUNTU: SAUCE: (efi-lockdown) efi: ignore efivar_ssdt cmdline parameter when locked down | expand |
Sorry, I guess I should have prefixed the subject with "[SRU][B]", though I don't know if there are additional subtitles and tags beyond those ones. You'll probably also want to look whether this applies to the other kernels; I was only concerned with Bionic in my brief investigation. Should be easy enough to open up drivers/firmware/efi/efi.c and see if efivar_ssdt_setup has anything about lockdown at the top of the function.
On Sun, Jun 14, 2020 at 11:55:24PM -0600, Jason A. Donenfeld wrote: > Sorry, I guess I should have prefixed the subject with "[SRU][B]", > though I don't know if there are additional subtitles and tags beyond > those ones. You'll probably also want to look whether this applies to > the other kernels; I was only concerned with Bionic in my brief > investigation. Should be easy enough to open up > drivers/firmware/efi/efi.c and see if efivar_ssdt_setup has anything > about lockdown at the top of the function. The patch looks good to me. Acked-by: Seth Forshee <seth.forshee@canonical.com> I'm having a look at our other kernel trees to see which might need a similar patch. The lockdown patches have been evolving for a number of years now, and it looks like we missed flagging this addition to later versions as something which needed to be backported. Seth
On 15.06.20 07:50, Jason A. Donenfeld wrote: > The efivar_ssdt variable makes it possible for the root user to inject a > custom ACPI table that can be used to modify kernel memory and therefore > disable lockdown. So, this commit restricts efivar_ssdt when the kernel > is locked down. An example of this technique may be found at the link in > the trailer. > > Fixes: 49b04f8acc77 ("UBUNTU: SAUCE: (efi-lockdown) Add the ability to lock down access to the running kernel image") > Link: https://git.zx2c4.com/american-unsigned-language/tree/american-unsigned-language.sh > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> Acked-by: Stefan Bader <stefan.bader@canonical.com> > --- Formally all SRU patches need to have a public Launchpad bug open and patches refer to those with BugLink: https://bugs.launchpad.net/bugs/<#> Is there already one open? -Stefan > drivers/firmware/efi/efi.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c > index 4b4dd5532725..3d21488e35df 100644 > --- a/drivers/firmware/efi/efi.c > +++ b/drivers/firmware/efi/efi.c > @@ -228,6 +228,8 @@ static void generic_ops_unregister(void) > static char efivar_ssdt[EFIVAR_SSDT_NAME_MAX] __initdata; > static int __init efivar_ssdt_setup(char *str) > { > + if (kernel_is_locked_down("efivar-specified ACPI SSDT")) > + return -EPERM; > if (strlen(str) < sizeof(efivar_ssdt)) > memcpy(efivar_ssdt, str, strlen(str)); > else >
On Tue, Jun 16, 2020 at 07:19:20AM -0500, Seth Forshee wrote: > On Sun, Jun 14, 2020 at 11:55:24PM -0600, Jason A. Donenfeld wrote: > > Sorry, I guess I should have prefixed the subject with "[SRU][B]", > > though I don't know if there are additional subtitles and tags beyond > > those ones. You'll probably also want to look whether this applies to > > the other kernels; I was only concerned with Bionic in my brief > > investigation. Should be easy enough to open up > > drivers/firmware/efi/efi.c and see if efivar_ssdt_setup has anything > > about lockdown at the top of the function. > > The patch looks good to me. > > Acked-by: Seth Forshee <seth.forshee@canonical.com> > > I'm having a look at our other kernel trees to see which might need a > similar patch. The lockdown patches have been evolving for a number of > years now, and it looks like we missed flagging this addition to later > versions as something which needed to be backported. > > Seth From what I saw, this would be needed in 5.3 and 5.0 kernels too. And the patch applies cleanly on those two. For 5.4 forward, there is already 1957a85b0032a81e6482ca4aab883643b8dae06e ("efi: Restrict efivar_ssdt_load when the kernel is locked down"). Cascardo.
On Tue, Jun 16, 2020 at 02:49:44PM +0200, Stefan Bader wrote: > On 15.06.20 07:50, Jason A. Donenfeld wrote: > > The efivar_ssdt variable makes it possible for the root user to inject a > > custom ACPI table that can be used to modify kernel memory and therefore > > disable lockdown. So, this commit restricts efivar_ssdt when the kernel > > is locked down. An example of this technique may be found at the link in > > the trailer. > > > > Fixes: 49b04f8acc77 ("UBUNTU: SAUCE: (efi-lockdown) Add the ability to lock down access to the running kernel image") > > Link: https://git.zx2c4.com/american-unsigned-language/tree/american-unsigned-language.sh > > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > Acked-by: Stefan Bader <stefan.bader@canonical.com> > > --- > > Formally all SRU patches need to have a public Launchpad bug open and patches > refer to those with > > BugLink: https://bugs.launchpad.net/bugs/<#> > > Is there already one open? I've identified other lockdown enhancements which should be backported, so I'll create a bug and send them all as a group once the backports are done. > > -Stefan > > > drivers/firmware/efi/efi.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c > > index 4b4dd5532725..3d21488e35df 100644 > > --- a/drivers/firmware/efi/efi.c > > +++ b/drivers/firmware/efi/efi.c > > @@ -228,6 +228,8 @@ static void generic_ops_unregister(void) > > static char efivar_ssdt[EFIVAR_SSDT_NAME_MAX] __initdata; > > static int __init efivar_ssdt_setup(char *str) > > { > > + if (kernel_is_locked_down("efivar-specified ACPI SSDT")) > > + return -EPERM; > > if (strlen(str) < sizeof(efivar_ssdt)) > > memcpy(efivar_ssdt, str, strlen(str)); > > else > > > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
On Tue, Jun 16, 2020 at 6:50 AM Stefan Bader <stefan.bader@canonical.com> wrote: > > On 15.06.20 07:50, Jason A. Donenfeld wrote: > > The efivar_ssdt variable makes it possible for the root user to inject a > > custom ACPI table that can be used to modify kernel memory and therefore > > disable lockdown. So, this commit restricts efivar_ssdt when the kernel > > is locked down. An example of this technique may be found at the link in > > the trailer. > > > > Fixes: 49b04f8acc77 ("UBUNTU: SAUCE: (efi-lockdown) Add the ability to lock down access to the running kernel image") > > Link: https://git.zx2c4.com/american-unsigned-language/tree/american-unsigned-language.sh > > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > Acked-by: Stefan Bader <stefan.bader@canonical.com> > > --- > > Formally all SRU patches need to have a public Launchpad bug open and patches > refer to those with > > BugLink: https://bugs.launchpad.net/bugs/<#> > > Is there already one open? I'll leave it to you all to do the administrivia. I simply did some volunteer security research and provided a patch; the intricacies of paperwork for getting this into your commercial distro seem a bit above my pay grade.
Hi Jason, Seth included a similar patch in the patch set he mentioned in this email thread. You can see that patch set here: https://lists.ubuntu.com/archives/kernel-team/2020-June/111231.html Patch Series: [PATCH 0/6][B] Lockdown updates Patch: [PATCH 2/6][B] efi: Restrict efivar_ssdt_load when the kernel is locked down The above patch set is in a list of patches I'm currently applying to Bionic/master-next. Thank you for taking the time to submit and your help! :) -Kelsey On 2020-06-14 23:50:17 , Jason A. Donenfeld wrote: > The efivar_ssdt variable makes it possible for the root user to inject a > custom ACPI table that can be used to modify kernel memory and therefore > disable lockdown. So, this commit restricts efivar_ssdt when the kernel > is locked down. An example of this technique may be found at the link in > the trailer. > > Fixes: 49b04f8acc77 ("UBUNTU: SAUCE: (efi-lockdown) Add the ability to lock down access to the running kernel image") > Link: https://git.zx2c4.com/american-unsigned-language/tree/american-unsigned-language.sh > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > --- > drivers/firmware/efi/efi.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c > index 4b4dd5532725..3d21488e35df 100644 > --- a/drivers/firmware/efi/efi.c > +++ b/drivers/firmware/efi/efi.c > @@ -228,6 +228,8 @@ static void generic_ops_unregister(void) > static char efivar_ssdt[EFIVAR_SSDT_NAME_MAX] __initdata; > static int __init efivar_ssdt_setup(char *str) > { > + if (kernel_is_locked_down("efivar-specified ACPI SSDT")) > + return -EPERM; > if (strlen(str) < sizeof(efivar_ssdt)) > memcpy(efivar_ssdt, str, strlen(str)); > else > -- > 2.27.0 > > > -- > kernel-team mailing list > kernel-team@lists.ubuntu.com > https://lists.ubuntu.com/mailman/listinfo/kernel-team
On Thu, Jul 16, 2020 at 10:42:51PM -0600, Kelsey Skunberg wrote: > Hi Jason, > > Seth included a similar patch in the patch set he mentioned in this > email thread. You can see that patch set here: > > https://lists.ubuntu.com/archives/kernel-team/2020-June/111231.html > > Patch Series: [PATCH 0/6][B] Lockdown updates > Patch: [PATCH 2/6][B] efi: Restrict efivar_ssdt_load when the kernel is locked down > > The above patch set is in a list of patches I'm currently applying to > Bionic/master-next. > > Thank you for taking the time to submit and your help! :) Yes, sorry, I meant to send a nack after I sent those. I guess I forgot. Jason, since there was an upstream commit for this particular change I included a (virtually identical) backport of that instead of using your patch. Having it as a backport helps our scripting in cases where we want to know if a particular upstream commit as been backported to our supported kernels. > > -Kelsey > > On 2020-06-14 23:50:17 , Jason A. Donenfeld wrote: > > The efivar_ssdt variable makes it possible for the root user to inject a > > custom ACPI table that can be used to modify kernel memory and therefore > > disable lockdown. So, this commit restricts efivar_ssdt when the kernel > > is locked down. An example of this technique may be found at the link in > > the trailer. > > > > Fixes: 49b04f8acc77 ("UBUNTU: SAUCE: (efi-lockdown) Add the ability to lock down access to the running kernel image") > > Link: https://git.zx2c4.com/american-unsigned-language/tree/american-unsigned-language.sh > > Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> > > --- > > drivers/firmware/efi/efi.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c > > index 4b4dd5532725..3d21488e35df 100644 > > --- a/drivers/firmware/efi/efi.c > > +++ b/drivers/firmware/efi/efi.c > > @@ -228,6 +228,8 @@ static void generic_ops_unregister(void) > > static char efivar_ssdt[EFIVAR_SSDT_NAME_MAX] __initdata; > > static int __init efivar_ssdt_setup(char *str) > > { > > + if (kernel_is_locked_down("efivar-specified ACPI SSDT")) > > + return -EPERM; > > if (strlen(str) < sizeof(efivar_ssdt)) > > memcpy(efivar_ssdt, str, strlen(str)); > > else > > -- > > 2.27.0 > > > > > > -- > > kernel-team mailing list > > kernel-team@lists.ubuntu.com > > https://lists.ubuntu.com/mailman/listinfo/kernel-team
Hi Kelsey, Seth, I saw that you backported a different patch the day it was pushed into the repos. No problem at all. Thanks for being on top of things. Regards, Jason
diff --git a/drivers/firmware/efi/efi.c b/drivers/firmware/efi/efi.c index 4b4dd5532725..3d21488e35df 100644 --- a/drivers/firmware/efi/efi.c +++ b/drivers/firmware/efi/efi.c @@ -228,6 +228,8 @@ static void generic_ops_unregister(void) static char efivar_ssdt[EFIVAR_SSDT_NAME_MAX] __initdata; static int __init efivar_ssdt_setup(char *str) { + if (kernel_is_locked_down("efivar-specified ACPI SSDT")) + return -EPERM; if (strlen(str) < sizeof(efivar_ssdt)) memcpy(efivar_ssdt, str, strlen(str)); else
The efivar_ssdt variable makes it possible for the root user to inject a custom ACPI table that can be used to modify kernel memory and therefore disable lockdown. So, this commit restricts efivar_ssdt when the kernel is locked down. An example of this technique may be found at the link in the trailer. Fixes: 49b04f8acc77 ("UBUNTU: SAUCE: (efi-lockdown) Add the ability to lock down access to the running kernel image") Link: https://git.zx2c4.com/american-unsigned-language/tree/american-unsigned-language.sh Signed-off-by: Jason A. Donenfeld <Jason@zx2c4.com> --- drivers/firmware/efi/efi.c | 2 ++ 1 file changed, 2 insertions(+)