From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.3 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 5CBF8C0044D for ; Tue, 17 Mar 2020 01:25:19 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 3AB642051A for ; Tue, 17 Mar 2020 01:25:19 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1733165AbgCQBZR (ORCPT ); Mon, 16 Mar 2020 21:25:17 -0400 Received: from ale.deltatee.com ([207.54.116.67]:55060 "EHLO ale.deltatee.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1733017AbgCQBZQ (ORCPT ); Mon, 16 Mar 2020 21:25:16 -0400 Received: from guinness.priv.deltatee.com ([172.16.1.162]) by ale.deltatee.com with esmtp (Exim 4.92) (envelope-from ) id 1jE0yo-0008I8-Hx; Mon, 16 Mar 2020 19:25:15 -0600 To: Thomas Gleixner , linux-kernel@vger.kernel.org, linux-pci@vger.kernel.org, Bjorn Helgaas Cc: Sebastian Andrzej Siewior References: <20200313183608.2646-1-logang@deltatee.com> <87mu8fdck6.fsf@nanos.tec.linutronix.de> From: Logan Gunthorpe Message-ID: <38781f88-32df-e89a-7d00-fd2fcc097497@deltatee.com> Date: Mon, 16 Mar 2020 19:25:14 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.9.0 MIME-Version: 1.0 In-Reply-To: <87mu8fdck6.fsf@nanos.tec.linutronix.de> Content-Type: text/plain; charset=utf-8 Content-Language: en-CA Content-Transfer-Encoding: 7bit X-SA-Exim-Connect-IP: 172.16.1.162 X-SA-Exim-Rcpt-To: bigeasy@linutronix.de, bhelgaas@google.com, linux-pci@vger.kernel.org, linux-kernel@vger.kernel.org, tglx@linutronix.de X-SA-Exim-Mail-From: logang@deltatee.com Subject: Re: [PATCH] PCI/switchtec: Fix init_completion race condition with poll_wait() X-SA-Exim-Version: 4.2.1 (built Wed, 08 May 2019 21:11:16 +0000) X-SA-Exim-Scanned: Yes (on ale.deltatee.com) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2020-03-16 6:56 p.m., Thomas Gleixner wrote: > Logan, > > Logan Gunthorpe writes: > >> The call to init_completion() in mrpc_queue_cmd() can theoretically >> race with the call to poll_wait() in switchtec_dev_poll(). >> >> poll() write() >> switchtec_dev_poll() switchtec_dev_write() >> poll_wait(&s->comp.wait); mrpc_queue_cmd() >> init_completion(&s->comp) >> init_waitqueue_head(&s->comp.wait) > > just a nitpick. As you took the liberty to copy the description of the > race, which was btw. disovered by me, verbatim from a changelog written > by someone else w/o providing the courtesy of linking to that original > analysis, allow me the liberty to add the missing link: > > Link: https://lore.kernel.org/lkml/20200313174701.148376-4-bigeasy@linutronix.de Well, I just copied the call chain. I had no way to know you were the one who discovered the bug given the way it was presented to me. And the original patch didn't include much in the way of analysis of the bug, just "It's Racy". I didn't deliberately omit the link, it just never occurred to me to add it. In retrospect, I should have included it, sorry about that. >> To my knowledge, no one has hit this bug, but we should fix it for >> correctness. > > s/,but we should fix/.Fix/ ? Yes, that's an improvement. >> Fix this by using reinit_completion() instead of init_completion() in >> mrpc_queue_cmd(). >> >> Fixes: 080b47def5e5 ("MicroSemi Switchtec management interface driver") >> Reported-by: Sebastian Andrzej Siewior >> Signed-off-by: Logan Gunthorpe > > Acked-by: Thomas Gleixner Thanks. > @Bjorn: Can you please hold off on this for a few days until we sorted > out the remaining issues to avoid potential merge conflicts > vs. the completion series? I'd suggest simply rebasing the completion patch on this patch, or a patch like it. Then we'll have the proper bug fix commit and there won't be a conflict. Logan