All of lore.kernel.org
 help / color / mirror / Atom feed
From: Doug Ledford <dledford@redhat.com>
To: Dennis Dalessandro <dennis.dalessandro@intel.com>,
	Jason Gunthorpe <jgg@ziepe.ca>
Cc: linux-rdma@vger.kernel.org,
	"Michael J. Ruhl" <michael.j.ruhl@intel.com>,
	Mike Marciniszyn <mike.marciniszyn@intel.com>,
	stable@vger.kernel.org
Subject: Re: [PATCH for-next 05/14] IB/hfi1: Use after free race condition in send context error path
Date: Wed, 09 May 2018 10:38:52 -0400	[thread overview]
Message-ID: <1525876732.11756.372.camel@redhat.com> (raw)
In-Reply-To: <add45f95-61e0-aa96-6738-08c15a234f33@intel.com>

[-- Attachment #1: Type: text/plain, Size: 2879 bytes --]

On Fri, 2018-05-04 at 16:01 -0400, Dennis Dalessandro wrote:
> On 5/4/2018 2:38 PM, Jason Gunthorpe wrote:
> > On Wed, May 02, 2018 at 06:42:51AM -0700, Dennis Dalessandro wrote:
> > > From: Michael J. Ruhl <michael.j.ruhl@intel.com>
> > > 
> > > A pio send egress error can occur when the PSM library attempts to
> > > to send a bad packet.  That issue is still being investigated.
> > > 
> > > The pio error interrupt handler then attempts to progress the recovery
> > > of the errored pio send context.
> > > 
> > > Code inspection reveals that the handling lacks the necessary locking
> > > if that recovery interleaves with a PSM close of the "context" object
> > > contains the pio send context.
> > > 
> > > The lack of the locking can cause the recovery to access the already
> > > freed pio send context object and incorrectly deduce that the pio
> > > send context is actually a kernel pio send context as shown by the
> > > NULL deref stack below:
> > > 
> > > [<ffffffff8143d78c>] _dev_info+0x6c/0x90
> > > [<ffffffffc0613230>] sc_restart+0x70/0x1f0 [hfi1]
> > > [<ffffffff816ab124>] ? __schedule+0x424/0x9b0
> > > [<ffffffffc06133c5>] sc_halted+0x15/0x20 [hfi1]
> > > [<ffffffff810aa3ba>] process_one_work+0x17a/0x440
> > > [<ffffffff810ab086>] worker_thread+0x126/0x3c0
> > > [<ffffffff810aaf60>] ? manage_workers.isra.24+0x2a0/0x2a0
> > > [<ffffffff810b252f>] kthread+0xcf/0xe0
> > > [<ffffffff810b2460>] ? insert_kthread_work+0x40/0x40
> > > [<ffffffff816b8798>] ret_from_fork+0x58/0x90
> > > [<ffffffff810b2460>] ? insert_kthread_work+0x40/0x40
> > > 
> > > This is the best case scenario and other scenarios can corrupt the
> > > already freed memory.
> > > 
> > > Fix by adding the necessary locking in the pio send context error
> > > handler.
> > > 
> > > Cc: <stable@vger.kernel.org> # 4.9.x
> > > Reviewed-by: Mike Marciniszyn <mike.marciniszyn@intel.com>
> > > Reviewed-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
> > > Signed-off-by: Michael J. Ruhl <michael.j.ruhl@intel.com>
> > > Signed-off-by: Dennis Dalessandro <dennis.dalessandro@intel.com>
> > > ---
> > >   drivers/infiniband/hw/hfi1/chip.c |    4 ++++
> > >   1 files changed, 4 insertions(+), 0 deletions(-)
> > 
> > Why are you sending this to for-next not for-rc?
> 
> I went back and forth on this one. In the end decided against it because 
> we've lived with it for so long, note stable tag is all the way back to 
> 4.9, that and the fact that it's extremely unlikely to occur. I would be 
> fine including it with the -rc though. I think a case could be made 
> either way.
> 
> -Denny
> 
> 
> 

I went ahead and pulled this into for-rc instead of for-next.  Thanks.

-- 
Doug Ledford <dledford@redhat.com>
    GPG KeyID: B826A3330E572FDD
    Key fingerprint = AE6B 1BDA 122B 23B4 265B  1274 B826 A333 0E57 2FDD

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

  reply	other threads:[~2018-05-09 14:38 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-05-02 13:42 [PATCH for-next 00/14] IB/hfi1: Updates for-next 5/2/2018 Dennis Dalessandro
2018-05-02 13:42 ` Dennis Dalessandro
2018-05-02 13:42 ` [PATCH for-next 04/14] IB/hfi1: Fix fault injection init/exit issues Dennis Dalessandro
2018-05-02 13:42 ` [PATCH for-next 05/14] IB/hfi1: Use after free race condition in send context error path Dennis Dalessandro
2018-05-04 18:38   ` Jason Gunthorpe
2018-05-04 20:01     ` Dennis Dalessandro
2018-05-09 14:38       ` Doug Ledford [this message]
2018-05-02 13:43 ` [PATCH for-next 07/14] IB/hfi1: Reorder incorrect send context disable Dennis Dalessandro
2018-05-02 13:43 ` [PATCH for-next 08/14] IB/{hfi1, qib}: Add handling of kernel restart Dennis Dalessandro
2018-05-02 13:43 ` [PATCH for-next 11/14] IB/hfi1: Optimize kthread pointer locking when queuing CQ entries Dennis Dalessandro
2018-05-15 14:35 ` [PATCH for-next 00/14] IB/hfi1: Updates for-next 5/2/2018 Doug Ledford

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=1525876732.11756.372.camel@redhat.com \
    --to=dledford@redhat.com \
    --cc=dennis.dalessandro@intel.com \
    --cc=jgg@ziepe.ca \
    --cc=linux-rdma@vger.kernel.org \
    --cc=michael.j.ruhl@intel.com \
    --cc=mike.marciniszyn@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.