Re: Link modules to DYN type

[Zhenlei Huang]

> On Apr 28, 2023, at 12:32 AM, Zhenlei Huang  wrote:
> 
> 
> 
>> On Apr 26, 2023, at 7:12 PM, Konstantin Belousov > > wrote:
>> 
>> On Wed, Apr 26, 2023 at 12:55:02PM +0200, Hans Petter Selasky wrote:
>>> On 4/26/23 12:36, Zhenlei Huang wrote:
 Hi,
 
 I'm recently working on https://reviews.freebsd.org/D39638 
  (sysctl(9): Enable vnet sysctl 
 variables be loader tunable),
 the changes to `sys/kern/link_elf_obj.c` are runtime tested, but not those 
 to `sys/kern/link_elf.c` .
 
 After some hacking I realized that `link_elf.c` is for EXEC (Executable 
 file) or DYN (Shared object file), and `link_elf_obj.c` is
 for REL (Relocatable file).
 
 ```
 /* link_elf.c */
 static int
 link_elf_load_file(linker_class_t cls, const char* filename,
 linker_file_t* result)
 {
 ...
if (hdr->e_type != ET_EXEC && hdr->e_type != ET_DYN) {
error = ENOSYS;
goto out;
}
 ...
 }
 
 
 /* link_elf_obj.c */
 static int
 link_elf_load_file(linker_class_t cls, const char *filename,
 linker_file_t *result)
 {
 ...
if (hdr->e_type != ET_REL) {
error = ENOSYS;
goto out;
}
 ...
 }
 ```
 
 Run the following snip:
 ```
 # find /boot/kernel -type f -name "*.ko" -exec readelf -h {} \; | grep Type
 ```
 shows that all the kernel modules' types are `REL (Relocatable file)`.
 
 I guess if some module such as if_bridge is linked to DYN type, then I can 
 do runtime for the changes to `sys/kern/link_elf.c`.
 
 I'm not familiar with elf and linkers, is that ( compile module and link 
 it to DYN type ) possible ?
>> 
>> Module file type (shared object vs. object file) depends on architecture.
>> For amd64 modules are objects, while kernel is shared library.
>> For arm64 (and all other arches, I believe) modules and kernels are shared
>> libraries.
>> 
>> I think you can link amd64 module as shared object, but this require enough
>> hacking of the build infrastructure.  At least I am not aware of a simple
>> knob to switch the produced type.
> 
> I did some hack on `sys/conf/kmod.mk` and finally produced DYN kernel modules.
> The good news is I do some basic sysctl tests, but the bad news is the module 
> does not function correctly.
> 
> For exampe the if_disc.c
> 
> ```
> static void
> vnet_disc_init(const void *unused __unused)
> {
>   /* Reference V_disc_cloner will immediately trigger page fault panic */
>   V_disc_cloner = if_clone_simple(discname, disc_clone_create,
>   disc_clone_destroy, 0);
> }
> VNET_SYSINIT(vnet_disc_init, SI_SUB_PSEUDO, SI_ORDER_ANY,
> vnet_disc_init, NULL);
> ```
> 
> I suspect the relocation is not done correctly for  DYN elf kmod on amd64.
> 
> My local patch to kmod.mk:
> 
> ```
> diff --git a/sys/conf/kmod.mk b/sys/conf/kmod.mk
> index 134b150af1d9..1fc5386204a5 100644
> --- a/sys/conf/kmod.mk
> +++ b/sys/conf/kmod.mk
> @@ -84,6 +84,7 @@ __KLD_SHARED=yes
>  .else
>  __KLD_SHARED=no
>  .endif
> +__KLD_SHARED=yes
>  
>  .if !empty(CFLAGS:M-O[23s]) && empty(CFLAGS:M-fno-strict-aliasing)
>  CFLAGS+=   -fno-strict-aliasing
> @@ -167,6 +168,7 @@ CFLAGS+=-fno-omit-frame-pointer 
> -mno-omit-leaf-frame-pointer
>  ${MACHINE_CPUARCH} == "powerpc"
>  CFLAGS+=   -fPIC
>  .endif
> +CFLAGS+=   -fPIC
>  
>  # Temporary workaround for PR 196407, which contains the fascinating details.
>  # Don't allow clang to use fpu instructions or registers in kernel modules.
> ```
> 
> 
> As for https://reviews.freebsd.org/D39638 
> , for other platform such as arm, I think 
> the `link_elf_propagate_vnets()` should work if `parse_vnet()` works.
> 
> I'll appreciate if someone can test it on platforms those have DYN type 
> kernel modules. 

