freebsd stable - Jul 2015 - amd64 kernel dynamic linking allows extern references to statics

Hi,

(All on 10.2-BETA1.)

I noticed that the latest patch in the bug
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=187594
<https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=187594> works on amd64
but fails to load zfs.ko on i386 with a symbol not found error.

Looking at the patch, there is one file that has ?extern int zio_use_uma? to
reference a variable that is declared elsewhere as ?static int zio_use_uma?. To
me this obviously should not work. However it does work on amd64 but fails on
i386.

Below is a small test case that reproduces the problem. The generated kernel
module loads on amd64 but fails on i386. On amd64 one compilation unit is
accessing a static in from another compilation unit by declaring the variable
?extern?.

I haven?t looked further to attempt to find the bug. However, it looks like a
Bad Thing? to me.

Regards,

Jan.

??Makefile:

KMOD=   refstatic
SRCS=   refstatic.c testfunc.c bus_if.h device_if.h pci_if.h

.include <bsd.kmod.mk>



?? refstatic.c:

#include <sys/param.h>
#include <sys/kernel.h>
#include <sys/bus.h>
#include <sys/module.h>
#include <sys/systm.h>
#include <sys/types.h>
#include <sys/malloc.h>
#include <sys/kthread.h>

#include <sys/conf.h>
#include <sys/uio.h>
#include <sys/bus.h>
#include <sys/rman.h>
#include <sys/queue.h>

#include <dev/pci/pcireg.h>
#include <dev/pci/pcivar.h>

#include <machine/bus.h>
#include <machine/resource.h>

static int testvar = 1;
int func(void);

static int refstatic_probe(device_t dev)
{
    ++testvar;
    device_printf(dev, "func returns %d\n", func());
    return ENXIO;
}

static int refstatic_attach(device_t dev)
{
    (void) dev;
    return ENXIO;
}

static int refstatic_detach(device_t dev)
{
    return 0;
}


static device_method_t refstatic_methods[] = {
    DEVMETHOD(device_probe, refstatic_probe),
    DEVMETHOD(device_attach, refstatic_attach),
    DEVMETHOD(device_detach, refstatic_detach),
    { 0, 0 }
};

static driver_t refstatic_driver = {
    "refstatic",
    refstatic_methods,
    64
};

static devclass_t refstatic_devclass;

DRIVER_MODULE(refstatic, pci, refstatic_driver, refstatic_devclass, 0, 0);

??testfunc.c:

extern int testvar;

int func(void);

int func(void)
{
    return testvar;
}

On Wed, Jul 15, 2015 at 06:17:20PM +1000, Jan Mikkelsen
wrote:
> Hi,
> 
> (All on 10.2-BETA1.)
> 
> I noticed that the latest patch in the bug
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=187594
<https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=187594> works on amd64
but fails to load zfs.ko on i386 with a symbol not found error.
> 
> Looking at the patch, there is one file that has ???extern int
zio_use_uma??? to reference a variable that is declared elsewhere as ???static
int zio_use_uma???. To me this obviously should not work. However it does work
on amd64 but fails on i386.
> 
> Below is a small test case that reproduces the problem. The generated
kernel module loads on amd64 but fails on i386. On amd64 one compilation unit is
accessing a static in from another compilation unit by declaring the variable
???extern???.
> 
> I haven???t looked further to attempt to find the bug. However, it looks
like a Bad Thing??? to me.
> 
I am not sure that this is fixable.  Issue is that amd64 modules are
relinked object files, and they might have unresolved relocations against
local symbols.  Change like the following probably fix your test case,
but also quite possible would break legitimate local references.

diff --git a/sys/kern/link_elf_obj.c b/sys/kern/link_elf_obj.c
index 021381d..6fa5276 100644
--- a/sys/kern/link_elf_obj.c
+++ b/sys/kern/link_elf_obj.c
@@ -1096,7 +1096,8 @@ link_elf_lookup_symbol(linker_file_t lf, const char *name,
c_linker_sym_t *sym)
 
 	for (i = 0, symp = ef->ddbsymtab; i < ef->ddbsymcnt; i++, symp++) {
 		strp = ef->ddbstrtab + symp->st_name;
-		if (symp->st_shndx != SHN_UNDEF && strcmp(name, strp) == 0) {
+		if (symp->st_shndx != SHN_UNDEF && strcmp(name, strp) == 0
&&
+		    ELF_ST_BIND(symp->st_info) != STB_LOCAL) {
 			*sym = (c_linker_sym_t) symp;
 			return 0;
 		}