Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <20190911122646.13838-1-will@kernel.org>
In-Reply-To: <20190911122646.13838-1-will@kernel.org>
From:   Ard Biesheuvel <ard.biesheuvel@linaro.org>
Date:   Wed, 11 Sep 2019 17:51:38 +0100
Message-ID: <CAKv+Gu8G_HHE+xerEQpGoy0wJ1iqhuFqRpL34At9ue2BpT+XeA@mail.gmail.com>
Subject: Re: [PATCH] module: Fix link failure due to invalid relocation on
 namespace offset
To:     Will Deacon <will@kernel.org>
Cc:     Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
        Martijn Coenen <maco@android.com>,
        Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        Matthias Maennich <maennich@google.com>,
        Jessica Yu <jeyu@kernel.org>,
        Catalin Marinas <catalin.marinas@arm.com>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

On Wed, 11 Sep 2019 at 13:26, Will Deacon <will@kernel.org> wrote:
>
> Commit 8651ec01daed ("module: add support for symbol namespaces.")
> broke linking for arm64 defconfig:
>
>   | lib/crypto/arc4.o: In function `__ksymtab_arc4_setkey':
>   | arc4.c:(___ksymtab+arc4_setkey+0x8): undefined reference to `no symbol'
>   | lib/crypto/arc4.o: In function `__ksymtab_arc4_crypt':
>   | arc4.c:(___ksymtab+arc4_crypt+0x8): undefined reference to `no symbol'
>
> This is because the dummy initialisation of the 'namespace_offset' field
> in 'struct kernel_symbol' when using EXPORT_SYMBOL on architectures with
> support for PREL32 locations uses an offset from an absolute address (0)
> in an effort to trick 'offset_to_pointer' into behaving as a NOP,
> allowing non-namespaced symbols to be treated in the same way as those
> belonging to a namespace.
>
> Unfortunately, place-relative relocations require a symbol reference
> rather than an absolute value and, although x86 appears to get away with
> this due to placing the kernel text at the top of the address space, it
> almost certainly results in a runtime failure if the kernel is relocated
> dynamically as a result of KASLR.
>
> Rework 'namespace_offset' so that a value of 0, which cannot occur for a
> valid namespaced symbol, indicates that the corresponding symbol does
> not belong to a namespace.
>
> Cc: Matthias Maennich <maennich@google.com>
> Cc: Jessica Yu <jeyu@kernel.org>
> Cc: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> Cc: Catalin Marinas <catalin.marinas@arm.com>
> Fixes: 8651ec01daed ("module: add support for symbol namespaces.")
> Reported-by: kbuild test robot <lkp@intel.com>
> Signed-off-by: Will Deacon <will@kernel.org>
> ---
>
> Please note that I've not been able to test this at LPC, but it's been
> submitted to kernelci.
>

The build tests all passed, and the boot tests that have completed by
now all look fine, so

Acked-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
Tested-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>  include/asm-generic/export.h | 2 +-
>  include/linux/export.h       | 2 +-
>  kernel/module.c              | 2 ++
>  3 files changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/include/asm-generic/export.h b/include/asm-generic/export.h
> index e2b5d0f569d3..d0912c7ac2fc 100644
> --- a/include/asm-generic/export.h
> +++ b/include/asm-generic/export.h
> @@ -17,7 +17,7 @@
>
>  .macro __put, val, name
>  #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
> -       .long   \val - ., \name - ., 0 - .
> +       .long   \val - ., \name - ., 0
>  #elif defined(CONFIG_64BIT)
>         .quad   \val, \name, 0
>  #else
> diff --git a/include/linux/export.h b/include/linux/export.h
> index 2c5468d8ea9a..ef5d015d754a 100644
> --- a/include/linux/export.h
> +++ b/include/linux/export.h
> @@ -68,7 +68,7 @@ extern struct module __this_module;
>             "__ksymtab_" #sym ":                                \n"     \
>             "   .long   " #sym "- .                             \n"     \
>             "   .long   __kstrtab_" #sym "- .                   \n"     \
> -           "   .long   0 - .                                   \n"     \
> +           "   .long   0                                       \n"     \
>             "   .previous                                       \n")
>
>  struct kernel_symbol {
> diff --git a/kernel/module.c b/kernel/module.c
> index f76efcf2043e..7ab244c4e1ba 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -547,6 +547,8 @@ static const char *kernel_symbol_name(const struct kernel_symbol *sym)
>  static const char *kernel_symbol_namespace(const struct kernel_symbol *sym)
>  {
>  #ifdef CONFIG_HAVE_ARCH_PREL32_RELOCATIONS
> +       if (!sym->namespace_offset)
> +               return NULL;
>         return offset_to_ptr(&sym->namespace_offset);
>  #else
>         return sym->namespace;
> --
> 2.23.0.162.g0b9fbb3734-goog
>