(alpha) process_reloc_for_got confuses r_offset and r_addend

(alpha) process_reloc_for_got confuses r_offset and r_addend

[Chaskiel Grundman]

arch/alpha/kernel/module.c:process_reloc_for_got(), which figures out how 
big the .got section for a module should be, appears to be confusing 
r_offset (the file offset that the relocation needs to be applied to) with 
r_addend (the offset of the relocation's actual target address from the 
address of the relocation's symbol). Because of this, one .got entry is 
allocated for each relocation instead of one each unique symbol/addend.

In the module I am working with, this causes the .got section to be almost 
10 times larger than it needs to be (75544 bytes instead of 7608 bytes). 
As the .got is accessed with global-pointer-relative instructions, it 
needs to be within the 64k gp "zone", and a 75544 byte .got clearly does 
not fit. The result of this is that relocation overflows are detected 
during module load and the load is aborted.

Does anyone see anything wrong with this analysis? I tested a patch that 
makes the obvious change to struct got_entry/process_reloc_for_got and it 
seems to work ok.

(Please cc me on replies. thanks)

Re: (alpha) process_reloc_for_got confuses r_offset and r_addend

[Jesper Juhl]

On 9/5/05, Chaskiel Grundman <cg2v@andrew.cmu.edu> wrote:
> arch/alpha/kernel/module.c:process_reloc_for_got(), which figures out how
> big the .got section for a module should be, appears to be confusing
> r_offset (the file offset that the relocation needs to be applied to) with
> r_addend (the offset of the relocation's actual target address from the
> address of the relocation's symbol). Because of this, one .got entry is
> allocated for each relocation instead of one each unique symbol/addend.
> 
> In the module I am working with, this causes the .got section to be almost
> 10 times larger than it needs to be (75544 bytes instead of 7608 bytes).
> As the .got is accessed with global-pointer-relative instructions, it
> needs to be within the 64k gp "zone", and a 75544 byte .got clearly does
> not fit. The result of this is that relocation overflows are detected
> during module load and the load is aborted.
> 
> Does anyone see anything wrong with this analysis? I tested a patch that
> makes the obvious change to struct got_entry/process_reloc_for_got and it
> seems to work ok.
> 
> (Please cc me on replies. thanks)

Why not post the patch you made for review as well?

-- 
Jesper Juhl <jesper.juhl@gmail.com>
Don't top-post  http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please      http://www.expita.com/nomime.html

Re: (alpha) process_reloc_for_got confuses r_offset and r_addend

[Chaskiel Grundman]

On Mon, 5 Sep 2005, Jesper Juhl wrote:
> Why not post the patch you made for review as well?

In part because if the analysis is wrong, then the patch surely is.

but mostly because I didn't want to post my message with the bland subject 
that the faq recommends for patches.

--- linux-2.6.12.5/arch/alpha/kernel/module.c      2005-08-14 20:20:18.000000000 -0400
+++ linux/arch/alpha/kernel/module.c   2005-09-05 12:38:43.000000000 -0400
@@ -47,7 +47,7 @@

  struct got_entry {
         struct got_entry *next;
-       Elf64_Addr r_offset;
+       Elf64_Sxword r_addend;
         int got_offset;
  };

@@ -57,14 +57,14 @@
  {
         unsigned long r_sym = ELF64_R_SYM (rela->r_info);
         unsigned long r_type = ELF64_R_TYPE (rela->r_info);
-       Elf64_Addr r_offset = rela->r_offset;
+       Elf64_Sxword r_addend = rela->r_addend;
         struct got_entry *g;

         if (r_type != R_ALPHA_LITERAL)
                 return;

         for (g = chains + r_sym; g ; g = g->next)
-               if (g->r_offset == r_offset) {
+               if (g->r_addend == r_addend) {
                         if (g->got_offset == 0) {
                                 g->got_offset = *poffset;
                                 *poffset += 8;
@@ -74,7 +74,7 @@

         g = kmalloc (sizeof (*g), GFP_KERNEL);
         g->next = chains[r_sym].next;
-       g->r_offset = r_offset;
+       g->r_addend = r_addend;
         g->got_offset = *poffset;
         *poffset += 8;
         chains[r_sym].next = g;

Re: (alpha) process_reloc_for_got confuses r_offset and r_addend

[Richard Henderson]

This patch is correct.


r~

On Mon, Sep 05, 2005 at 02:42:36PM -0400, Chaskiel Grundman wrote:
> On Mon, 5 Sep 2005, Jesper Juhl wrote:
> > Why not post the patch you made for review as well?
> 
> In part because if the analysis is wrong, then the patch surely is.
> 
> but mostly because I didn't want to post my message with the bland subject 
> that the faq recommends for patches.
> 
> --- linux-2.6.12.5/arch/alpha/kernel/module.c      2005-08-14 20:20:18.000000000 -0400
> +++ linux/arch/alpha/kernel/module.c   2005-09-05 12:38:43.000000000 -0400
> @@ -47,7 +47,7 @@
> 
>   struct got_entry {
>          struct got_entry *next;
> -       Elf64_Addr r_offset;
> +       Elf64_Sxword r_addend;
>          int got_offset;
>   };
> 
> @@ -57,14 +57,14 @@
>   {
>          unsigned long r_sym = ELF64_R_SYM (rela->r_info);
>          unsigned long r_type = ELF64_R_TYPE (rela->r_info);
> -       Elf64_Addr r_offset = rela->r_offset;
> +       Elf64_Sxword r_addend = rela->r_addend;
>          struct got_entry *g;
> 
>          if (r_type != R_ALPHA_LITERAL)
>                  return;
> 
>          for (g = chains + r_sym; g ; g = g->next)
> -               if (g->r_offset == r_offset) {
> +               if (g->r_addend == r_addend) {
>                          if (g->got_offset == 0) {
>                                  g->got_offset = *poffset;
>                                  *poffset += 8;
> @@ -74,7 +74,7 @@
> 
>          g = kmalloc (sizeof (*g), GFP_KERNEL);
>          g->next = chains[r_sym].next;
> -       g->r_offset = r_offset;
> +       g->r_addend = r_addend;
>          g->got_offset = *poffset;
>          *poffset += 8;
>          chains[r_sym].next = g;
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/