linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] usercopy: Do not fail on memory from former init sections
@ 2022-01-07  0:19 Helge Deller
  2022-01-07 23:44 ` Andrew Morton
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Helge Deller @ 2022-01-07  0:19 UTC (permalink / raw)
  To: Linux Kernel, Andrew Morton, linux-mm; +Cc: linux-parisc

On some platforms the memory area between the _stext and the _etext
symbols includes the init sections (parisc and csky). If the init
sections are freed after bootup, the kernel may reuse this memory.

In one test the usercopy checks if the given address is inside the .text
section (from _stext to _etext), and it wrongly fails on the mentioned
platforms if the memory is from the former init section.

Fix this failure by first checking against the init sections before
checking against the _stext/_etext section.

Signed-off-by: Helge Deller <deller@gmx.de>
Fixes: 98400ad75e95 ("parisc: Fix backtrace to always include init funtion names")

diff --git a/mm/usercopy.c b/mm/usercopy.c
index b3de3c4eefba..37a35c6051bc 100644
--- a/mm/usercopy.c
+++ b/mm/usercopy.c
@@ -113,6 +113,15 @@ static bool overlaps(const unsigned long ptr, unsigned long n,
 	return true;
 }

+static bool inside_init_area(const unsigned long ptr, unsigned long n,
+		char *start, char *end)
+{
+	unsigned long initlow = (unsigned long) start;
+	unsigned long inithigh = (unsigned long) end;
+
+	return (ptr >= initlow && (ptr + n) < inithigh);
+}
+
 /* Is this address range in the kernel text area? */
 static inline void check_kernel_text_object(const unsigned long ptr,
 					    unsigned long n, bool to_user)
@@ -121,6 +130,12 @@ static inline void check_kernel_text_object(const unsigned long ptr,
 	unsigned long texthigh = (unsigned long)_etext;
 	unsigned long textlow_linear, texthigh_linear;

+	/* Ok if inside the former init sections */
+	if (inside_init_area(ptr, n, __init_begin, __init_end))
+		return;
+	if (inside_init_area(ptr, n, _sinittext, _einittext))
+		return;
+
 	if (overlaps(ptr, n, textlow, texthigh))
 		usercopy_abort("kernel text", NULL, to_user, ptr - textlow, n);


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] usercopy: Do not fail on memory from former init sections
  2022-01-07  0:19 [PATCH] usercopy: Do not fail on memory from former init sections Helge Deller
@ 2022-01-07 23:44 ` Andrew Morton
  2022-01-07 23:46 ` Andrew Morton
  2022-01-07 23:51 ` Andrew Morton
  2 siblings, 0 replies; 5+ messages in thread
From: Andrew Morton @ 2022-01-07 23:44 UTC (permalink / raw)
  To: Helge Deller; +Cc: Linux Kernel, linux-mm, linux-parisc

On Fri, 7 Jan 2022 01:19:24 +0100 Helge Deller <deller@gmx.de> wrote:

> On some platforms the memory area between the _stext and the _etext
> symbols includes the init sections (parisc and csky). If the init
> sections are freed after bootup, the kernel may reuse this memory.
> 
> In one test the usercopy checks if the given address is inside the .text
> section (from _stext to _etext), and it wrongly fails on the mentioned
> platforms if the memory is from the former init section.
> 
> Fix this failure by first checking against the init sections before
> checking against the _stext/_etext section.

This sounds like it might have very serious runtime effects?

Please always fully describe a bug's runtime effects when fixing that bug.

> Fixes: 98400ad75e95 ("parisc: Fix backtrace to always include init funtion names")

So is this a must-have for 5.16?


