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 for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CACZ9PQV0uQ+0Nzn10tCxyVo0_8qRFexh0iQLQErAkFc1T2WGHA@mail.gmail.com>
Date:	Thu, 18 Sep 2014 22:31:45 +0900
From:	Roman Peniaev <r.peniaev@...il.com>
To:	Oleg Nesterov <oleg@...hat.com>
Cc:	Ming Lei <ming.lei@...onical.com>,
	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [v2 PATCH 1/1] init: fix race between rootfs mount and firmware loading

On Thu, Sep 18, 2014 at 2:46 AM, Oleg Nesterov <oleg@...hat.com> wrote:
> On 09/17, Roman Pen wrote:
>>
>> +void wait_for_rootfs(void)
>> +{
>> +     /* Avoid waiting for ourselves */
>> +     if (is_global_init(current))
>> +             pr_warn("init: it is not a good idea to wait for the rootfs mount from the init task\n");
>> +     else
>> +             wait_event(rootfs_waitq, rootfs_mounted);
>> +}
>
> Well, this pr_warn() doesn't look very useful, how about
>
>         if (WARN_ON((is_global_init(current))))
>                 return;
>
> ? this will show the caller.

Ok, I will change.

>
>> +static inline void wake_up_rootfs_waiters(void)
>> +{
>> +     rootfs_mounted = true;
>> +     /* wake_up guarantees write memory barrier if and only if
>> +        there is a task to be woken up, it is not always true
>> +        for our case. */
>
> Yes, but this doesn't matter. wait_event() takes care,
>
>> +     smp_wmb();
>
> so please remove this wmb() and the comment.


I was confused by these lines in memory-barriers.txt document:

 (5) LOCK operations.

     This acts as a one-way permeable barrier.  It guarantees that all memory
     operations after the LOCK operation will appear to happen after the LOCK
     operation with respect to the other components of the system.

So it turns out to be, that without explicit barrier the order can be changed
and CONDITION set can be done _after_ list check.

But, it still can't cross UNLOCK border and will be between LOCK-UNLOCK.
And this is the key.

If my final understanding is right (please, correct me if I am still wrong),
following reordering can happen, but we are fine with it:

wake_up_rootfs                 wait_event

LOCK
check the list, but empty
set CONDITION <<< reordered
UNLOCK

                                          LOCK
                                          add to the list
                                          rmb            <<< now we
see CONDITION
                                          UNLOCK

                                          check CONDITION <<< it is
set, we are woken up


--
Roman
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