On Thu, Mar 12, 2020 at 2:02 AM Jessica Yu jeyu@kernel.org wrote:
In order to preserve backwards compatability with kmod tools, we have to move the namespace field in Module.symvers last, as the depmod -e -E option looks at the first three fields in Module.symvers to check symbol versions (and it's expected they stay in the original order of crc, symbol, module).
In addition, update an ancient comment above read_dump() in modpost that suggested that the export type field in Module.symvers was optional. I suspect that there were historical reasons behind that comment that are no longer accurate. We have been unconditionally printing the export type since 2.6.18 (commit bd5cbcedf44), which is over a decade ago now.
Fix up read_dump() to treat each field as non-optional. I suspect the original read_dump() code treated the export field as optional in order to support pre <= 2.6.18 Module.symvers (which did not have the export type field). Note that although symbol namespaces are optional, the field will not be omitted from Module.symvers if a symbol does not have a namespace. In this case, the field will simply be empty and the next delimiter or end of line will follow.
Cc: stable@vger.kernel.org Fixes: cb9b55d21fe0 ("modpost: add support for symbol namespaces") Tested-by: Matthias Maennich maennich@google.com Reviewed-by: Matthias Maennich maennich@google.com Reviewed-by: Lucas De Marchi lucas.demarchi@intel.com Signed-off-by: Jessica Yu jeyu@kernel.org
While I am not opposed to this fix, I did not even notice Module.symvers was official interface.
Kbuild invokes scripts/depmod.sh to finalize the 'make modules_install', but I do not see the -E option being used there.
I do not see Module.symvers installed in /lib/modules/$(uname -r)/.
v2:
- Explain the changes to read_dump() and the comment (and provide historical context) in the commit message. (Lucas De Marchi)
Documentation/kbuild/modules.rst | 4 ++-- scripts/export_report.pl | 2 +- scripts/mod/modpost.c | 24 ++++++++++++------------ 3 files changed, 15 insertions(+), 15 deletions(-)
diff --git a/Documentation/kbuild/modules.rst b/Documentation/kbuild/modules.rst index 69fa48ee93d6..e0b45a257f21 100644 --- a/Documentation/kbuild/modules.rst +++ b/Documentation/kbuild/modules.rst @@ -470,9 +470,9 @@ build.
The syntax of the Module.symvers file is::
<CRC> <Symbol> <Namespace> <Module> <Export Type>
<CRC> <Symbol> <Module> <Export Type> <Namespace>
0xe1cc2a05 usb_stor_suspend USB_STORAGE drivers/usb/storage/usb-storage EXPORT_SYMBOL_GPL
0xe1cc2a05 usb_stor_suspend drivers/usb/storage/usb-storage EXPORT_SYMBOL_GPL USB_STORAGE The fields are separated by tabs and values may be empty (e.g. if no namespace is defined for an exported symbol).diff --git a/scripts/export_report.pl b/scripts/export_report.pl index 548330e8c4e7..feb3d5542a62 100755 --- a/scripts/export_report.pl +++ b/scripts/export_report.pl @@ -94,7 +94,7 @@ if (defined $opt{'o'}) { # while ( <$module_symvers> ) { chomp;
my (undef, $symbol, $namespace, $module, $gpl) = split('\t');
my (undef, $symbol, $module, $gpl, $namespace) = split('\t'); $SYMBOL { $symbol } = [ $module , "0" , $symbol, $gpl];} close($module_symvers); diff --git a/scripts/mod/modpost.c b/scripts/mod/modpost.c index a3d8370f9544..e1963ef8c07c 100644 --- a/scripts/mod/modpost.c +++ b/scripts/mod/modpost.c @@ -2421,7 +2421,7 @@ static void write_if_changed(struct buffer *b, const char *fname) }
/* parse Module.symvers file. line format:
- 0x12345678<tab>symbol<tab>module[[<tab>export]<tab>something]
**/
- 0x12345678<tab>symbol<tab>module<tab>export<tab>namespace
static void read_dump(const char *fname, unsigned int kernel) { @@ -2434,7 +2434,7 @@ static void read_dump(const char *fname, unsigned int kernel) return;
while ((line = get_next_line(&pos, file, size))) {
char *symname, *namespace, *modname, *d, *export, *end;
char *symname, *namespace, *modname, *d, *export; unsigned int crc; struct module *mod; struct symbol *s;@@ -2442,16 +2442,16 @@ static void read_dump(const char *fname, unsigned int kernel) if (!(symname = strchr(line, '\t'))) goto fail; *symname++ = '\0';
if (!(namespace = strchr(symname, '\t')))goto fail;*namespace++ = '\0';if (!(modname = strchr(namespace, '\t')))
if (!(modname = strchr(symname, '\t'))) goto fail; *modname++ = '\0';
if ((export = strchr(modname, '\t')) != NULL)*export++ = '\0';if (export && ((end = strchr(export, '\t')) != NULL))*end = '\0';
if (!(export = strchr(modname, '\t')))goto fail;*export++ = '\0';if (!(namespace = strchr(export, '\t')))goto fail;*namespace++ = '\0';crc = strtoul(line, &d, 16); if (*symname == '\0' || *modname == '\0' || *d != '\0') goto fail;@@ -2502,9 +2502,9 @@ static void write_dump(const char *fname) namespace = symbol->namespace; buf_printf(&buf, "0x%08x\t%s\t%s\t%s\t%s\n", symbol->crc, symbol->name,
namespace ? namespace : "", symbol->module->name,export_str(symbol->export));
export_str(symbol->export),namespace ? namespace : ""); } symbol = symbol->next; }-- 2.16.4