Good news on this!

I've managed to test the change for `link_elf.c` on QEMU RISCV . It works as 
expected and solid !


> 
>> 
>> 
 
>>> 
>>> Hi,
>>> 
>>> I don't have an answer for you either, but I have seen in the past, loading
>>> kernel modules behaves a bit like libraries, in the following regard:
>>> 
>>> If two kernel modules define the same global symbol, then no warning is
>>> given and the first loaded symbol definition (I think) is used to resolve
>>> that symbol for all kernel modules, regardless of the prototype. Probably we
>>> should not allow this. That's why building LINT is a good thing, to avoid
>>> this issue.
>> No, in-kernel linker does not behave this way.
>> Modules need to contain explicit reference to all modules they depend upon,
>> using the MODULE_DEPEND() macro.  Only symbols from the dependencies are
>> resolved.
>> 
>> All modules get an implicit reference to kernel.
>> 
>>> 
>>> Even if we don't have C++ support in the FreeBSD kernel, defining symbol

Re: Link modules to DYN type

[Zhenlei Huang]

> On Apr 26, 2023, at 7:12 PM, Konstantin Belousov  wrote:
> 
> On Wed, Apr 26, 2023 at 12:55:02PM +0200, Hans Petter Selasky wrote:
>> On 4/26/23 12:36, Zhenlei Huang wrote:
>>> Hi,
>>> 
>>> I'm recently working on https://reviews.freebsd.org/D39638 (sysctl(9): 
>>> Enable vnet sysctl variables be loader tunable),
>>> the changes to `sys/kern/link_elf_obj.c` are runtime tested, but not those 
>>> to `sys/kern/link_elf.c` .
>>> 
>>> After some hacking I realized that `link_elf.c` is for EXEC (Executable 
>>> file) or DYN (Shared object file), and `link_elf_obj.c` is
>>> for REL (Relocatable file).
>>> 
>>> ```
>>> /* link_elf.c */
>>> static int
>>> link_elf_load_file(linker_class_t cls, const char* filename,
>>> linker_file_t* result)
>>> {
>>> ...
>>> if (hdr->e_type != ET_EXEC && hdr->e_type != ET_DYN) {
>>> error = ENOSYS;
>>> goto out;
>>> }
>>> ...
>>> }
>>> 
>>> 
>>> /* link_elf_obj.c */
>>> static int
>>> link_elf_load_file(linker_class_t cls, const char *filename,
>>> linker_file_t *result)
>>> {
>>> ...
>>> if (hdr->e_type != ET_REL) {
>>> error = ENOSYS;
>>> goto out;
>>> }
>>> ...
>>> }
>>> ```
>>> 
>>> Run the following snip:
>>> ```
>>> # find /boot/kernel -type f -name "*.ko" -exec readelf -h {} \; | grep Type
>>> ```
>>> shows that all the kernel modules' types are `REL (Relocatable file)`.
>>> 
>>> I guess if some module such as if_bridge is linked to DYN type, then I can 
>>> do runtime for the changes to `sys/kern/link_elf.c`.
>>> 
>>> I'm not familiar with elf and linkers, is that ( compile module and link it 
>>> to DYN type ) possible ?
> 
> Module file type (shared object vs. object file) depends on architecture.
> For amd64 modules are objects, while kernel is shared library.
> For arm64 (and all other arches, I believe) modules and kernels are shared
> libraries.
> 
> I think you can link amd64 module as shared object, but this require enough
> hacking of the build infrastructure.  At least I am not aware of a simple
> knob to switch the produced type.

I did some hack on `sys/conf/kmod.mk` and finally produced DYN kernel modules.
The good news is I do some basic sysctl tests, but the bad news is the module 
does not function correctly.

For exampe the if_disc.c

```
static void
vnet_disc_init(const void *unused __unused)
{
/* Reference V_disc_cloner will immediately trigger page fault panic */
V_disc_cloner = if_clone_simple(discname, disc_clone_create,
disc_clone_destroy, 0);
}
VNET_SYSINIT(vnet_disc_init, SI_SUB_PSEUDO, SI_ORDER_ANY,
vnet_disc_init, NULL);
```

I suspect the relocation is not done correctly for  DYN elf kmod on amd64.

My local patch to kmod.mk:

