[PATCH net-next v2] sections: global data can be in .bss

Subsystems: generic include/asm header files, the rest

STALE1721d REVIEWED: 17 (17M)

1 review trailer.

5 messages, 4 authors, 2021-11-22 · open the first message on its own page

[PATCH net-next v2] sections: global data can be in .bss

From: Antoine Tenart <atenart@kernel.org>
Date: 2021-11-22 14:25:32

When checking an address is located in a global data section also check
for the .bss section as global variables initialized to 0 can be in
there (-fzero-initialized-in-bss).

This was found when looking at ensure_safe_net_sysctl which was failing
to detect non-init sysctl pointing to a global data section when the
data was in the .bss section.

Signed-off-by: Antoine Tenart <atenart@kernel.org>
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---

A few remarks:

- This still targets net-next but I added Arnd if he prefers to take it
  through the 'asm-generic' tree, now that is_kernel_core_data is in
  include/asm-generic/.

- I kept the Acked-by tag as the change is the same really, the
  difference is the core_kernel_data function was renamed to
  is_kernel_core_data and moved since then.

- @Jonathon: with your analysis and suggestion I think you should be
  listed as a co-developer. If that's fine please say so, and reply
  with both a Co-developed-by and a Signed-off-by tags.

Since v1:
  - Grouped the .data and .bss checks in the same function.

v1 was https://lore.kernel.org/all/20211020083854.1101670-1-atenart@kernel.org/T/

Thanks!
Antoine

 include/asm-generic/sections.h | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index 1dfadb2e878d..76a0f16e56cf 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -130,18 +130,24 @@ static inline bool init_section_intersects(void *virt, size_t size)
 
 /**
  * is_kernel_core_data - checks if the pointer address is located in the
- *			 .data section
+ *			 .data or .bss section
  *
  * @addr: address to check
  *
- * Returns: true if the address is located in .data, false otherwise.
+ * Returns: true if the address is located in .data or .bss, false otherwise.
  * Note: On some archs it may return true for core RODATA, and false
  *       for others. But will always be true for core RW data.
  */
 static inline bool is_kernel_core_data(unsigned long addr)
 {
-	return addr >= (unsigned long)_sdata &&
-	       addr < (unsigned long)_edata;
+	if (addr >= (unsigned long)_sdata && addr < (unsigned long)_edata)
+		return true;
+
+	if (addr >= (unsigned long)__bss_start &&
+	    addr < (unsigned long)__bss_stop)
+		return true;
+
+	return false;
 }
 
 /**
-- 
2.33.1

Re: [PATCH net-next v2] sections: global data can be in .bss

From: Arnd Bergmann <arnd@arndb.de>
Date: 2021-11-22 14:39:41

On Mon, Nov 22, 2021 at 3:24 PM Antoine Tenart [off-list ref] wrote:
When checking an address is located in a global data section also check
for the .bss section as global variables initialized to 0 can be in
there (-fzero-initialized-in-bss).

This was found when looking at ensure_safe_net_sysctl which was failing
to detect non-init sysctl pointing to a global data section when the
data was in the .bss section.

Signed-off-by: Antoine Tenart <atenart@kernel.org>
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
---

A few remarks:

- This still targets net-next but I added Arnd if he prefers to take it
  through the 'asm-generic' tree, now that is_kernel_core_data is in
  include/asm-generic/.
I have nothing else for asm-generic at the moment, please take
this through net-next.

Acked-by: Arnd Bergmann <arnd@arndb.de>

Re: [PATCH net-next v2] sections: global data can be in .bss

From: patchwork-bot+netdevbpf@kernel.org
Date: 2021-11-22 15:00:25

Hello:

This patch was applied to netdev/net-next.git (master)
by David S. Miller [off-list ref]:

On Mon, 22 Nov 2021 15:24:56 +0100 you wrote:
When checking an address is located in a global data section also check
for the .bss section as global variables initialized to 0 can be in
there (-fzero-initialized-in-bss).

This was found when looking at ensure_safe_net_sysctl which was failing
to detect non-init sysctl pointing to a global data section when the
data was in the .bss section.

[...]
Here is the summary with links:
  - [net-next,v2] sections: global data can be in .bss
    https://git.kernel.org/netdev/net-next/c/cb902b332f95

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net-next v2] sections: global data can be in .bss

From: Jonathon Reinhart <hidden>
Date: 2021-11-22 16:57:22

On Mon, Nov 22, 2021 at 9:24 AM Antoine Tenart [off-list ref] wrote:
When checking an address is located in a global data section also check
for the .bss section as global variables initialized to 0 can be in
there (-fzero-initialized-in-bss).

This was found when looking at ensure_safe_net_sysctl which was failing
to detect non-init sysctl pointing to a global data section when the
data was in the .bss section.

Signed-off-by: Antoine Tenart <atenart@kernel.org>
Acked-by: Steven Rostedt (VMware) <rostedt@goodmis.org>
Co-Developed-by: Jonathon Reinhart <redacted>
Signed-off-by: Jonathon Reinhart <redacted>
---

A few remarks:

- This still targets net-next but I added Arnd if he prefers to take it
  through the 'asm-generic' tree, now that is_kernel_core_data is in
  include/asm-generic/.

- I kept the Acked-by tag as the change is the same really, the
  difference is the core_kernel_data function was renamed to
  is_kernel_core_data and moved since then.

- @Jonathon: with your analysis and suggestion I think you should be
  listed as a co-developer. If that's fine please say so, and reply
  with both a Co-developed-by and a Signed-off-by tags.
Added, thanks. Although it appears I may have missed the boat.

Re: [PATCH net-next v2] sections: global data can be in .bss

From: Antoine Tenart <atenart@kernel.org>
Date: 2021-11-22 16:59:53

Quoting Jonathon Reinhart (2021-11-22 17:56:55)
On Mon, Nov 22, 2021 at 9:24 AM Antoine Tenart [off-list ref] wrote:
quoted
- @Jonathon: with your analysis and suggestion I think you should be
  listed as a co-developer. If that's fine please say so, and reply
  with both a Co-developed-by and a Signed-off-by tags.
Added, thanks. Although it appears I may have missed the boat.
Yes, the patch was applied quickly. Anyway, thanks for the investigation!

Antoine
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help