diff mbox

x86/KASLR: use the right memcpy

Message ID 20170530091446.1000183-1-arnd@arndb.de
State New
Headers show

Commit Message

Arnd Bergmann May 30, 2017, 9:14 a.m. UTC
The decompressor has its own implementation of the string functions,
but has to include the right header to get those, while implicitly
including linux/string.h may result in a link error:

arch/x86/boot/compressed/kaslr.o: In function `choose_random_location':
kaslr.c:(.text+0xf51): undefined reference to `_mmx_memcpy'

This has appeared now as kaslr started using memcpy. Other files in the
decompressor already do the same thing.

Fixes: d52e7d5a952c ("x86/KASLR: Parse all 'memmap=' boot option entries")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 arch/x86/boot/compressed/kaslr.c | 1 +
 1 file changed, 1 insertion(+)

-- 
2.9.0

Comments

Baoquan He May 30, 2017, 1:56 p.m. UTC | #1
On 05/30/17 at 11:14am, Arnd Bergmann wrote:
> The decompressor has its own implementation of the string functions,

> but has to include the right header to get those, while implicitly

> including linux/string.h may result in a link error:

> 

> arch/x86/boot/compressed/kaslr.o: In function `choose_random_location':

> kaslr.c:(.text+0xf51): undefined reference to `_mmx_memcpy'

> 

> This has appeared now as kaslr started using memcpy. Other files in the

> decompressor already do the same thing.

> 

> Fixes: d52e7d5a952c ("x86/KASLR: Parse all 'memmap=' boot option entries")

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


Thanks for this fix, Arnd!

The linking error didn't happen when I tested the patch of d52e7d5a952c.
Could you tell in what condition it will be triggered? Not sure if I
should wait for this fix being merged and do a back porting, or can
defer it if it's not risky.

Thanks
Baoquan

> ---

>  arch/x86/boot/compressed/kaslr.c | 1 +

>  1 file changed, 1 insertion(+)

> 

> diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c

> index e0eba12bffe7..fe318b44f7b8 100644

> --- a/arch/x86/boot/compressed/kaslr.c

> +++ b/arch/x86/boot/compressed/kaslr.c

> @@ -30,6 +30,7 @@

>  

>  #include "misc.h"

>  #include "error.h"

> +#include "../string.h"

>  

>  #include <generated/compile.h>

>  #include <linux/module.h>

> -- 

> 2.9.0

>
Arnd Bergmann May 30, 2017, 2:24 p.m. UTC | #2
On Tue, May 30, 2017 at 3:56 PM, Baoquan He <bhe@redhat.com> wrote:
> On 05/30/17 at 11:14am, Arnd Bergmann wrote:

>> The decompressor has its own implementation of the string functions,

>> but has to include the right header to get those, while implicitly

>> including linux/string.h may result in a link error:

>>

>> arch/x86/boot/compressed/kaslr.o: In function `choose_random_location':

>> kaslr.c:(.text+0xf51): undefined reference to `_mmx_memcpy'

>>

>> This has appeared now as kaslr started using memcpy. Other files in the

>> decompressor already do the same thing.

>>

>> Fixes: d52e7d5a952c ("x86/KASLR: Parse all 'memmap=' boot option entries")

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

>

> Thanks for this fix, Arnd!

>

> The linking error didn't happen when I tested the patch of d52e7d5a952c.

> Could you tell in what condition it will be triggered? Not sure if I

> should wait for this fix being merged and do a back porting, or can

> defer it if it's not risky.


It only happens on 32-bit kernels with CONFIG_X86_USE_3DNOW, maybe
there are additional requirements.

       Arnd
Baoquan He May 31, 2017, 1:43 a.m. UTC | #3
On 05/30/17 at 04:24pm, Arnd Bergmann wrote:
> On Tue, May 30, 2017 at 3:56 PM, Baoquan He <bhe@redhat.com> wrote:

> > On 05/30/17 at 11:14am, Arnd Bergmann wrote:

> >> The decompressor has its own implementation of the string functions,

> >> but has to include the right header to get those, while implicitly

> >> including linux/string.h may result in a link error:

> >>

> >> arch/x86/boot/compressed/kaslr.o: In function `choose_random_location':

> >> kaslr.c:(.text+0xf51): undefined reference to `_mmx_memcpy'

> >>

> >> This has appeared now as kaslr started using memcpy. Other files in the

> >> decompressor already do the same thing.

> >>

> >> Fixes: d52e7d5a952c ("x86/KASLR: Parse all 'memmap=' boot option entries")

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

> >

> > Thanks for this fix, Arnd!

> >

> > The linking error didn't happen when I tested the patch of d52e7d5a952c.

> > Could you tell in what condition it will be triggered? Not sure if I

> > should wait for this fix being merged and do a back porting, or can

> > defer it if it's not risky.

> 

> It only happens on 32-bit kernels with CONFIG_X86_USE_3DNOW, maybe

> there are additional requirements.


Checked code again, in commit d52e7d5a952c "#include "../boot.h" is
removed. Not sure if that removal caused the 32-bit kernel link error.
While I didn't see boot/string.h is included into the boot/boot.h.
diff mbox

Patch

diff --git a/arch/x86/boot/compressed/kaslr.c b/arch/x86/boot/compressed/kaslr.c
index e0eba12bffe7..fe318b44f7b8 100644
--- a/arch/x86/boot/compressed/kaslr.c
+++ b/arch/x86/boot/compressed/kaslr.c
@@ -30,6 +30,7 @@ 
 
 #include "misc.h"
 #include "error.h"
+#include "../string.h"
 
 #include <generated/compile.h>
 #include <linux/module.h>