```
diff --git a/sys/conf/kmod.mk b/sys/conf/kmod.mk
index 134b150af1d9..1fc5386204a5 100644
--- a/sys/conf/kmod.mk
+++ b/sys/conf/kmod.mk
@@ -84,6 +84,7 @@ __KLD_SHARED=yes
 .else
 __KLD_SHARED=no
 .endif
+__KLD_SHARED=yes
 
 .if !empty(CFLAGS:M-O[23s]) && empty(CFLAGS:M-fno-strict-aliasing)
 CFLAGS+=   -fno-strict-aliasing
@@ -167,6 +168,7 @@ CFLAGS+=-fno-omit-frame-pointer 
-mno-omit-leaf-frame-pointer
 ${MACHINE_CPUARCH} == "powerpc"
 CFLAGS+=   -fPIC
 .endif
+CFLAGS+=   -fPIC
 
 # Temporary workaround for PR 196407, which contains the fascinating details.
 # Don't allow clang to use fpu instructions or registers in kernel modules.
```


As for https://reviews.freebsd.org/D39638 , 
for other platform such as arm, I think the `link_elf_propagate_vnets()` should 
work if `parse_vnet()` works.

I'll appreciate if someone can test it on platforms those have DYN type kernel 
modules. 

> 
> 
>>> 
>> 
>> Hi,
>> 
>> I don't have an answer for you either, but I have seen in the past, loading
>> kernel modules behaves a bit like libraries, in the following regard:
>> 
>> If two kernel modules define the same global symbol, then no warning is
>> given and the first loaded symbol definition (I think) is used to resolve
>> that symbol for all kernel modules, regardless of the prototype. Probably we
>> should not allow this. That's why building LINT is a good thing, to avoid
>> this issue.
> No, in-kernel linker does not behave this way.
> Modules need to contain explicit reference to all modules they depend upon,
> using the MODULE_DEPEND() macro.  Only symbols from the dependencies are
> resolved.
> 
> All modules get an implicit reference to kernel.
> 
>> 
>> Even if we don't have C++ support in the FreeBSD kernel, defining symbol
>> names the way C++ does for C could be nice for the kernel too, also with
>> regards to debugging systems.
>> 
>> Many times when I don't know what is going on, I do like this:
>> 
>> #include 
>> 
>> 
>> 
>> if (not too fast or my sysctl debug) {
>>  printf("My tracer\n");
>>  kdb_backtrace();
>> }
>> 
>> Dtrace can also do this, but not during boot. Just track who is calling
>> those functions, and you'll probably find the answer 

Re: Link modules to DYN type

[Jan Martin Mikkelsen]

> On 26. Apr 2023, at 13:38, Hans Petter Selasky  wrote:
> 
> On 4/26/23 13:12, Konstantin Belousov wrote:
>> No, in-kernel linker does not behave this way.
>> Modules need to contain explicit reference to all modules they depend upon,
>> using the MODULE_DEPEND() macro.  Only symbols from the dependencies are
>> resolved.
>> All modules get an implicit reference to kernel.
> 
> Hi Konstantin,
> 
> Maybe I wasn't so clear. Trying again:
> 
>> diff --git a/sys/tests/ktest.c b/sys/tests/ktest.c
>> index 495fedf95dde..eb42cf062487 100644
>> --- a/sys/tests/ktest.c
>> +++ b/sys/tests/ktest.c
>> @@ -409,6 +409,12 @@ static moduledata_t ktestmod = {
>> 0
>> };
>> +int
>> +printf(const char *fmt, ...)
>> +{
>> +   return (0);
>> +}
>> +
>> DECLARE_MODULE(ktestmod, ktestmod, SI_SUB_PSEUDO, SI_ORDER_ANY);
>> MODULE_VERSION(ktestmod, 1);
>> MODULE_DEPEND(ktestmod, netlink, 1, 1, 1);
> 
> Then kldload ktest.ko . Which printf() function will be used if ktest.c calls 
> printf() ?
> 
> I would expect a warning from the kernel at least …


Hi,

This looks similar to this:

https://lists.freebsd.org/pipermail/freebsd-stable/2015-July/082751.html

https://lists.freebsd.org/pipermail/freebsd-stable/2015-July/082760.html

The “not knowing” about how symbols are going to be resolved has bothered me 
for a while.

Regards,

Jan M.

Re: Link modules to DYN type

[Konstantin Belousov]