^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] usercopy: Do not fail on memory from former init sections
  2022-01-07  0:19 [PATCH] usercopy: Do not fail on memory from former init sections Helge Deller
  2022-01-07 23:44 ` Andrew Morton
@ 2022-01-07 23:46 ` Andrew Morton
  2022-01-07 23:51 ` Andrew Morton
  2 siblings, 0 replies; 5+ messages in thread
From: Andrew Morton @ 2022-01-07 23:46 UTC (permalink / raw)
  To: Helge Deller; +Cc: Linux Kernel, linux-mm, linux-parisc

On Fri, 7 Jan 2022 01:19:24 +0100 Helge Deller <deller@gmx.de> wrote:

> On some platforms the memory area between the _stext and the _etext
> symbols includes the init sections (parisc and csky). If the init
> sections are freed after bootup, the kernel may reuse this memory.
> 
> In one test the usercopy checks if the given address is inside the .text
> section (from _stext to _etext), and it wrongly fails on the mentioned
> platforms if the memory is from the former init section.
> 
> Fix this failure by first checking against the init sections before
> checking against the _stext/_etext section.
> 
> Signed-off-by: Helge Deller <deller@gmx.de>
> Fixes: 98400ad75e95 ("parisc: Fix backtrace to always include init funtion names")
> 

And 98400ad75e95 has cc:stable so we'll want cc:stable on this patch
also, yes?

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] usercopy: Do not fail on memory from former init sections
  2022-01-07  0:19 [PATCH] usercopy: Do not fail on memory from former init sections Helge Deller
  2022-01-07 23:44 ` Andrew Morton
  2022-01-07 23:46 ` Andrew Morton
@ 2022-01-07 23:51 ` Andrew Morton
  2022-01-08  0:22   ` Helge Deller
  2 siblings, 1 reply; 5+ messages in thread
From: Andrew Morton @ 2022-01-07 23:51 UTC (permalink / raw)
  To: Helge Deller; +Cc: Linux Kernel, linux-mm, linux-parisc

On Fri, 7 Jan 2022 01:19:24 +0100 Helge Deller <deller@gmx.de> wrote:

> On some platforms the memory area between the _stext and the _etext
> symbols includes the init sections (parisc and csky). If the init
> sections are freed after bootup, the kernel may reuse this memory.
> 
> In one test the usercopy checks if the given address is inside the .text
> section (from _stext to _etext), and it wrongly fails on the mentioned
> platforms if the memory is from the former init section.
> 
> Fix this failure by first checking against the init sections before
> checking against the _stext/_etext section.
> 
> Signed-off-by: Helge Deller <deller@gmx.de>
> Fixes: 98400ad75e95 ("parisc: Fix backtrace to always include init funtion names")

Wait.  98400ad75e95 is actually called

	Revert "parisc: Fix backtrace to always include init funtion names"

and it reverts 279917e27edc2.  This isn't making a lot of sense.


And neither 98400ad75e95 nor 279917e27edc2 touch csky.

And I really wouldn't want to jam a patch into mm/usercopy.c at this
point in the life of 5.16 anyway.

I'll drop this patch.  Please revisit and clarify all these things.  A lot!

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] usercopy: Do not fail on memory from former init sections
  2022-01-07 23:51 ` Andrew Morton
@ 2022-01-08  0:22   ` Helge Deller
  0 siblings, 0 replies; 5+ messages in thread
From: Helge Deller @ 2022-01-08  0:22 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Linux Kernel, linux-mm, linux-parisc

Hi Andrew,

On 1/8/22 00:51, Andrew Morton wrote:
> On Fri, 7 Jan 2022 01:19:24 +0100 Helge Deller <deller@gmx.de> wrote:
>
>> On some platforms the memory area between the _stext and the _etext
>> symbols includes the init sections (parisc and csky). If the init
>> sections are freed after bootup, the kernel may reuse this memory.
>>
>> In one test the usercopy checks if the given address is inside the .text
>> section (from _stext to _etext), and it wrongly fails on the mentioned
>> platforms if the memory is from the former init section.
>>
>> Fix this failure by first checking against the init sections before
>> checking against the _stext/_etext section.
>>
>> Signed-off-by: Helge Deller <deller@gmx.de>
>> Fixes: 98400ad75e95 ("parisc: Fix backtrace to always include init funtion names")

You asked:
> This sounds like it might have very serious runtime effects?

Yes, the patch fixes very negative effects in the sense that the kernel fails to boot
on parisc if the following patch is included (and if userchecks are enabled):
279917e27edc - parisc: Fix backtrace to always include init funtion names

That patch (279917e27edc) was once in v5.16, but because of the failure
I *reverted* it again with:
98400ad75e95 - Revert "parisc: Fix backtrace to always include init funtion names"

Both patches were originally tagged for stable.
So, all current trees are fine as they are right now as they are.

> And 98400ad75e95 has cc:stable so we'll want cc:stable on this patch
> also, yes?

We could push it to stable if we think it's worth it.
But I'm fine if we simply start fresh with kernel 5.17.
So: no.

> So is this a must-have for 5.16?

No. Current trees are fine.

> And I really wouldn't want to jam a patch into mm/usercopy.c at this
> point in the life of 5.16 anyway.

Right.
I sent the patch for review and for v5.17 (it didn't had a stable tag).

It would be nice if you could queue it up for v5.17.

Helge

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-01-08  0:24 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-01-07  0:19 [PATCH] usercopy: Do not fail on memory from former init sections Helge Deller
2022-01-07 23:44 ` Andrew Morton
2022-01-07 23:46 ` Andrew Morton
2022-01-07 23:51 ` Andrew Morton
2022-01-08  0:22   ` Helge Deller

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).