use / misuse of EXPORT_SYMBOL ?

From: jim . cromie
Date: Fri Dec 23 2022 - 20:38:16 EST

Next message: Sasha Levin: "[PATCH AUTOSEL 5.10 06/11] chardev: Fix potential memory leak when cdev_add() failed"
Previous message: Sasha Levin: "[PATCH AUTOSEL 5.10 03/11] usb: gadget: aspeed: fix buffer overflow"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

hi folks,

so Im trying to fix a regression in
CONFIG_DRM_USE_DYNAMIC_DEBUG=y
drm.debugs in drivers and helpers arent enabled
cuz theyre not modprobed until after drm,
when its debug=0x1f is applied.
Previously, __drm_debug settings were seen by drm_dbg when called,
but now with dyndbg underneath, that runtime test is replaced
by a static-key, which missed the debug callback and enablement

I want to export a symbol / struct _var
from inside a macro, the _var is placed into its own
linker section, but this appears to make it un-linkable
(at least not the way Im expecting)

WIP patchset at
https://github.com/jimc/linux/tree/hf-5f

Heres the final change, and the ensuing compile errs,
which are more helpful than the previous: silence and misbehavior.

The change isnt correct, but I have a hunch that
the errs reveal something about the underlying problem.

[jimc@frodo f2]$ git diff
diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index 02a66fd047b2..1d88ebf00bb1 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -111,7 +111,7 @@ struct ddebug_class_map {
.length = ARRAY_SIZE(_var##_classnames), \
.class_names = _var##_classnames, \
}; \
- EXPORT_SYMBOL(_var)
+ __EXPORT_SYMBOL(_var, "__dyndbg_classes", "")

struct ddebug_class_user {
const char *user_mod_name;
[jimc@frodo f2]$ make
....
LD vmlinux.o
OBJCOPY modules.builtin.modinfo
GEN modules.builtin
MODPOST Module.symvers
ERROR: modpost: Could not update namespace() for symbol map_disjoint_bits
ERROR: modpost: Could not update namespace() for symbol map_disjoint_names
ERROR: modpost: Could not update namespace() for symbol map_level_num
ERROR: modpost: Could not update namespace() for symbol map_level_names
ERROR: modpost: Could not update namespace() for symbol drm_debug_classes
ERROR: modpost: "map_level_names" [lib/test_dynamic_debug_submod.ko] undefined!
ERROR: modpost: "map_level_num" [lib/test_dynamic_debug_submod.ko] undefined!
ERROR: modpost: "map_disjoint_names"
[lib/test_dynamic_debug_submod.ko] undefined!
ERROR: modpost: "map_disjoint_bits" [lib/test_dynamic_debug_submod.ko]
undefined!
ERROR: modpost: "drm_debug_classes"
[drivers/gpu/drm/display/drm_display_helper.ko] undefined!
ERROR: modpost: "drm_debug_classes"
[drivers/gpu/drm/drm_kms_helper.ko] undefined!
ERROR: modpost: "drm_debug_classes"
[drivers/gpu/drm/amd/amdgpu/amdgpu.ko] undefined!
ERROR: modpost: "drm_debug_classes" [drivers/gpu/drm/i915/i915.ko] undefined!
ERROR: modpost: "drm_debug_classes"
[drivers/gpu/drm/nouveau/nouveau.ko] undefined!
make[1]: *** [/home/jimc/projects/lx/wk-test/scripts/Makefile.modpost:126:
Module.symvers] Error 1
make: *** [/home/jimc/projects/lx/wk-test/Makefile:1944: modpost] Error 2
[jimc@frodo f2]$

Heres the relevant macros, including above change.

/**
* DYNDBG_CLASSMAP_DEFINE - define the debug classes used in this module.
* This tells dyndbg what debug classes it should control for the client.
*
* @_var: struct ddebug_class_map, as passed to module_param_cb
* @_type: enum ddebug_class_map_type, chooses bits/verbose, numeric/symbolic
* @classes: enum class values used in module, such as: DRM_UT_*
*/
#define DYNDBG_CLASSMAP_DEFINE(_var, _maptype, _base, ...) \
const char *_var##_classnames[] = { \
iMAP_LIST(__stringify, _base, __VA_ARGS__) }; \
struct ddebug_class_map __aligned(8) __used \
__section("__dyndbg_classes") _var = { \
.mod = THIS_MODULE, \
.mod_name = KBUILD_MODNAME, \
.base = _base, \
.map_type = _maptype, \
.length = ARRAY_SIZE(_var##_classnames), \
.class_names = _var##_classnames, \
}; \
__EXPORT_SYMBOL(_var, "__dyndbg_classes", "")

struct ddebug_class_user {
const char *user_mod_name;
const struct ddebug_class_map *map;
};
/**
* DYNDBG_CLASSMAP_USE - refer to a classmap, DEFINEd elsewhere.
* @_var: name of the exported classmap
*
* This registers the module's use of another module's classmap defn,
* allowing dyndbg to find the controlling kparam, and propagate its
* settings to the dependent module being loaded.
*/
#define DYNDBG_CLASSMAP_USE(_var) \
DYNDBG_CLASSMAP_USE_(_var, __UNIQUE_ID(ddebug_class_user))
#define DYNDBG_CLASSMAP_USE_(_var, _uname) \
extern struct ddebug_class_map _var; \
struct ddebug_class_user __used \
__section("__dyndbg_class_refs") _uname = { \
.user_mod_name = KBUILD_MODNAME, \
.map = &_var, \
}

Translating that for the skimmers -
_DEFINE creates a record in __dyndbg_classes section, exports it.
_USE creates a user-record, in __dyndbg_class_refs section,
which refs the 1st record.

plain old EXPORT_SYMBOL doesnt know the __dyndbg_classes section,
so users dont get it either, they somehow get new records,
which are "wrong" (have pointer to disallowed addr)full
The "ddebug_sanity" patch - 21/22 - isolates the problem linkage
to very early in dynamic_debug_init()

delving into the error itself, its from modpost.c:
static void sym_update_namespace(const char *symname, const char *namespace)

which is called from:
static void read_symbols(const char *modname)
...
for (sym = info.symtab_start; sym < info.symtab_stop; sym++) {
symname = remove_dot(info.strtab + sym->st_name);

/* Apply symbol namespaces from __kstrtabns_<symbol> entries. */
if (strstarts(symname, "__kstrtabns_"))
sym_update_namespace(symname + strlen("__kstrtabns_"),
sym_get_data(&info, sym));
}

consulting git blame leads me to:

commit 69923208431e097ce3830647aee98e5bd3e889c8
Author: Matthias Maennich <maennich@xxxxxxxxxx>
Date: Fri Oct 18 10:31:42 2019 +0100

symbol namespaces: revert to previous __ksymtab name scheme

The introduction of Symbol Namespaces changed the naming schema of the
__ksymtab entries from __kysmtab__symbol to __ksymtab_NAMESPACE.symbol.

That caused some breakages in tools that depend on the name layout in
either the binaries(vmlinux,*.ko) or in System.map. E.g. kmod's depmod
would not be able to read System.map without a patch to support symbol
namespaces. A warning reported by depmod for namespaced symbols would
look like

depmod: WARNING: [...]/uas.ko needs unknown symbol usb_stor_adjust_quirks

In order to address this issue, revert to the original naming scheme and
rather read the __kstrtabns_<symbol> entries and their corresponding
values from __ksymtab_strings to update the namespace values for
symbols. After having read all symbols and handled them in
handle_modversions(), the symbols are created. In a second pass, read
the __kstrtabns_ entries and update the namespaces accordingly.

Fixes: 8651ec01daed ("module: add support for symbol namespaces.")
Reported-by: Stefan Wahren <stefan.wahren@xxxxxxxx>
Suggested-by: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
Acked-by: Will Deacon <will@xxxxxxxxxx>
Reviewed-by: Greg Kroah-Hartman <gregkh@xxxxxxxxxxxxxxxxxxx>
Reviewed-by: Masahiro Yamada <yamada.masahiro@xxxxxxxxxxxxx>
Signed-off-by: Matthias Maennich <maennich@xxxxxxxxxx>
Signed-off-by: Jessica Yu <jeyu@xxxxxxxxxx>

Im well out of my depth here, calling the experts

I think what I want is EXPORT_SYMBOL_FROM_SECTION ??
am I just doing it wrong ?

Next message: Sasha Levin: "[PATCH AUTOSEL 5.10 06/11] chardev: Fix potential memory leak when cdev_add() failed"
Previous message: Sasha Levin: "[PATCH AUTOSEL 5.10 03/11] usb: gadget: aspeed: fix buffer overflow"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]