On Wed, Apr 26, 2023 at 01:38:32PM +0200, Hans Petter Selasky wrote:
> On 4/26/23 13:12, Konstantin Belousov wrote:
> > No, in-kernel linker does not behave this way.
> > Modules need to contain explicit reference to all modules they depend upon,
> > using the MODULE_DEPEND() macro.  Only symbols from the dependencies are
> > resolved.
> > 
> > All modules get an implicit reference to kernel.
> 
> Hi Konstantin,
> 
> Maybe I wasn't so clear. Trying again:
> 
> > diff --git a/sys/tests/ktest.c b/sys/tests/ktest.c
> > index 495fedf95dde..eb42cf062487 100644
> > --- a/sys/tests/ktest.c
> > +++ b/sys/tests/ktest.c
> > @@ -409,6 +409,12 @@ static moduledata_t ktestmod = {
> >  0
> >  };
> > +int
> > +printf(const char *fmt, ...)
> > +{
> > +   return (0);
> > +}
> > +
> >  DECLARE_MODULE(ktestmod, ktestmod, SI_SUB_PSEUDO, SI_ORDER_ANY);
> >  MODULE_VERSION(ktestmod, 1);
> >  MODULE_DEPEND(ktestmod, netlink, 1, 1, 1);
> 
> Then kldload ktest.ko . Which printf() function will be used if ktest.c
> calls printf() ?
My best guess is that printf is resolved locally by the static linker,
even before the module is loaded.  In other words, it would be ktest.c
printf.

> 
> I would expect a warning from the kernel at least ...
This is not how ELF behaves, even in such not fully compliant env as kernel.

Re: Link modules to DYN type

[Hans Petter Selasky]

On 4/26/23 13:12, Konstantin Belousov wrote:

No, in-kernel linker does not behave this way.
Modules need to contain explicit reference to all modules they depend upon,
using the MODULE_DEPEND() macro.  Only symbols from the dependencies are
resolved.

All modules get an implicit reference to kernel.


Hi Konstantin,

Maybe I wasn't so clear. Trying again:


diff --git a/sys/tests/ktest.c b/sys/tests/ktest.c
index 495fedf95dde..eb42cf062487 100644
--- a/sys/tests/ktest.c
+++ b/sys/tests/ktest.c
@@ -409,6 +409,12 @@ static moduledata_t ktestmod = {
 0
 };
 
+int

+printf(const char *fmt, ...)
+{
+   return (0);
+}
+
 DECLARE_MODULE(ktestmod, ktestmod, SI_SUB_PSEUDO, SI_ORDER_ANY);
 MODULE_VERSION(ktestmod, 1);
 MODULE_DEPEND(ktestmod, netlink, 1, 1, 1);


Then kldload ktest.ko . Which printf() function will be used if ktest.c 
calls printf() ?


I would expect a warning from the kernel at least ...

--HPS

Re: Link modules to DYN type

[Konstantin Belousov]

On Wed, Apr 26, 2023 at 12:55:02PM +0200, Hans Petter Selasky wrote:
> On 4/26/23 12:36, Zhenlei Huang wrote:
> > Hi,
> > 
> > I'm recently working on https://reviews.freebsd.org/D39638 (sysctl(9): 
> > Enable vnet sysctl variables be loader tunable),
> > the changes to `sys/kern/link_elf_obj.c` are runtime tested, but not those 
> > to `sys/kern/link_elf.c` .
> > 
> > After some hacking I realized that `link_elf.c` is for EXEC (Executable 
> > file) or DYN (Shared object file), and `link_elf_obj.c` is
> > for REL (Relocatable file).
> > 
> > ```
> > /* link_elf.c */
> > static int
> > link_elf_load_file(linker_class_t cls, const char* filename,
> >  linker_file_t* result)
> > {
> > ...
> > if (hdr->e_type != ET_EXEC && hdr->e_type != ET_DYN) {
> > error = ENOSYS;
> > goto out;
> > }
> > ...
> > }
> > 
> > 
> > /* link_elf_obj.c */
> > static int
> > link_elf_load_file(linker_class_t cls, const char *filename,
> >  linker_file_t *result)
> > {
> > ...
> > if (hdr->e_type != ET_REL) {
> > error = ENOSYS;
> > goto out;
> > }
> > ...
> > }
> > ```
> > 
> > Run the following snip:
> > ```
> > # find /boot/kernel -type f -name "*.ko" -exec readelf -h {} \; | grep Type
> > ```
> > shows that all the kernel modules' types are `REL (Relocatable file)`.
> > 
> > I guess if some module such as if_bridge is linked to DYN type, then I can 
> > do runtime for the changes to `sys/kern/link_elf.c`.
> > 
> > I'm not familiar with elf and linkers, is that ( compile module and link it 
> > to DYN type ) possible ?

