Hi Greg,
This patchset includes a few backported fixes for the 4.19 stable tree. I would appreciate if you could kindly consider including them in the next release.
Thank you!
Regards, Srivatsa
---
Gen Zhang (3): efi/x86/Add missing error handling to old_memmap 1:1 mapping code ip_sockglue: Fix missing-check bug in ip_ra_control() ipv6_sockglue: Fix a missing-check bug in ip6_ra_control()
arch/x86/platform/efi/efi.c | 2 ++ arch/x86/platform/efi/efi_64.c | 9 ++++++--- net/ipv4/ip_sockglue.c | 2 ++ net/ipv6/ipv6_sockglue.c | 2 ++ 4 files changed, 12 insertions(+), 3 deletions(-)
From: Gen Zhang blackgod016574@gmail.com
commit 4e78921ba4dd0aca1cc89168f45039add4183f8e upstream.
The old_memmap flow in efi_call_phys_prolog() performs numerous memory allocations, and either does not check for failure at all, or it does but fails to propagate it back to the caller, which may end up calling into the firmware with an incomplete 1:1 mapping.
So let's fix this by returning NULL from efi_call_phys_prolog() on memory allocation failures only, and by handling this condition in the caller. Also, clean up any half baked sets of page tables that we may have created before returning with a NULL return value.
Note that any failure at this level will trigger a panic() two levels up, so none of this makes a huge difference, but it is a nice cleanup nonetheless.
[ardb: update commit log, add efi_call_phys_epilog() call on error path]
Signed-off-by: Gen Zhang blackgod016574@gmail.com Signed-off-by: Ard Biesheuvel ard.biesheuvel@linaro.org Cc: Linus Torvalds torvalds@linux-foundation.org Cc: Peter Zijlstra peterz@infradead.org Cc: Rob Bradford robert.bradford@intel.com Cc: Thomas Gleixner tglx@linutronix.de Cc: linux-efi@vger.kernel.org Link: http://lkml.kernel.org/r/20190525112559.7917-2-ard.biesheuvel@linaro.org Signed-off-by: Ingo Molnar mingo@kernel.org Signed-off-by: Srivatsa S. Bhat (VMware) srivatsa@csail.mit.edu ---
arch/x86/platform/efi/efi.c | 2 ++ arch/x86/platform/efi/efi_64.c | 9 ++++++--- 2 files changed, 8 insertions(+), 3 deletions(-)
diff --git a/arch/x86/platform/efi/efi.c b/arch/x86/platform/efi/efi.c index 9061bab..353019d 100644 --- a/arch/x86/platform/efi/efi.c +++ b/arch/x86/platform/efi/efi.c @@ -86,6 +86,8 @@ static efi_status_t __init phys_efi_set_virtual_address_map( pgd_t *save_pgd;
save_pgd = efi_call_phys_prolog(); + if (!save_pgd) + return EFI_ABORTED;
/* Disable interrupts around EFI calls: */ local_irq_save(flags); diff --git a/arch/x86/platform/efi/efi_64.c b/arch/x86/platform/efi/efi_64.c index ee5d08f..dfc809b 100644 --- a/arch/x86/platform/efi/efi_64.c +++ b/arch/x86/platform/efi/efi_64.c @@ -84,13 +84,15 @@ pgd_t * __init efi_call_phys_prolog(void)
if (!efi_enabled(EFI_OLD_MEMMAP)) { efi_switch_mm(&efi_mm); - return NULL; + return efi_mm.pgd; }
early_code_mapping_set_exec(1);
n_pgds = DIV_ROUND_UP((max_pfn << PAGE_SHIFT), PGDIR_SIZE); save_pgd = kmalloc_array(n_pgds, sizeof(*save_pgd), GFP_KERNEL); + if (!save_pgd) + return NULL;
/* * Build 1:1 identity mapping for efi=old_map usage. Note that @@ -138,10 +140,11 @@ pgd_t * __init efi_call_phys_prolog(void) pgd_offset_k(pgd * PGDIR_SIZE)->pgd &= ~_PAGE_NX; }
-out: __flush_tlb_all(); - return save_pgd; +out: + efi_call_phys_epilog(save_pgd); + return NULL; }
void __init efi_call_phys_epilog(pgd_t *save_pgd)
On Fri, Jun 28, 2019 at 11:42:13AM -0700, Srivatsa S. Bhat wrote:
From: Gen Zhang blackgod016574@gmail.com
commit 4e78921ba4dd0aca1cc89168f45039add4183f8e upstream.
The old_memmap flow in efi_call_phys_prolog() performs numerous memory allocations, and either does not check for failure at all, or it does but fails to propagate it back to the caller, which may end up calling into the firmware with an incomplete 1:1 mapping.
So let's fix this by returning NULL from efi_call_phys_prolog() on memory allocation failures only, and by handling this condition in the caller. Also, clean up any half baked sets of page tables that we may have created before returning with a NULL return value.
Note that any failure at this level will trigger a panic() two levels up, so none of this makes a huge difference, but it is a nice cleanup nonetheless.
With a description like this, why is this needed in a stable kernel if it does not really fix anything useful?
thanks,
greg k-h
On Sat, 29 Jun 2019 at 08:57, Greg KH gregkh@linuxfoundation.org wrote:
On Fri, Jun 28, 2019 at 11:42:13AM -0700, Srivatsa S. Bhat wrote:
From: Gen Zhang blackgod016574@gmail.com
commit 4e78921ba4dd0aca1cc89168f45039add4183f8e upstream.
The old_memmap flow in efi_call_phys_prolog() performs numerous memory allocations, and either does not check for failure at all, or it does but fails to propagate it back to the caller, which may end up calling into the firmware with an incomplete 1:1 mapping.
So let's fix this by returning NULL from efi_call_phys_prolog() on memory allocation failures only, and by handling this condition in the caller. Also, clean up any half baked sets of page tables that we may have created before returning with a NULL return value.
Note that any failure at this level will trigger a panic() two levels up, so none of this makes a huge difference, but it is a nice cleanup nonetheless.
With a description like this, why is this needed in a stable kernel if it does not really fix anything useful?
Because it fixes a 'CVE', remember? :-)
On Sat, Jun 29, 2019 at 10:47:00AM +0200, Ard Biesheuvel wrote:
On Sat, 29 Jun 2019 at 08:57, Greg KH gregkh@linuxfoundation.org wrote:
On Fri, Jun 28, 2019 at 11:42:13AM -0700, Srivatsa S. Bhat wrote:
From: Gen Zhang blackgod016574@gmail.com
commit 4e78921ba4dd0aca1cc89168f45039add4183f8e upstream.
The old_memmap flow in efi_call_phys_prolog() performs numerous memory allocations, and either does not check for failure at all, or it does but fails to propagate it back to the caller, which may end up calling into the firmware with an incomplete 1:1 mapping.
So let's fix this by returning NULL from efi_call_phys_prolog() on memory allocation failures only, and by handling this condition in the caller. Also, clean up any half baked sets of page tables that we may have created before returning with a NULL return value.
Note that any failure at this level will trigger a panic() two levels up, so none of this makes a huge difference, but it is a nice cleanup nonetheless.
With a description like this, why is this needed in a stable kernel if it does not really fix anything useful?
Because it fixes a 'CVE', remember? :-)
Thanks for your concerns. I have received other people disputing the CVEs these days. And if you are interested, you could kindly search all the CVEs I requested, almost all of them are marked *DISPUTED* or under update to that, haha...
Anyway, I am just a starter in requesting CVE. It's my instructor told me to request CVE for the patches... It is disputed by everybody now.
I am getting to know that a bug hard to exploit can not request CVE. We should be really careful in doing so. Right?
Thanks Gen
On Sat, Jun 29, 2019 at 10:47:00AM +0200, Ard Biesheuvel wrote:
On Sat, 29 Jun 2019 at 08:57, Greg KH gregkh@linuxfoundation.org wrote:
On Fri, Jun 28, 2019 at 11:42:13AM -0700, Srivatsa S. Bhat wrote:
From: Gen Zhang blackgod016574@gmail.com
commit 4e78921ba4dd0aca1cc89168f45039add4183f8e upstream.
The old_memmap flow in efi_call_phys_prolog() performs numerous memory allocations, and either does not check for failure at all, or it does but fails to propagate it back to the caller, which may end up calling into the firmware with an incomplete 1:1 mapping.
So let's fix this by returning NULL from efi_call_phys_prolog() on memory allocation failures only, and by handling this condition in the caller. Also, clean up any half baked sets of page tables that we may have created before returning with a NULL return value.
Note that any failure at this level will trigger a panic() two levels up, so none of this makes a huge difference, but it is a nice cleanup nonetheless.
With a description like this, why is this needed in a stable kernel if it does not really fix anything useful?
Because it fixes a 'CVE', remember? :-)
No, I don't remember that at all.
Remember, I get 1000+ emails a day to do something with, and hence, have the short-term memory of prior emails of a squirrel.
Also, CVEs mean nothing, anyone can get one and they are impossible to revoke, so don't treat them like they are "authoritative" at all.
greg k-h
On Sat, 29 Jun 2019 at 11:11, Greg KH gregkh@linuxfoundation.org wrote:
On Sat, Jun 29, 2019 at 10:47:00AM +0200, Ard Biesheuvel wrote:
On Sat, 29 Jun 2019 at 08:57, Greg KH gregkh@linuxfoundation.org wrote:
On Fri, Jun 28, 2019 at 11:42:13AM -0700, Srivatsa S. Bhat wrote:
From: Gen Zhang blackgod016574@gmail.com
commit 4e78921ba4dd0aca1cc89168f45039add4183f8e upstream.
The old_memmap flow in efi_call_phys_prolog() performs numerous memory allocations, and either does not check for failure at all, or it does but fails to propagate it back to the caller, which may end up calling into the firmware with an incomplete 1:1 mapping.
So let's fix this by returning NULL from efi_call_phys_prolog() on memory allocation failures only, and by handling this condition in the caller. Also, clean up any half baked sets of page tables that we may have created before returning with a NULL return value.
Note that any failure at this level will trigger a panic() two levels up, so none of this makes a huge difference, but it is a nice cleanup nonetheless.
With a description like this, why is this needed in a stable kernel if it does not really fix anything useful?
Because it fixes a 'CVE', remember? :-)
No, I don't remember that at all.
Remember, I get 1000+ emails a day to do something with, and hence, have the short-term memory of prior emails of a squirrel.
Also, CVEs mean nothing, anyone can get one and they are impossible to revoke, so don't treat them like they are "authoritative" at all.
To refresh your memory: I already nacked this backport once before, on the grounds that the CVE is completely bogus.
On 6/29/19 2:11 PM, Ard Biesheuvel wrote:
On Sat, 29 Jun 2019 at 11:11, Greg KH gregkh@linuxfoundation.org wrote:
On Sat, Jun 29, 2019 at 10:47:00AM +0200, Ard Biesheuvel wrote:
On Sat, 29 Jun 2019 at 08:57, Greg KH gregkh@linuxfoundation.org wrote:
On Fri, Jun 28, 2019 at 11:42:13AM -0700, Srivatsa S. Bhat wrote:
From: Gen Zhang blackgod016574@gmail.com
commit 4e78921ba4dd0aca1cc89168f45039add4183f8e upstream.
The old_memmap flow in efi_call_phys_prolog() performs numerous memory allocations, and either does not check for failure at all, or it does but fails to propagate it back to the caller, which may end up calling into the firmware with an incomplete 1:1 mapping.
So let's fix this by returning NULL from efi_call_phys_prolog() on memory allocation failures only, and by handling this condition in the caller. Also, clean up any half baked sets of page tables that we may have created before returning with a NULL return value.
Note that any failure at this level will trigger a panic() two levels up, so none of this makes a huge difference, but it is a nice cleanup nonetheless.
With a description like this, why is this needed in a stable kernel if it does not really fix anything useful?
Because it fixes a 'CVE', remember? :-)
No, I don't remember that at all.
Remember, I get 1000+ emails a day to do something with, and hence, have the short-term memory of prior emails of a squirrel.
Also, CVEs mean nothing, anyone can get one and they are impossible to revoke, so don't treat them like they are "authoritative" at all.
To refresh your memory: I already nacked this backport once before, on the grounds that the CVE is completely bogus.
Oh! I didn't know that this patch was discussed here before (this is the first time I'm posting these backports). Anyway, based on the discussion above, Greg, please ignore this patchset.
Regards, Srivatsa
From: Gen Zhang blackgod016574@gmail.com
commit 425aa0e1d01513437668fa3d4a971168bbaa8515 upstream.
In function ip_ra_control(), the pointer new_ra is allocated a memory space via kmalloc(). And it is used in the following codes. However, when there is a memory allocation error, kmalloc() fails. Thus null pointer dereference may happen. And it will cause the kernel to crash. Therefore, we should check the return value and handle the error.
Signed-off-by: Gen Zhang blackgod016574@gmail.com Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Srivatsa S. Bhat (VMware) srivatsa@csail.mit.edu ---
net/ipv4/ip_sockglue.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/net/ipv4/ip_sockglue.c b/net/ipv4/ip_sockglue.c index b7a2612..faaf688 100644 --- a/net/ipv4/ip_sockglue.c +++ b/net/ipv4/ip_sockglue.c @@ -343,6 +343,8 @@ int ip_ra_control(struct sock *sk, unsigned char on, return -EINVAL;
new_ra = on ? kmalloc(sizeof(*new_ra), GFP_KERNEL) : NULL; + if (on && !new_ra) + return -ENOMEM;
mutex_lock(&net->ipv4.ra_mutex); for (rap = &net->ipv4.ra_chain;
On Fri, Jun 28, 2019 at 11:42:30AM -0700, Srivatsa S. Bhat wrote:
From: Gen Zhang blackgod016574@gmail.com
commit 425aa0e1d01513437668fa3d4a971168bbaa8515 upstream.
In function ip_ra_control(), the pointer new_ra is allocated a memory space via kmalloc(). And it is used in the following codes. However, when there is a memory allocation error, kmalloc() fails. Thus null pointer dereference may happen. And it will cause the kernel to crash. Therefore, we should check the return value and handle the error.
Signed-off-by: Gen Zhang blackgod016574@gmail.com Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Srivatsa S. Bhat (VMware) srivatsa@csail.mit.edu
net/ipv4/ip_sockglue.c | 2 ++ 1 file changed, 2 insertions(+)
Unless you can actually trigger these kmalloc failures, I do not want to take these patches as they are not worth it at all.
thanks,
greg k-h
From: Gen Zhang blackgod016574@gmail.com
commit 95baa60a0da80a0143e3ddd4d3725758b4513825 upstream.
In function ip6_ra_control(), the pointer new_ra is allocated a memory space via kmalloc(). And it is used in the following codes. However, when there is a memory allocation error, kmalloc() fails. Thus null pointer dereference may happen. And it will cause the kernel to crash. Therefore, we should check the return value and handle the error.
Signed-off-by: Gen Zhang blackgod016574@gmail.com Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Srivatsa S. Bhat (VMware) srivatsa@csail.mit.edu ---
net/ipv6/ipv6_sockglue.c | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/net/ipv6/ipv6_sockglue.c b/net/ipv6/ipv6_sockglue.c index c0cac9c..4bc97b1 100644 --- a/net/ipv6/ipv6_sockglue.c +++ b/net/ipv6/ipv6_sockglue.c @@ -68,6 +68,8 @@ int ip6_ra_control(struct sock *sk, int sel) return -ENOPROTOOPT;
new_ra = (sel >= 0) ? kmalloc(sizeof(*new_ra), GFP_KERNEL) : NULL; + if (sel >= 0 && !new_ra) + return -ENOMEM;
write_lock_bh(&ip6_ra_lock); for (rap = &ip6_ra_chain; (ra = *rap) != NULL; rap = &ra->next) {
On Fri, Jun 28, 2019 at 11:41:54AM -0700, Srivatsa S. Bhat wrote:
Hi Greg,
This patchset includes a few backported fixes for the 4.19 stable tree. I would appreciate if you could kindly consider including them in the next release.
+ netdev@
David Miller deals with the 4.19 net/ patches, so this will need to go through (or acked by) him.
-- Thanks, Sasha
linux-stable-mirror@lists.linaro.org
-
Ard Biesheuvel -
Gen Zhang -
Greg KH -
Sasha Levin -
Srivatsa S. Bhat