From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <stable-owner@vger.kernel.org>
Received: from wout2-smtp.messagingengine.com ([64.147.123.25]:51785 "EHLO
        wout2-smtp.messagingengine.com" rhost-flags-OK-OK-OK-OK)
        by vger.kernel.org with ESMTP id S1727738AbeK1WBT (ORCPT
        <rfc822;stable@vger.kernel.org>); Wed, 28 Nov 2018 17:01:19 -0500
Date: Wed, 28 Nov 2018 12:00:00 +0100
From: Greg KH <greg@kroah.com>
To: "Michael J. Ruhl" <michael.j.ruhl@intel.com>
Cc: stable@vger.kernel.org
Subject: Re: [PATCH] IB/hfi1: Eliminate races in the SDMA send error path
Message-ID: <20181128110000.GE1822@kroah.com>
References: <20181030163113.26452.83360.stgit@phstlrhel7.ph.intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20181030163113.26452.83360.stgit@phstlrhel7.ph.intel.com>
Sender: stable-owner@vger.kernel.org
List-ID: <stable.vger.kernel.org>

On Tue, Oct 30, 2018 at 12:31:13PM -0400, Michael J. Ruhl wrote:
> From: Michael J. Ruhl <michael.j.ruhl@intel.com>
> 
> commit a0e0cb82804a6a21d9067022c2dfdf80d11da429 upstream
> 
> pq_update() can only be called in two places: from the completion
> function when the complete (npkts) sequence of packets has been
> submitted and processed, or from setup function if a subset of the
> packets were submitted (i.e. the error path).
> 
> Currently both paths can call pq_update() if an error occurrs.  This
> race will cause the n_req value to go negative, hanging file_close(),
> or cause a crash by freeing the txlist more than once.
> 
> Several variables are used to determine SDMA send state.  Most of
> these are unnecessary, and have code inspectible races between the
> setup function and the completion function, in both the send path and
> the error path.
> 
> The request 'status' value can be set by the setup or by the
> completion function.  This is code inspectibly racy.  Since the status
> is not needed in the completion code or by the caller it has been
> removed.
> 
> The request 'done' value races between usage by the setup and the
> completion function.  The completion function does not need this.
> When the number of processed packets matches npkts, it is done.
> 
> The 'has_error' value races between usage of the setup and the
> completion function.  This can cause incorrect error handling and leave
> the n_req in an incorrect value (i.e. negative).
> 
> Simplify the code by removing all of the unneeded state checks and
> variables.
> 
> Clean up iovs node when it is freed.
> 
> Eliminate race conditions in the error path:
> 
> If all packets are submitted, the completion handler will set the
> completion status correctly (ok or aborted).
> 
> If all packets are not submitted, the caller must wait until the
> submitted packets have completed, and then set the completion status.
> 
> These two change eliminate the race condition in the error path.
> 
> Cc: <stable@vger.kernel.org> # 4.9.0

Now applied, thanks.

greg k-h