* [PATCH] mm/sparse:avoid null pointer access in memory_present()
@ 2023-06-17 4:40 Liam Ni
2023-06-17 5:44 ` Andrew Morton
0 siblings, 1 reply; 6+ messages in thread
From: Liam Ni @ 2023-06-17 4:40 UTC (permalink / raw)
To: dave.hansen, luto, peterz, tglx, mingo, bp, x86, hpa, akpm, rppt
Cc: linux-kernel, linux-mm, zhiguangni01
__nr_to_section() may return a null pointer,
before accessing the member variable section_mem_map,
we should first determine whether it is a null pointer.
Signed-off-by: Liam Ni <zhiguangni01@gmail.com>
---
mm/sparse.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/mm/sparse.c b/mm/sparse.c
index 4e6e3a9d49dc..37fa3818bc25 100644
--- a/mm/sparse.c
+++ b/mm/sparse.c
@@ -258,7 +258,7 @@ static void __init memory_present(int nid, unsigned long start, unsigned long en
set_section_nid(section, nid);
ms = __nr_to_section(section);
- if (!ms->section_mem_map) {
+ if (ms && !ms->section_mem_map) {
ms->section_mem_map = sparse_encode_early_nid(nid) |
SECTION_IS_ONLINE;
__section_mark_present(ms, section);
--
2.25.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/sparse:avoid null pointer access in memory_present()
2023-06-17 4:40 [PATCH] mm/sparse:avoid null pointer access in memory_present() Liam Ni
@ 2023-06-17 5:44 ` Andrew Morton
2023-06-17 6:17 ` Liam Ni
0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2023-06-17 5:44 UTC (permalink / raw)
To: Liam Ni
Cc: dave.hansen, luto, peterz, tglx, mingo, bp, x86, hpa, rppt,
linux-kernel, linux-mm
On Sat, 17 Jun 2023 14:40:36 +1000 Liam Ni <zhiguangni01@gmail.com> wrote:
> __nr_to_section() may return a null pointer,
> before accessing the member variable section_mem_map,
> we should first determine whether it is a null pointer.
>
> ...
>
> --- a/mm/sparse.c
> +++ b/mm/sparse.c
> @@ -258,7 +258,7 @@ static void __init memory_present(int nid, unsigned long start, unsigned long en
> set_section_nid(section, nid);
>
> ms = __nr_to_section(section);
> - if (!ms->section_mem_map) {
> + if (ms && !ms->section_mem_map) {
> ms->section_mem_map = sparse_encode_early_nid(nid) |
> SECTION_IS_ONLINE;
> __section_mark_present(ms, section);
I'm suspecting that if __nr_to_section() returns NULL here, we should
just panic. But a null-deref gives the same information, so why change
things?
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/sparse:avoid null pointer access in memory_present()
2023-06-17 5:44 ` Andrew Morton
@ 2023-06-17 6:17 ` Liam Ni
2023-06-17 7:00 ` Mike Rapoport
0 siblings, 1 reply; 6+ messages in thread
From: Liam Ni @ 2023-06-17 6:17 UTC (permalink / raw)
To: Andrew Morton
Cc: dave.hansen, luto, peterz, tglx, mingo, bp, x86, hpa, rppt,
linux-kernel, linux-mm
On Sat, 17 Jun 2023 at 13:44, Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Sat, 17 Jun 2023 14:40:36 +1000 Liam Ni <zhiguangni01@gmail.com> wrote:
>
> > __nr_to_section() may return a null pointer,
> > before accessing the member variable section_mem_map,
> > we should first determine whether it is a null pointer.
> >
> > ...
> >
> > --- a/mm/sparse.c
> > +++ b/mm/sparse.c
> > @@ -258,7 +258,7 @@ static void __init memory_present(int nid, unsigned long start, unsigned long en
> > set_section_nid(section, nid);
> >
> > ms = __nr_to_section(section);
> > - if (!ms->section_mem_map) {
> > + if (ms && !ms->section_mem_map) {
> > ms->section_mem_map = sparse_encode_early_nid(nid) |
> > SECTION_IS_ONLINE;
> > __section_mark_present(ms, section);
>
> I'm suspecting that if __nr_to_section() returns NULL here, we should
> just panic. But a null-deref gives the same information, so why change
> things?
Do you mean if ms is a null pointer，ms->section_mem_map will cause
system panic,so we needn't change?
>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/sparse:avoid null pointer access in memory_present()
2023-06-17 6:17 ` Liam Ni
@ 2023-06-17 7:00 ` Mike Rapoport
2023-06-17 8:59 ` Liam Ni
0 siblings, 1 reply; 6+ messages in thread
From: Mike Rapoport @ 2023-06-17 7:00 UTC (permalink / raw)
To: Liam Ni
Cc: Andrew Morton, dave.hansen, luto, peterz, tglx, mingo, bp, x86,
hpa, linux-kernel, linux-mm
On Sat, Jun 17, 2023 at 02:17:58PM +0800, Liam Ni wrote:
> On Sat, 17 Jun 2023 at 13:44, Andrew Morton <akpm@linux-foundation.org> wrote:
> >
> > On Sat, 17 Jun 2023 14:40:36 +1000 Liam Ni <zhiguangni01@gmail.com> wrote:
> >
> > > __nr_to_section() may return a null pointer,
> > > before accessing the member variable section_mem_map,
> > > we should first determine whether it is a null pointer.
> > >
> > > ...
> > >
> > > --- a/mm/sparse.c
> > > +++ b/mm/sparse.c
> > > @@ -258,7 +258,7 @@ static void __init memory_present(int nid, unsigned long start, unsigned long en
> > > set_section_nid(section, nid);
> > >
> > > ms = __nr_to_section(section);
> > > - if (!ms->section_mem_map) {
> > > + if (ms && !ms->section_mem_map) {
> > > ms->section_mem_map = sparse_encode_early_nid(nid) |
> > > SECTION_IS_ONLINE;
> > > __section_mark_present(ms, section);
> >
> > I'm suspecting that if __nr_to_section() returns NULL here, we should
> > just panic. But a null-deref gives the same information, so why change
> > things?
>
> Do you mean if ms is a null pointer，ms->section_mem_map will cause
> system panic,so we needn't change?
Yes, if __nr_to_section ever returns NULL the system will crash anyway.
--
Sincerely yours,
Mike.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/sparse:avoid null pointer access in memory_present()
2023-06-17 7:00 ` Mike Rapoport
@ 2023-06-17 8:59 ` Liam Ni
2023-06-18 7:16 ` Mike Rapoport
0 siblings, 1 reply; 6+ messages in thread
From: Liam Ni @ 2023-06-17 8:59 UTC (permalink / raw)
To: Mike Rapoport
Cc: Andrew Morton, dave.hansen, luto, peterz, tglx, mingo, bp, x86,
hpa, linux-kernel, linux-mm
On Sat, 17 Jun 2023 at 15:01, Mike Rapoport <rppt@kernel.org> wrote:
>
> On Sat, Jun 17, 2023 at 02:17:58PM +0800, Liam Ni wrote:
> > On Sat, 17 Jun 2023 at 13:44, Andrew Morton <akpm@linux-foundation.org> wrote:
> > >
> > > On Sat, 17 Jun 2023 14:40:36 +1000 Liam Ni <zhiguangni01@gmail.com> wrote:
> > >
> > > > __nr_to_section() may return a null pointer,
> > > > before accessing the member variable section_mem_map,
> > > > we should first determine whether it is a null pointer.
> > > >
> > > > ...
> > > >
> > > > --- a/mm/sparse.c
> > > > +++ b/mm/sparse.c
> > > > @@ -258,7 +258,7 @@ static void __init memory_present(int nid, unsigned long start, unsigned long en
> > > > set_section_nid(section, nid);
> > > >
> > > > ms = __nr_to_section(section);
> > > > - if (!ms->section_mem_map) {
> > > > + if (ms && !ms->section_mem_map) {
> > > > ms->section_mem_map = sparse_encode_early_nid(nid) |
> > > > SECTION_IS_ONLINE;
> > > > __section_mark_present(ms, section);
> > >
> > > I'm suspecting that if __nr_to_section() returns NULL here, we should
> > > just panic. But a null-deref gives the same information, so why change
> > > things?
> >
> > Do you mean if ms is a null pointer，ms->section_mem_map will cause
> > system panic,so we needn't change?
>
> Yes, if __nr_to_section ever returns NULL the system will crash anyway.
I got it,do we need to print some information by panic()?
>
> --
> Sincerely yours,
> Mike.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] mm/sparse:avoid null pointer access in memory_present()
2023-06-17 8:59 ` Liam Ni
@ 2023-06-18 7:16 ` Mike Rapoport
0 siblings, 0 replies; 6+ messages in thread
From: Mike Rapoport @ 2023-06-18 7:16 UTC (permalink / raw)
To: Liam Ni
Cc: Andrew Morton, dave.hansen, luto, peterz, tglx, mingo, bp, x86,
hpa, linux-kernel, linux-mm
On Sat, Jun 17, 2023 at 04:59:46PM +0800, Liam Ni wrote:
> On Sat, 17 Jun 2023 at 15:01, Mike Rapoport <rppt@kernel.org> wrote:
> >
> > On Sat, Jun 17, 2023 at 02:17:58PM +0800, Liam Ni wrote:
> > > On Sat, 17 Jun 2023 at 13:44, Andrew Morton <akpm@linux-foundation.org> wrote:
> > > >
> > > > On Sat, 17 Jun 2023 14:40:36 +1000 Liam Ni <zhiguangni01@gmail.com> wrote:
> > > >
> > > > > __nr_to_section() may return a null pointer,
> > > > > before accessing the member variable section_mem_map,
> > > > > we should first determine whether it is a null pointer.
> > > > >
> > > > > ...
> > > > >
> > > > > --- a/mm/sparse.c
> > > > > +++ b/mm/sparse.c
> > > > > @@ -258,7 +258,7 @@ static void __init memory_present(int nid, unsigned long start, unsigned long en
> > > > > set_section_nid(section, nid);
> > > > >
> > > > > ms = __nr_to_section(section);
> > > > > - if (!ms->section_mem_map) {
> > > > > + if (ms && !ms->section_mem_map) {
> > > > > ms->section_mem_map = sparse_encode_early_nid(nid) |
> > > > > SECTION_IS_ONLINE;
> > > > > __section_mark_present(ms, section);
> > > >
> > > > I'm suspecting that if __nr_to_section() returns NULL here, we should
> > > > just panic. But a null-deref gives the same information, so why change
> > > > things?
> > >
> > > Do you mean if ms is a null pointer，ms->section_mem_map will cause
> > > system panic,so we needn't change?
> >
> > Yes, if __nr_to_section ever returns NULL the system will crash anyway.
>
> I got it,do we need to print some information by panic()?
Accessing a NULL pointer will cause panic and there will be lots of
information spilled into the log anyway.
--
Sincerely yours,
Mike.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-06-18 7:17 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-06-17 4:40 [PATCH] mm/sparse:avoid null pointer access in memory_present() Liam Ni
2023-06-17 5:44 ` Andrew Morton
2023-06-17 6:17 ` Liam Ni
2023-06-17 7:00 ` Mike Rapoport
2023-06-17 8:59 ` Liam Ni
2023-06-18 7:16 ` Mike Rapoport
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox