kexec_file: Adjust type of kexec_purgatory

PATCH Superseded Kees Cook 3 participants 5 messages May 9, 2017 – May 10, 2017

Subsystems (3)

Live update kexec handover (KHO) kexec

lore

Share & tools

lore

Kees Cook

author

2017-05-09 23:06 UTC #0 lore raw

PATCH 1/1 kexec_file: Adjust type of kexec_purgatory

Defining kexec_purgatory as a zero-length char array upsets compile time size checking. Since this is entirely runtime sized, switch this to void *. This silences the warning generated by the future CONFIG_FORTIFY_SOURCE, which did not like the memcmp() of a "0 byte" array.

Cc Daniel Micay <[email protected]>

Signed-off-by Kees Cook <[email protected]>

---
 kernel/kexec_file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

kernel/kexec_file.c +1 -1

index b118735fea9d..bc86f85f1329 100644
@@ -30,7 +30,7 @@
  * Declare these symbols weak so that if architecture provides a purgatory,
  * these will be overridden.
  */
-char __weak kexec_purgatory[0];
+void * __weak kexec_purgatory;
 size_t __weak kexec_purgatory_size = 0;
 
 static int kexec_calculate_store_digests(struct kimage *image);

Signature · 6 lines

2.7.4


-- 
Kees Cook
Pixel Security

Daniel Micay

2017-05-09 23:13 UTC #1 lore raw

Quoted 25 lines · Kees Cook

Defining kexec_purgatory as a zero-length char array upsets compile time size checking. Since this is entirely runtime sized, switch this to void *. This silences the warning generated by the future CONFIG_FORTIFY_SOURCE, which did not like the memcmp() of a "0 byte" array.
Cc Daniel Micay <[email protected]>
Signed-off-by Kees Cook <[email protected]>
---
 kernel/kexec_file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/kexec_file.c +1 -1
index b118735fea9d..bc86f85f1329 100644
@@ -30,7 +30,7 @@
  * Declare these symbols weak so that if architecture provides a
 purgatory,
  * these will be overridden.
  */
-char __weak kexec_purgatory[0];
+void * __weak kexec_purgatory;
 size_t __weak kexec_purgatory_size = 0;
 

Quoted 3 lines

static int kexec_calculate_store_digests(struct kimage *image);
Signature · 1 line
2.7.4

It seems more correct to use char char __weak kexec_purgatory[], otherwise isn't __builtin_object_size ending up as 8, which is still wrong?

Kees Cook

author

2017-05-09 23:22 UTC #2 lore raw

Quoted 26 lines · Daniel Micay

Quoted 25 lines · Kees Cook
Defining kexec_purgatory as a zero-length char array upsets compile time size checking. Since this is entirely runtime sized, switch this to void *. This silences the warning generated by the future CONFIG_FORTIFY_SOURCE, which did not like the memcmp() of a "0 byte" array.
Cc Daniel Micay <[email protected]>
Signed-off-by Kees Cook <[email protected]>
---
 kernel/kexec_file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/kexec_file.c +1 -1
index b118735fea9d..bc86f85f1329 100644
@@ -30,7 +30,7 @@
  * Declare these symbols weak so that if architecture provides a
 purgatory,
  * these will be overridden.
  */
-char __weak kexec_purgatory[0];
+void * __weak kexec_purgatory;
 size_t __weak kexec_purgatory_size = 0;
 

Quoted 7 lines

Quoted 3 lines
static int kexec_calculate_store_digests(struct kimage *image);
--
2.7.4
It seems more correct to use char char __weak kexec_purgatory[], otherwise isn't __builtin_object_size ending up as 8, which is still wrong?

I tried [], that was my instinct, too, but since this is a __weak and not an extern, that doesn't work:

kernel/kexec_file.c:33:13: warning: array ‘kexec_purgatory’ assumed to have one element

 char __weak kexec_purgatory[];
             ^~~~~~~~~~~~~~~

-Kees

Signature · 2 lines

Kees Cook
Pixel Security

Eric W. Biederman

2017-05-10 00:22 UTC #3 lore raw

Quoted 35 lines · Kees Cook

Quoted 26 lines · Daniel Micay
Quoted 25 lines · Kees Cook
Defining kexec_purgatory as a zero-length char array upsets compile time size checking. Since this is entirely runtime sized, switch this to void *. This silences the warning generated by the future CONFIG_FORTIFY_SOURCE, which did not like the memcmp() of a "0 byte" array.
Cc Daniel Micay <[email protected]>
Signed-off-by Kees Cook <[email protected]>
---
 kernel/kexec_file.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/kexec_file.c +1 -1
index b118735fea9d..bc86f85f1329 100644
@@ -30,7 +30,7 @@
  * Declare these symbols weak so that if architecture provides a
 purgatory,
  * these will be overridden.
  */
-char __weak kexec_purgatory[0];
+void * __weak kexec_purgatory;
 size_t __weak kexec_purgatory_size = 0;
 
Quoted 7 lines
Quoted 3 lines
static int kexec_calculate_store_digests(struct kimage *image);
--
2.7.4
It seems more correct to use char char __weak kexec_purgatory[], otherwise isn't __builtin_object_size ending up as 8, which is still wrong?

Quoted 7 lines

I tried [], that was my instinct, too, but since this is a __weak and not an extern, that doesn't work:
kernel/kexec_file.c:33:13: warning: array ‘kexec_purgatory’ assumed to have one element
 char __weak kexec_purgatory[];
             ^~~~~~~~~~~~~~~

Nor does "void *kexec_purgatory" as that says at the address known as kexec_purgatory is a void pointer not a blob a bytes that can be used for something interesting.

Better to get rid of the __weak and deal with that fallout.

Eric

Kees Cook

author

2017-05-10 19:54 UTC #4 lore raw

Quoted 11 lines · Eric W. Biederman

Quoted 4 lines · Kees Cook
kernel/kexec_file.c:33:13: warning: array ‘kexec_purgatory’ assumed to have one element
 char __weak kexec_purgatory[];
             ^~~~~~~~~~~~~~~
Nor does "void *kexec_purgatory" as that says at the address known as kexec_purgatory is a void pointer not a blob a bytes that can be used for something interesting.
Better to get rid of the __weak and deal with that fallout.

Agreed. This is actually pretty easy, since only x86 and PPC use kexec_file, and both define purgatories. I'll send an updated patch.

Thanks!

-Kees

Signature · 2 lines

Kees Cook
Pixel Security