Module file type (shared object vs. object file) depends on architecture.
For amd64 modules are objects, while kernel is shared library.
For arm64 (and all other arches, I believe) modules and kernels are shared
libraries.

I think you can link amd64 module as shared object, but this require enough
hacking of the build infrastructure.  At least I am not aware of a simple
knob to switch the produced type.


> > 
> 
> Hi,
> 
> I don't have an answer for you either, but I have seen in the past, loading
> kernel modules behaves a bit like libraries, in the following regard:
> 
> If two kernel modules define the same global symbol, then no warning is
> given and the first loaded symbol definition (I think) is used to resolve
> that symbol for all kernel modules, regardless of the prototype. Probably we
> should not allow this. That's why building LINT is a good thing, to avoid
> this issue.
No, in-kernel linker does not behave this way.
Modules need to contain explicit reference to all modules they depend upon,
using the MODULE_DEPEND() macro.  Only symbols from the dependencies are
resolved.

All modules get an implicit reference to kernel.

> 
> Even if we don't have C++ support in the FreeBSD kernel, defining symbol
> names the way C++ does for C could be nice for the kernel too, also with
> regards to debugging systems.
> 
> Many times when I don't know what is going on, I do like this:
> 
> #include 
> 
> 
> 
> if (not too fast or my sysctl debug) {
>   printf("My tracer\n");
>   kdb_backtrace();
> }
> 
> Dtrace can also do this, but not during boot. Just track who is calling
> those functions, and you'll probably find the answer to your question!
> 
> --HPS
> 
> > 
> > Best regards,
> > Zhenlei
> > 
> 

Re: Link modules to DYN type

[Hans Petter Selasky]

On 4/26/23 12:36, Zhenlei Huang wrote:

Hi,

I'm recently working on https://reviews.freebsd.org/D39638 (sysctl(9): Enable 
vnet sysctl variables be loader tunable),
the changes to `sys/kern/link_elf_obj.c` are runtime tested, but not those to 
`sys/kern/link_elf.c` .

After some hacking I realized that `link_elf.c` is for EXEC (Executable file) 
or DYN (Shared object file), and `link_elf_obj.c` is
for REL (Relocatable file).

```
/* link_elf.c */
static int
link_elf_load_file(linker_class_t cls, const char* filename,
 linker_file_t* result)
{
...
if (hdr->e_type != ET_EXEC && hdr->e_type != ET_DYN) {
error = ENOSYS;
goto out;
}
...
}


/* link_elf_obj.c */
static int
link_elf_load_file(linker_class_t cls, const char *filename,
 linker_file_t *result)
{
...
if (hdr->e_type != ET_REL) {
error = ENOSYS;
goto out;
}
...
}
```

Run the following snip:
```
# find /boot/kernel -type f -name "*.ko" -exec readelf -h {} \; | grep Type
```
shows that all the kernel modules' types are `REL (Relocatable file)`.

I guess if some module such as if_bridge is linked to DYN type, then I can do 
runtime for the changes to `sys/kern/link_elf.c`.

I'm not familiar with elf and linkers, is that ( compile module and link it to 
DYN type ) possible ?



Hi,

I don't have an answer for you either, but I have seen in the past, 
loading kernel modules behaves a bit like libraries, in the following 
regard:


If two kernel modules define the same global symbol, then no warning is 
given and the first loaded symbol definition (I think) is used to 
resolve that symbol for all kernel modules, regardless of the prototype. 
Probably we should not allow this. That's why building LINT is a good 
thing, to avoid this issue.


Even if we don't have C++ support in the FreeBSD kernel, defining symbol 
names the way C++ does for C could be nice for the kernel too, also with 
regards to debugging systems.


Many times when I don't know what is going on, I do like this:

#include 



if (not too fast or my sysctl debug) {
  printf("My tracer\n");
  kdb_backtrace();
}

Dtrace can also do this, but not during boot. Just track who is calling 
those functions, and you'll probably find the answer to your question!


--HPS



Best regards,
Zhenlei