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  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:   Tue, 12 Jan 2021 11:46:55 +0100
From:   Jessica Yu <jeyu@...nel.org>
To:     Adam Zabrocki <pi3@....com.pl>
Cc:     Nicolas Morey-Chaisemartin <nmoreychaisemartin@...e.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org, Solar Designer <solar@...nwall.com>
Subject: Re: Linux Kernel module notification bug

+++ Adam Zabrocki [12/01/21 01:15 +0100]:
>Hello,
>
>On Mon, Jan 11, 2021 at 03:20:48PM +0100, Jessica Yu wrote:
>> +++ Adam Zabrocki [10/01/21 18:54 +0100]:
>> > Hello,
>> >
>> > I believe that the following commit does introduce a gentle "functionality
>> > bug":
>> >
>> > "module: delay kobject uevent until after module init call":
>> > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/kernel/module.c?id=38dc717e97153e46375ee21797aa54777e5498f3
>> >
>> > The official Linux Kernel API for the kernel module activities notification has
>> > been divided based on the readiness 'stage' for such module. We have the
>> > following stages:
>> >
>> >        MODULE_STATE_LIVE,      /* Normal state. */
>> >        MODULE_STATE_COMING,    /* Full formed, running module_init. */
>> >        MODULE_STATE_GOING,     /* Going away. */
>> >        MODULE_STATE_UNFORMED,  /* Still setting it up. */
>> >
>> > LIVE means that the kernel module is correctly running and all initialization
>> > work has been already done. Otherwise, we have event 'COMING' or 'UNFORMED'.
>> >
>> > In the described commit, creation of the KOBJECT has been moved after invoking
>> > a notficiation of the newly formed kernel module state (LIVE). That's somehow
>> > inconsistent from my understanding of the kernel modules notifiers logic.
>> > Creation of the new objects (like KOBJ) should be done before notification of
>> > the stage LIVE is invoked.
>>
>> I'm confused. We're not creating any kobjects here. That is all done
>> in mod_sysfs_setup(), which is called while the module is still
>> COMING.  What that commit does is delay telling userspace about the
>> module (specifically, systemd/udev) until the module is basically
>> ready. Systemd was basically receiving the uevent too early, before
>> the module has initialized, hence we decided to delay the uevent [1].
>>
>
>Sorry for the confusion on my side. I was referring to the internal state of
>the KOBJ itself which is being actively modified when uevent is sent. During
>the module creation KOBJ_ADD modifies 'kobj->state_add_uevent_sent'. Until
>recent commit, kernel didn't modify KOBJ after sending LIVE notification.
>
>> > This commit breaks some of the projects that rely on the LIVE notification to
>> > monitor when the newly loaded module is ready.
>>
>> Hm, could you please explain specifically what is the issue you're seeing?
>> What projects is it breaking?
>>
>
>I'm specifically referencing these projects which are tracking kernel modules
>for integrity purpose (e.g. anti-rootkit tools) like Linux Kernel Runtime
>Guard.
>It is possible to modify these tools to adopt and take into account
>modification of KOBJ after LIVE state notification. However, from my
>understanding of the kernel modules notifiers logic, KOBJ should be fully
>formed at this stage.

Ah I see, thanks for the clarification. I was under the impression
that kobj->state_add_uevent_sent is an internal field interesting
only to lib/kobject.c, and did not know tools like you mention are
implicitly tracking that.

In any case, I think your suggested change doesn't affect the
systemd/udev issue we were trying to fix, as long as the uevent is
sent after module init(). Would you mind sending a patch? (with a
Fixes: tag and including the explanations you've provided in this
thread in the changelog)

Thanks!

Jessica





Powered by blists - more mailing lists