linux-kernel - Re: [PATCH 1/3] mailbox: always wait in mbox_send

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]

Message-ID: <CABb+yY0=j=BwbOzVtjYEMSTL0bCjhFNTw0ak5U2b+q4W4dYdrw@mail.gmail.com>
Date:   Wed, 29 Mar 2017 23:16:08 +0530
From:   Jassi Brar <jassisinghbrar@...il.com>
To:     Sudeep Holla <sudeep.holla@....com>
Cc:     Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Jassi Brar <jaswinder.singh@...aro.org>,
        Alexey Klimov <alexey.klimov@....com>
Subject: Re: [PATCH 1/3] mailbox: always wait in mbox_send_message for
 blocking Tx mode

On Wed, Mar 29, 2017 at 5:04 PM, Sudeep Holla <sudeep.holla@....com> wrote:
>
> On 28/03/17 19:20, Jassi Brar wrote:
>> Hi Sudeep,
>>
>> On Tue, Mar 21, 2017 at 5:00 PM, Sudeep Holla <sudeep.holla@....com> wrote:
>>> There exists a race when msg_submit return immediately as there was an
>>> active request being processed which may have completed just before it's
>>> checked again in mbox_send_message. This will result in return to the
>>> caller without waiting in mbox_send_message even when it's blocking Tx.
>>>
>>> This patch fixes the issue by waiting for the completion always if Tx
>>> is in blocking mode.
>>>
>>> Fixes: 2b6d83e2b8b7 ("mailbox: Introduce framework for mailbox")
>>> Cc: Jassi Brar <jassisinghbrar@...il.com>
>>> Reported-by: Alexey Klimov <alexey.klimov@....com>
>>> Signed-off-by: Sudeep Holla <sudeep.holla@....com>
>>> ---
>>>  drivers/mailbox/mailbox.c | 2 +-
>>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> Hi Jassi,
>>>
>>> Here are fixes for few issues we encountered when dealing with multiple
>>> requests on multiple channels simultaneously.
>>>
>> Thanks for finding the bug.
>>
>> I see patch-1 tries to fix the bug.  Patch-2,3 try to fix the
>> ramifications of the bug but they may change behaviour for some users.
>> Do you face any issue even after applying patch-1 ?
>>
>
> Unfortunately yes. Are you concerned with the change in return value on
> timeout ? I understand and then I chose -ETIME vs -ETIMEDOUT as hardware
> can still use it and we can distinguish the software timer expiry from
> that. Even -EIO seems incorrect for s/w timeout as it exists today, but
> I agree it has some impact on existing users.
>
> Also Patch 3 seems independent for me just to avoid complete call if it
> was empty message.
>
> Alexey also brought up another issue which is relating to ordering and
> may require completion flags per message instead of per channel. Today
> we can't guarantee that first blocker on the wait queue is same as the
> first in the mailbox queue.
>
> e.g.:
>         Thread#1(T1)               Thread#2(T2)
>      mbox_send_message           mbox_send_message
>             |                           |
>             V                           |
>         add_to_rbuf(M1)                 V
>             |                     add_to_rbuf(M2)
>             |                           |
>             |                           V
>             V                      msg_submit(picks M1)
>         msg_submit                      |
>             |                           V
>             V                   wait_for_completion(on M2)
>      wait_for_completion(on M1)         |  (1st in waitQ)
>             |   (2nd in waitQ)          V
>             V                   wake_up(on completion of M1)<--incorrect
>
Yes, that is a possibility. I have sent a fix for this. It would help
if Alexey/you could give it a try.

Thanks