All of lore.kernel.org
 help / color / mirror / Atom feed
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
Date: Wed, 28 Nov 2018 11:59:49 +0100	[thread overview]
Message-ID: <20181128105949.GD1822@kroah.com> (raw)
In-Reply-To: <20181030161732.25095.53283.stgit@phstlrhel7.ph.intel.com>

On Tue, Oct 30, 2018 at 12:17:32PM -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.18.x+

Now applied, thanks.

greg k-h

  reply	other threads:[~2018-11-28 22:01 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-30 16:17 [PATCH] IB/hfi1: Eliminate races in the SDMA send error path Michael J. Ruhl
2018-11-28 10:59 ` Greg KH [this message]
2018-10-30 16:31 Michael J. Ruhl
2018-11-28 11:00 ` Greg KH
2018-10-31 14:48 Michael J. Ruhl
2018-11-28 11:00 ` Greg KH

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181128105949.GD1822@kroah.com \
    --to=greg@kroah.com \
    --cc=michael.j.ruhl@intel.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.