From: Radek Krejci <radek.krejci@oracle.com>
To: masahiroy@kernel.org, linux-kbuild@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Subject: srcversion hash does not cover all the module's source/dependency files
Date: Wed, 14 Feb 2024 10:42:55 +0100 [thread overview]
Message-ID: <08693d17-f2a1-4b5e-a136-81138ca3a58d@oracle.com> (raw)
[-- Attachment #1: Type: text/plain, Size: 2119 bytes --]
Hi,
I've found a bug in modpost - when it gets the list of source files to
generate srcversion hash, it skips all the source/dependency files
except for the first one.
There is patch [1] in v5.8rc1 replacing get_next_line() by get_line() in
parse_source_file() function. Besides other things, the difference
between those 2 functions is that get_next_line() trims the leading
spaces of the line being returned. The issue is, that the source (deps_
located at the same directory) file names in the list, being processed
in parse_source_file(), are indented. So, when the code gets to
"Terminate line at first space, to get rid of final ' \'", it
effectively hides the source file name from further processing, since
the first space is at the beginning of the line.
I checked this behavior with modpost from v5.4 and v5.14 (and confirmed
with the current master in git). In my case, my kernel module had just 2
source files - mymodule.c and mymodule.h (both located at the same
directory). With modpost from v5.4, the code change in any of the files
was reflected in srcversion hash. But with modpost from v5.14 (and
master) there is no hash change when the code change appears in the
header file, which is listed at the end of the deps_ list. I believe
this is quite simple to reproduce with any module, but if needed, I can
prepare some example code to reproduce the issue.
I noticed this [2] email thread in the list. It mentions a similar
issue. However, since it happened a half year before the change [1] was
introduced and because I was unable to find any further details,
including the promised patch, I believe that these 2 things are unrelated.
The enclosed patch worked for me, but there might be some other
consequences that I've missed, so feel free to modify it on your own or
let me know.
Is there anything else I can do to help fixing this issue?
Regards,
Radek Krejci
[1] -
https://lore.kernel.org/linux-kbuild/20200601055731.3006266-26-masahiroy@kernel.org/
[2] -
https://lore.kernel.org/linux-kbuild/CAN19L9G-mFN-MTmw0FS3ZX4d1MjeoL2U+s-Fk7Qw9UYWn5Q1YA@mail.gmail.com/
[-- Attachment #2: 0001-modpost-trim-leading-spaces-when-processing-source-f.patch --]
[-- Type: text/x-patch, Size: 1123 bytes --]
From 8e14b8fdb71385ab53af48fc56d9cd9e323e1265 Mon Sep 17 00:00:00 2001
From: Radek Krejci <radek.krejci@oracle.com>
Date: Wed, 14 Feb 2024 10:14:07 +0100
Subject: [PATCH] modpost: trim leading spaces when processing source files
list
get_line() does not trim the leading spaces, but the
parse_source_files() expects to get lines with source files paths where
the first space occurs after the file path.
Signed-off-by: Radek Krejci <radek.krejci@oracle.com>
---
scripts/mod/sumversion.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/scripts/mod/sumversion.c b/scripts/mod/sumversion.c
index 31066bfdba04..dc4878502276 100644
--- a/scripts/mod/sumversion.c
+++ b/scripts/mod/sumversion.c
@@ -326,7 +326,12 @@ static int parse_source_files(const char *objfile, struct md4_ctx *md)
/* Sum all files in the same dir or subdirs. */
while ((line = get_line(&pos))) {
- char* p = line;
+ char* p;
+
+ /* trim the leading spaces away */
+ while (isspace(*line))
+ line++;
+ p = line;
if (strncmp(line, "source_", sizeof("source_")-1) == 0) {
p = strrchr(line, ' ');
--
2.43.0
next reply other threads:[~2024-02-14 9:43 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-02-14 9:42 Radek Krejci [this message]
2024-02-14 13:36 ` srcversion hash does not cover all the module's source/dependency files Masahiro Yamada
2024-02-14 13:47 ` [External] : " Radek Krejci
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=08693d17-f2a1-4b5e-a136-81138ca3a58d@oracle.com \
--to=radek.krejci@oracle.com \
--cc=linux-kbuild@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=masahiroy@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).