Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1619378 > unrolled thread

[PATCH v3] ia64: fix module loading for gcc-5.4

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH v3] ia64: fix module loading for gcc-5.4 Sergei Trofimovich <slyfox@gentoo.org> - 2017-04-08 22:00 +0200
    Re: [PATCH v3] ia64: fix module loading for gcc-5.4 SF Markus Elfring <elfring@users.sourceforge.net> - 2017-04-09 10:30 +0200
      Re: [PATCH v3] ia64: fix module loading for gcc-5.4 Sergei Trofimovich <slyfox@gentoo.org> - 2017-04-09 11:00 +0200
        Re: ia64: fix module loading for gcc-5.4 SF Markus Elfring <elfring@users.sourceforge.net> - 2017-04-09 11:10 +0200
          Re: ia64: fix module loading for gcc-5.4 Sergei Trofimovich <slyfox@gentoo.org> - 2017-04-09 12:40 +0200

#1619378 — [PATCH v3] ia64: fix module loading for gcc-5.4

Starting from gcc-5.4+ gcc generates MLX
instructions in more cases to refer local
symbols:
    https://gcc.gnu.org/PR60465

That caused ia64 module loader to choke
on such instructions:
    fuse: invalid slot number 1 for IMM64

Linux kernel used to handle only case where
relocation pointed to slot=2 instruction in
the bundle. That limitation was fixed in linux by
commit 9c184a073bfd ("[IA64] Fix 2.6 kernel for the new ia64 assembler")
See http://sources.redhat.com/bugzilla/show_bug.cgi?id=1433

This change lifts the slot=2 restriction from
linux kernel module loader.

Tested on 'fuse' and 'btrfs' kernel modules.

Cc: Markus Elfring <elfring@users.sourceforge.net>
Cc: H. J. Lu <hjl.tools@gmail.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Fenghua Yu <fenghua.yu@intel.com>
Cc: linux-ia64@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: Andrew Morton <akpm@linux-foundation.org>
Bug: https://bugs.gentoo.org/601014
Tested-by: Émeric MASCHINO <emeric.maschino@gmail.com>
Signed-off-by: Sergei Trofimovich <slyfox@gentoo.org>
---
Change since v1: added 'Tested-by'
Change since v2: checkpatched, fixed typos by found by Markus Elfring

 arch/ia64/kernel/module.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/arch/ia64/kernel/module.c b/arch/ia64/kernel/module.c
index 6ab0ae7d6535..d1d945c6bd05 100644
--- a/arch/ia64/kernel/module.c
+++ b/arch/ia64/kernel/module.c
@@ -153,7 +153,7 @@ slot (const struct insn *insn)
 static int
 apply_imm64 (struct module *mod, struct insn *insn, uint64_t val)
 {
-	if (slot(insn) != 2) {
+	if (slot(insn) != 1 && slot(insn) != 2) {
 		printk(KERN_ERR "%s: invalid slot number %d for IMM64\n",
 		       mod->name, slot(insn));
 		return 0;
@@ -165,7 +165,7 @@ apply_imm64 (struct module *mod, struct insn *insn, uint64_t val)
 static int
 apply_imm60 (struct module *mod, struct insn *insn, uint64_t val)
 {
-	if (slot(insn) != 2) {
+	if (slot(insn) != 1 && slot(insn) != 2) {
 		printk(KERN_ERR "%s: invalid slot number %d for IMM60\n",
 		       mod->name, slot(insn));
 		return 0;
-- 
2.12.0

[toc] | [next] | [standalone]

#1619452

> Starting from gcc-5.4+ gcc generates MLX

How do you think about to omit the plus character?


> instructions in more cases to refer local
> symbols:

I wonder about your choice of a line length limit here.


> That caused ia64 module loader to choke
> on such instructions:
>     fuse: invalid slot number 1 for IMM64

Why does it matter to check such a value?


> … That limitation was fixed in linux by

Would it be nicer to write “in corresponding source code by the”?


> Change since v2: checkpatched, fixed typos by found by Markus Elfring

Does this version information contain an unwanted word repetition?
How does it fit to the identifier “v3” in the commit subject?

Regards,
Markus

[toc] | [prev] | [next] | [standalone]

#1619456

[Multipart message — attachments visible in raw view] — view raw

On Sun, 9 Apr 2017 10:27:52 +0200
SF Markus Elfring <elfring@users.sourceforge.net> wrote:

> > That caused ia64 module loader to choke
> > on such instructions:
> >     fuse: invalid slot number 1 for IMM64  
> 
> Why does it matter to check such a value?

I'm not sure I follow the question. Is your question about
linux kernel relocation code handler, gcc or ia64 instruction
format?

-- 

  Sergei

[toc] | [prev] | [next] | [standalone]

#1619459 — Re: ia64: fix module loading for gcc-5.4

>>> That caused ia64 module loader to choke
>>> on such instructions:
>>>     fuse: invalid slot number 1 for IMM64  
>>
>> Why does it matter to check such a value?
> 
> I'm not sure I follow the question. Is your question about
> linux kernel relocation code handler, gcc or ia64 instruction format?

I am just curious if this source code could also work without
the mentioned check.
Would it make sense to check more than two values there?

Regards,
Markus

[toc] | [prev] | [next] | [standalone]

#1619471 — Re: ia64: fix module loading for gcc-5.4

[Multipart message — attachments visible in raw view] — view raw

On Sun, 9 Apr 2017 11:02:43 +0200
SF Markus Elfring <elfring@users.sourceforge.net> wrote:

> >>> That caused ia64 module loader to choke
> >>> on such instructions:
> >>>     fuse: invalid slot number 1 for IMM64    
> >>
> >> Why does it matter to check such a value?  
> > 
> > I'm not sure I follow the question. Is your question about
> > linux kernel relocation code handler, gcc or ia64 instruction format?  
> 
> I am just curious if this source code could also work without
> the mentioned check.

It should work for valid code, yes. The flip side of check removal
is to miss malformed relocation (say, when instruction "address" is
wrong due to obscure toolchain bug). In this case apply_imm64()
would silently corrupt unrelated memory instead of crashing kernel.

> Would it make sense to check more than two values there?

AFAIU ia64 does not allow encoding imm64/imm60 instructions
spanning slot=0 at all.

ia64_patch_imm64() can handle only imm64 bundles that span
only both slot 1 and slot 2 at the same time. It can accept
either slot=1 "address" or slot=2 "address". Anything else would
be malformed.

-- 

  Sergei

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web