[4.19.y PATCH 0/3] Backported fixes for 4.19 stable tree

[4.19.y PATCH 0/3] Backported fixes for 4.19 stable tree

[Srivatsa S. Bhat]

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(-)

[4.19.y PATCH 1/3] efi/x86/Add missing error handling to old_memmap 1:1 mapping code

[Srivatsa S. Bhat]

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)

Re: [4.19.y PATCH 1/3] efi/x86/Add missing error handling to old_memmap 1:1 mapping code

[Greg KH]

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

Re: [4.19.y PATCH 1/3] efi/x86/Add missing error handling to old_memmap 1:1 mapping code

[Ard Biesheuvel]

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? :-)

Re: [4.19.y PATCH 1/3] efi/x86/Add missing error handling to old_memmap 1:1 mapping code

[Gen Zhang]

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

Re: [4.19.y PATCH 1/3] efi/x86/Add missing error handling to old_memmap 1:1 mapping code

[Greg KH]

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

Re: [4.19.y PATCH 1/3] efi/x86/Add missing error handling to old_memmap 1:1 mapping code

[Ard Biesheuvel]

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.

Re: [4.19.y PATCH 1/3] efi/x86/Add missing error handling to old_memmap 1:1 mapping code

[Srivatsa S. Bhat]

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

[4.19.y PATCH 2/3] ip_sockglue: Fix missing-check bug in ip_ra_control()

[Srivatsa S. Bhat]

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;

Re: [4.19.y PATCH 2/3] ip_sockglue: Fix missing-check bug in ip_ra_control()

[Greg KH]

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

[4.19.y PATCH 3/3] ipv6_sockglue: Fix a missing-check bug in ip6_ra_control()

[Srivatsa S. Bhat]

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) {

Re: [4.19.y PATCH 0/3] Backported fixes for 4.19 stable tree

[Sasha Levin]

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