Page MenuHomeFreeBSD

modules: fix freebsd32_modstat on big endian platforms
ClosedPublic

Authored by khng on Jun 30 2023, 6:16 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sun, Nov 10, 10:28 AM
Unknown Object (File)
Thu, Oct 17, 2:50 PM
Unknown Object (File)
Thu, Oct 17, 12:45 AM
Unknown Object (File)
Tue, Oct 15, 1:23 PM
Unknown Object (File)
Oct 12 2024, 2:22 AM
Unknown Object (File)
Oct 11 2024, 9:57 AM
Unknown Object (File)
Oct 10 2024, 7:43 PM
Unknown Object (File)
Oct 9 2024, 10:23 PM
Subscribers

Details

Summary

The layout of modspecific_t on both little endian and big endian are as
follows:

|0|1|2|3|4|5|6|7|
+-------+-------+
|uintval|       |
+-------+-------+
|ulongval       |
+-------+-------+

For the following code snippet:

CP(mod->data, data32, longval);
CP(mod->data, data32, ulongval);

It only takes care of little endian platforms that it truncates the
highest 32bit automatically. However on big endian platforms it takes
the highest 32bit instead. This eventually returns a garbage syscall
number to the 32bit userland.

Since modspecific_t's usage currently is for the use of syscall modules,
we only initialize modspecific32_t with uintval. Now on both BE and LE
64-bit platforms it always pick up the first 4 bytes.

Sponsored by: Juniper Networks, Inc.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

khng requested review of this revision.Jun 30 2023, 6:16 PM
delphij added inline comments.
sys/kern/kern_module.c
525–526

Can you add a compile time assert that sizeof(data32) == sizeof(data32.uintval) so we don't need to zero data32?

(In fact I don't really like this approach, but it's probably the lesser evil without breaking ABI further. It would be much better if data is defined as a fixed width type on all platforms, but that would mean we would need v3 call.)

Seems ok to me Xin's suggestion incorporated.

This revision is now accepted and ready to land.Jul 4 2023, 3:53 PM

Seems ok to me Xin's suggestion incorporated.

Well, thinking about it a bit further, I'd also add a comment explaining why we only copy the one field.