Re: change in compat/linux breaking net/citrix_ica
On 4/26/23 16:00, Dmitry Chagin wrote: On Wed, Apr 26, 2023 at 09:01:00AM +0200, Jakob Alvermark wrote: Hi, I use net/citrix_ica for work. https://cgit.FreeBSD.org/src/commit/?id=76f8584e49cf7eedaa2e1312593bf46c7225d79a Yes, this works. Thanks for the quick response! After a recent change to -current in compat/linux it no longer works. The binary just segfaults. I have bisected and it happened after this commit: commit 40c36c4674eb9602709cf9d0483a4f34ad9753f6 Author: Dmitry Chagin Date: Sat Apr 22 22:17:17 2023 +0300 linux(4): Export the AT_RANDOM depending on the process osreldata AT_RANDOM has appeared in the 2.6.30 Linux kernel first time. Reviewed by: emaste Differential Revision: https://reviews.freebsd.org/D39646 MFC after: 1 month sys/compat/linux/linux_elf.c | 3 ++- sys/compat/linux/linux_mib.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) If I revert the change, citrix_ica works again. Thanks, Jakob Alvermark
Re: change in compat/linux breaking net/citrix_ica
On Wed, Apr 26, 2023 at 09:01:00AM +0200, Jakob Alvermark wrote: > Hi, > > > I use net/citrix_ica for work. https://cgit.FreeBSD.org/src/commit/?id=76f8584e49cf7eedaa2e1312593bf46c7225d79a > > After a recent change to -current in compat/linux it no longer works. > The binary just segfaults. > > I have bisected and it happened after this commit: > > commit 40c36c4674eb9602709cf9d0483a4f34ad9753f6 > Author: Dmitry Chagin > Date: Sat Apr 22 22:17:17 2023 +0300 > > linux(4): Export the AT_RANDOM depending on the process osreldata > > AT_RANDOM has appeared in the 2.6.30 Linux kernel first time. > > Reviewed by: emaste > Differential Revision: https://reviews.freebsd.org/D39646 > MFC after: 1 month > > sys/compat/linux/linux_elf.c | 3 ++- > sys/compat/linux/linux_mib.h | 1 + > 2 files changed, 3 insertions(+), 1 deletion(-) > > > If I revert the change, citrix_ica works again. > > > Thanks, > > Jakob Alvermark >
Re: Link modules to DYN type
> 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
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: github CI failures related to tzsetup
Yuri wrote:
> Looking at the CI jobs on github, all seem to fail in tzsetup while
> making kernel-toolchain target. Obviously this build for me locally,
> FreeBSD's CI is fine with it, it's only ubuntu and macos jobs reporting
> the errors, and I don't see why; any hints?
>
> https://github.com/freebsd/freebsd-src/actions/runs/4808074516/jobs/8557602371#step:7:878
>
> ===> usr.sbin/tzsetup (obj,all,install)
> /home/runner/work/freebsd-src/freebsd-src/usr.sbin/tzsetup/tzsetup.c: In
> function ‘dump_zonetab’:
> /home/runner/work/freebsd-src/freebsd-src/usr.sbin/tzsetup/tzsetup.c:809:12:
> error: ‘countries’ undeclared (first use in this function); did you mean
> ‘country’?
> 809 | for (cp = countries; cp->name != NULL; cp++) {
> |^
> |country
>
Got it, the definition is hidden under the ifdef BSDDIALOG, will fix.
github CI failures related to tzsetup
Looking at the CI jobs on github, all seem to fail in tzsetup while
making kernel-toolchain target. Obviously this build for me locally,
FreeBSD's CI is fine with it, it's only ubuntu and macos jobs reporting
the errors, and I don't see why; any hints?
https://github.com/freebsd/freebsd-src/actions/runs/4808074516/jobs/8557602371#step:7:878
===> usr.sbin/tzsetup (obj,all,install)
/home/runner/work/freebsd-src/freebsd-src/usr.sbin/tzsetup/tzsetup.c: In
function ‘dump_zonetab’:
/home/runner/work/freebsd-src/freebsd-src/usr.sbin/tzsetup/tzsetup.c:809:12:
error: ‘countries’ undeclared (first use in this function); did you mean
‘country’?
809 | for (cp = countries; cp->name != NULL; cp++) {
|^
|country
Re: Link modules to DYN type
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
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
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
Link modules to DYN type
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 ?
Best regards,
Zhenlei
Re: change in compat/linux breaking net/citrix_ica
On Wed, Apr 26, 2023 at 09:01:00AM +0200, Jakob Alvermark wrote: > Hi, > > > I use net/citrix_ica for work. > > After a recent change to -current in compat/linux it no longer works. > The binary just segfaults. > > I have bisected and it happened after this commit: > > commit 40c36c4674eb9602709cf9d0483a4f34ad9753f6 > Author: Dmitry Chagin > Date: Sat Apr 22 22:17:17 2023 +0300 > > linux(4): Export the AT_RANDOM depending on the process osreldata > > AT_RANDOM has appeared in the 2.6.30 Linux kernel first time. > > Reviewed by: emaste > Differential Revision: https://reviews.freebsd.org/D39646 > MFC after: 1 month > > sys/compat/linux/linux_elf.c | 3 ++- > sys/compat/linux/linux_mib.h | 1 + > 2 files changed, 3 insertions(+), 1 deletion(-) > > > If I revert the change, citrix_ica works again. > Hi, thanks, I'll commit fix today after some testing > > Thanks, > > Jakob Alvermark >
Re: change in compat/linux breaking net/citrix_ica
Quoting Jakob Alvermark (from Wed, 26 Apr 2023 09:01:00 +0200): Hi, I use net/citrix_ica for work. After a recent change to -current in compat/linux it no longer works. The binary just segfaults. What does "sysctl compat.linux.osrelease" display? If it is not 2.6.30 or higher, try to set it to 2.6.30 or higher. Bye, Alexander. I have bisected and it happened after this commit: commit 40c36c4674eb9602709cf9d0483a4f34ad9753f6 Author: Dmitry Chagin Date: Sat Apr 22 22:17:17 2023 +0300 linux(4): Export the AT_RANDOM depending on the process osreldata AT_RANDOM has appeared in the 2.6.30 Linux kernel first time. -- http://www.Leidinger.net alexan...@leidinger.net: PGP 0x8F31830F9F2772BF http://www.FreeBSD.orgnetch...@freebsd.org : PGP 0x8F31830F9F2772BF pgpvwszGFGPAo.pgp Description: Digitale PGP-Signatur
change in compat/linux breaking net/citrix_ica
Hi, I use net/citrix_ica for work. After a recent change to -current in compat/linux it no longer works. The binary just segfaults. I have bisected and it happened after this commit: commit 40c36c4674eb9602709cf9d0483a4f34ad9753f6 Author: Dmitry Chagin Date: Sat Apr 22 22:17:17 2023 +0300 linux(4): Export the AT_RANDOM depending on the process osreldata AT_RANDOM has appeared in the 2.6.30 Linux kernel first time. Reviewed by: emaste Differential Revision: https://reviews.freebsd.org/D39646 MFC after: 1 month sys/compat/linux/linux_elf.c | 3 ++- sys/compat/linux/linux_mib.h | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) If I revert the change, citrix_ica works again. Thanks, Jakob Alvermark
