linux-kernel - Re: srcversion hash does not cover all the module's source/dependency files

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date: Wed, 14 Feb 2024 22:36:19 +0900
From: Masahiro Yamada <masahiroy@...nel.org>
To: Radek Krejci <radek.krejci@...cle.com>
Cc: linux-kbuild@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: srcversion hash does not cover all the module's source/dependency files

On Wed, Feb 14, 2024 at 6:43 PM Radek Krejci <radek.krejci@...cle.com> wrote:
>
> 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.


Thanks. You are right.


> 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.


Correct. They are different things.

Parsing the headers located in the same directory seems
to be a design.



> 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?


I think the attached patch is correct.
I will pick it up later.



One question is, is this feature still used?

I assume the answer is yes because you noticed this bug.
(but you are the first/only person who noticed it
in the past 3 years)






Thanks.




> 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/



--
Best Regards
Masahiro Yamada