Starting from gcc-5.4+ gcc geperates MLX
instructions in more cases to refer local
symbols:
https://gcc.gnu.org/bugzilla/60465
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 9c184a073bfd650cc791956d6ca79725bb682716 commit.
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: H. J. Lu <[email protected]>
Cc: Tony Luck <[email protected]>
Cc: Fenghua Yu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Bug: https://bugs.gentoo.org/601014
Signed-off-by: Sergei Trofimovich <[email protected]>
---
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
Hi,
Backported and successfully tested on currently stable Gentoo kernel 4.9.6-r1.
>From [1], gcc 4.9.4 was first problematic release.
[1] http://marc.info/?l=linux-ia64&m=147993347915422
Émeric
2017-03-19 17:20 GMT+01:00 Sergei Trofimovich <[email protected]>:
> Starting from gcc-5.4+ gcc geperates MLX
> instructions in more cases to refer local
> symbols:
> https://gcc.gnu.org/bugzilla/60465
>
> 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 9c184a073bfd650cc791956d6ca79725bb682716 commit.
> 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: H. J. Lu <[email protected]>
> Cc: Tony Luck <[email protected]>
> Cc: Fenghua Yu <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Bug: https://bugs.gentoo.org/601014
> Signed-off-by: Sergei Trofimovich <[email protected]>
> ---
> 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
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
Starting from gcc-5.4+ gcc geperates MLX
instructions in more cases to refer local
symbols:
https://gcc.gnu.org/bugzilla/60465
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 9c184a073bfd650cc791956d6ca79725bb682716 commit.
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: H. J. Lu <[email protected]>
Cc: Tony Luck <[email protected]>
Cc: Fenghua Yu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Andrew Morton <[email protected]>
Bug: https://bugs.gentoo.org/601014
Tested-by: Émeric MASCHINO <[email protected]>
Signed-off-by: Sergei Trofimovich <[email protected]>
---
Change since v1: added 'Tested-by'
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
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 <[email protected]>
Cc: H. J. Lu <[email protected]>
Cc: Tony Luck <[email protected]>
Cc: Fenghua Yu <[email protected]>
Cc: [email protected]
Cc: [email protected]
Cc: Andrew Morton <[email protected]>
Bug: https://bugs.gentoo.org/601014
Tested-by: Émeric MASCHINO <[email protected]>
Signed-off-by: Sergei Trofimovich <[email protected]>
---
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
> 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
On Sun, 9 Apr 2017 10:27:52 +0200
SF Markus Elfring <[email protected]> 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
>>> 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
On Sun, 9 Apr 2017 11:02:43 +0200
SF Markus Elfring <[email protected]> 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
> +++ 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)
> {
I have got another idea (after your clarification) for the suggested change.
> - if (slot(insn) != 2) {
> + if (slot(insn) != 1 && slot(insn) != 2) {
+ int const s = slot(insn);
+ if (s < 1 || s > 2) {
Do run time characteristics matter for such a condition check here?
> printk(KERN_ERR "%s: invalid slot number %d for IMM64\n",
- mod->name, slot(insn));
+ mod->name, s);
> return 0;
How do you think about my update suggestion for this function implementation?
Regards,
Markus
On Mon, 10 Apr 2017 19:23:28 +0200
SF Markus Elfring <[email protected]> wrote:
> > - if (slot(insn) != 2) {
> > + if (slot(insn) != 1 && slot(insn) != 2) {
>
> + int const s = slot(insn);
> + if (s < 1 || s > 2) {
>
> Do run time characteristics matter for such a condition check here?
It's done once at kernel module load time. My guess would be
"not critical at all".
slot() is a pure arithmetic static inline function. You can compare
assembly output before and after your change.
You can measure the difference yourself using 'ski' emulator.
That's for example how I debugged and tested the patch:
http://trofi.github.io/posts/199-ia64-machine-emulation.html
--
Sergei
On Sat, 8 Apr 2017 20:53:18 +0100
Sergei Trofimovich <[email protected]> wrote:
> 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 <[email protected]>
> Cc: H. J. Lu <[email protected]>
> Cc: Tony Luck <[email protected]>
> Cc: Fenghua Yu <[email protected]>
> Cc: [email protected]
> Cc: [email protected]
> Cc: Andrew Morton <[email protected]>
> Bug: https://bugs.gentoo.org/601014
> Tested-by: Émeric MASCHINO <[email protected]>
> Signed-off-by: Sergei Trofimovich <[email protected]>
> ---
> Change since v1: added 'Tested-by'
> Change since v2: checkpatched, fixed typos by found by Markus Elfring
Ping :)
--
Sergei