Hi Folks,
Here are a handful of patches we'd like to target for the RC. These are bug fixes, except one may be a little iffy (Add wc_flags and wc_immdata...) but it's a very trivial patch and really helps with debugging.
---
Andrzej Witkowski (1): IB/hfi1: Use new API to deallocate vnic rdma-netdev in hfi1 driver
Brian Welty (1): IB/{hfi1,qib}: Fix WC.byte_len calculation for UD_SEND_WITH_IMM
Michael J. Ruhl (3): IB/{hfi1,qib,rdmavt}: Do not depend on IB Verbs name for driver logging IB/hfi1: Close race condition on user context disable and close IB/hfi1: Remove overly conservative VM_EXEC flag check
Mike Marciniszyn (2): IB/rdmavt: Add wc_flags and wc_immdata to cq entry trace IB/hfi1: Add limit test for RC/UC send via loopback
drivers/infiniband/hw/hfi1/affinity.c | 6 ++- drivers/infiniband/hw/hfi1/driver.c | 8 ++++ drivers/infiniband/hw/hfi1/file_ops.c | 4 ++ drivers/infiniband/hw/hfi1/hfi.h | 25 +++++++------- drivers/infiniband/hw/hfi1/init.c | 10 +++--- drivers/infiniband/hw/hfi1/ud.c | 1 - drivers/infiniband/hw/hfi1/verbs.c | 1 + drivers/infiniband/hw/hfi1/vnic_main.c | 4 +- drivers/infiniband/hw/qib/qib.h | 12 +++---- drivers/infiniband/hw/qib/qib_driver.c | 8 ++++ drivers/infiniband/hw/qib/qib_init.c | 10 ++++-- drivers/infiniband/hw/qib/qib_ud.c | 1 - drivers/infiniband/hw/qib/qib_verbs.c | 1 + drivers/infiniband/sw/rdmavt/qp.c | 7 +++- drivers/infiniband/sw/rdmavt/trace.h | 6 ++- drivers/infiniband/sw/rdmavt/trace_cq.h | 10 ++++-- drivers/infiniband/sw/rdmavt/vt.c | 2 + drivers/infiniband/sw/rdmavt/vt.h | 10 ++++-- drivers/infiniband/ulp/opa_vnic/opa_vnic_netdev.c | 5 +-- include/rdma/ib_verbs.h | 7 ---- include/rdma/rdma_vt.h | 38 ++++----------------- 21 files changed, 92 insertions(+), 84 deletions(-)
-- -Denny
From: Michael J. Ruhl michael.j.ruhl@intel.com
When disabling and removing a receive context, it is possible for an asynchronous event (i.e IRQ) to occur. Because of this there is a race between cleaning up the context, and the context being used by the asynchronous event.
cpu 0 (context cleanup) rc->ref_count-- (ref_count == 0) hfi1_rcd_free() cpu 1 (IRQ (with rcd index)) rcd_get_by_index() lock ref_count+++ <-- reference count race (WARNING) return rcd unlock cpu 0 hfi1_free_ctxtdata() <-- incorrect free location lock remove rcd from array unlock free rcd
This race will cause the following WARNING trace:
WARNING: CPU: 0 PID: 175027 at include/linux/kref.h:52 hfi1_rcd_get_by_index+0x84/0xa0 [hfi1] CPU: 0 PID: 175027 Comm: IMB-MPI1 Kdump: loaded Tainted: G OE ------------ 3.10.0-957.el7.x86_64 #1 Hardware name: Intel Corporation S2600KP/S2600KP, BIOS SE5C610.86B.11.01.0076.C4.111920150602 11/19/2015 Call Trace: dump_stack+0x19/0x1b __warn+0xd8/0x100 warn_slowpath_null+0x1d/0x20 hfi1_rcd_get_by_index+0x84/0xa0 [hfi1] is_rcv_urgent_int+0x24/0x90 [hfi1] general_interrupt+0x1b6/0x210 [hfi1] __handle_irq_event_percpu+0x44/0x1c0 handle_irq_event_percpu+0x32/0x80 handle_irq_event+0x3c/0x60 handle_edge_irq+0x7f/0x150 handle_irq+0xe4/0x1a0 do_IRQ+0x4d/0xf0 common_interrupt+0x162/0x162
The race can also lead to a use after free which could be similar to:
general protection fault: 0000 1 SMP CPU: 71 PID: 177147 Comm: IMB-MPI1 Kdump: loaded Tainted: G W OE ------------ 3.10.0-957.el7.x86_64 #1 Hardware name: Intel Corporation S2600KP/S2600KP, BIOS SE5C610.86B.11.01.0076.C4.111920150602 11/19/2015 task: ffff9962a8098000 ti: ffff99717a508000 task.ti: ffff99717a508000 __kmalloc+0x94/0x230 Call Trace: ? hfi1_user_sdma_process_request+0x9c8/0x1250 [hfi1] hfi1_user_sdma_process_request+0x9c8/0x1250 [hfi1] hfi1_aio_write+0xba/0x110 [hfi1] do_sync_readv_writev+0x7b/0xd0 do_readv_writev+0xce/0x260 ? handle_mm_fault+0x39d/0x9b0 ? pick_next_task_fair+0x5f/0x1b0 ? sched_clock_cpu+0x85/0xc0 ? __schedule+0x13a/0x890 vfs_writev+0x35/0x60 SyS_writev+0x7f/0x110 system_call_fastpath+0x22/0x27
Introduce a context enable flag to indicate if the context is enabled.
Reorder context cleanup to ensure context removal before cleanup occurs correctly.
Cc: stable@vger.kernel.org # v4.14.0+ Fixes: f683c80ca68e ("IB/hfi1: Resolve kernel panics by reference counting receive contexts") Reviewed-by: Mike Marciniszyn mike.marciniszyn@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/file_ops.c | 2 ++ drivers/infiniband/hw/hfi1/hfi.h | 3 ++- drivers/infiniband/hw/hfi1/init.c | 8 +++++--- 3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/infiniband/hw/hfi1/file_ops.c b/drivers/infiniband/hw/hfi1/file_ops.c index c22ebc7..0ba0cf5 100644 --- a/drivers/infiniband/hw/hfi1/file_ops.c +++ b/drivers/infiniband/hw/hfi1/file_ops.c @@ -671,6 +671,8 @@ static int hfi1_file_close(struct inode *inode, struct file *fp) } spin_unlock_irqrestore(&dd->uctxt_lock, flags);
+ uctxt->del_pend = 1; + /* * Disable receive context and interrupt available, reset all * RcvCtxtCtrl bits to default values. diff --git a/drivers/infiniband/hw/hfi1/hfi.h b/drivers/infiniband/hw/hfi1/hfi.h index dc8ba43..ea99768 100644 --- a/drivers/infiniband/hw/hfi1/hfi.h +++ b/drivers/infiniband/hw/hfi1/hfi.h @@ -284,7 +284,8 @@ struct hfi1_ctxtdata { u16 expected_base; /* Device context index */ u8 ctxt; - + /* ctxt deletion is pending */ + u8 del_pend; /* PSM Specific fields */ /* lock protecting all Expected TID data */ struct mutex exp_mutex; diff --git a/drivers/infiniband/hw/hfi1/init.c b/drivers/infiniband/hw/hfi1/init.c index 044d9a8..d2ac3ef 100644 --- a/drivers/infiniband/hw/hfi1/init.c +++ b/drivers/infiniband/hw/hfi1/init.c @@ -215,12 +215,11 @@ static void hfi1_rcd_free(struct kref *kref) struct hfi1_ctxtdata *rcd = container_of(kref, struct hfi1_ctxtdata, kref);
- hfi1_free_ctxtdata(rcd->dd, rcd); - spin_lock_irqsave(&rcd->dd->uctxt_lock, flags); rcd->dd->rcd[rcd->ctxt] = NULL; spin_unlock_irqrestore(&rcd->dd->uctxt_lock, flags);
+ hfi1_free_ctxtdata(rcd->dd, rcd); kfree(rcd); }
@@ -326,7 +325,10 @@ struct hfi1_ctxtdata *hfi1_rcd_get_by_index(struct hfi1_devdata *dd, u16 ctxt) spin_lock_irqsave(&dd->uctxt_lock, flags); if (dd->rcd[ctxt]) { rcd = dd->rcd[ctxt]; - hfi1_rcd_get(rcd); + if (rcd->del_pend) + rcd = NULL; + else + hfi1_rcd_get(rcd); } spin_unlock_irqrestore(&dd->uctxt_lock, flags);
On Thu, Jan 17, 2019 at 12:41:54PM -0800, Dennis Dalessandro wrote:
From: Michael J. Ruhl michael.j.ruhl@intel.com
When disabling and removing a receive context, it is possible for an asynchronous event (i.e IRQ) to occur. Because of this there is a race between cleaning up the context, and the context being used by the asynchronous event.
cpu 0 (context cleanup) rc->ref_count-- (ref_count == 0) hfi1_rcd_free() cpu 1 (IRQ (with rcd index)) rcd_get_by_index() lock ref_count+++ <-- reference count race (WARNING) return rcd unlock cpu 0 hfi1_free_ctxtdata() <-- incorrect free location lock remove rcd from array unlock free rcd
This race will cause the following WARNING trace:
WARNING: CPU: 0 PID: 175027 at include/linux/kref.h:52 hfi1_rcd_get_by_index+0x84/0xa0 [hfi1] CPU: 0 PID: 175027 Comm: IMB-MPI1 Kdump: loaded Tainted: G OE ------------ 3.10.0-957.el7.x86_64 #1 Hardware name: Intel Corporation S2600KP/S2600KP, BIOS SE5C610.86B.11.01.0076.C4.111920150602 11/19/2015 Call Trace: dump_stack+0x19/0x1b __warn+0xd8/0x100 warn_slowpath_null+0x1d/0x20 hfi1_rcd_get_by_index+0x84/0xa0 [hfi1] is_rcv_urgent_int+0x24/0x90 [hfi1] general_interrupt+0x1b6/0x210 [hfi1] __handle_irq_event_percpu+0x44/0x1c0 handle_irq_event_percpu+0x32/0x80 handle_irq_event+0x3c/0x60 handle_edge_irq+0x7f/0x150 handle_irq+0xe4/0x1a0 do_IRQ+0x4d/0xf0 common_interrupt+0x162/0x162
The race can also lead to a use after free which could be similar to:
general protection fault: 0000 1 SMP CPU: 71 PID: 177147 Comm: IMB-MPI1 Kdump: loaded Tainted: G W OE ------------ 3.10.0-957.el7.x86_64 #1 Hardware name: Intel Corporation S2600KP/S2600KP, BIOS SE5C610.86B.11.01.0076.C4.111920150602 11/19/2015 task: ffff9962a8098000 ti: ffff99717a508000 task.ti: ffff99717a508000 __kmalloc+0x94/0x230 Call Trace: ? hfi1_user_sdma_process_request+0x9c8/0x1250 [hfi1] hfi1_user_sdma_process_request+0x9c8/0x1250 [hfi1] hfi1_aio_write+0xba/0x110 [hfi1] do_sync_readv_writev+0x7b/0xd0 do_readv_writev+0xce/0x260 ? handle_mm_fault+0x39d/0x9b0 ? pick_next_task_fair+0x5f/0x1b0 ? sched_clock_cpu+0x85/0xc0 ? __schedule+0x13a/0x890 vfs_writev+0x35/0x60 SyS_writev+0x7f/0x110 system_call_fastpath+0x22/0x27
Introduce a context enable flag to indicate if the context is enabled.
Reorder context cleanup to ensure context removal before cleanup occurs correctly.
Cc: stable@vger.kernel.org # v4.14.0+ Fixes: f683c80ca68e ("IB/hfi1: Resolve kernel panics by reference counting receive contexts") Reviewed-by: Mike Marciniszyn mike.marciniszyn@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/file_ops.c | 2 ++ drivers/infiniband/hw/hfi1/hfi.h | 3 ++- drivers/infiniband/hw/hfi1/init.c | 8 +++++--- 3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/infiniband/hw/hfi1/file_ops.c b/drivers/infiniband/hw/hfi1/file_ops.c index c22ebc7..0ba0cf5 100644 +++ b/drivers/infiniband/hw/hfi1/file_ops.c @@ -671,6 +671,8 @@ static int hfi1_file_close(struct inode *inode, struct file *fp) } spin_unlock_irqrestore(&dd->uctxt_lock, flags);
- uctxt->del_pend = 1;
Not going to get good results if you update shared data outside the lock that is reading it..
@@ -326,7 +325,10 @@ struct hfi1_ctxtdata *hfi1_rcd_get_by_index(struct hfi1_devdata *dd, u16 ctxt) spin_lock_irqsave(&dd->uctxt_lock, flags); if (dd->rcd[ctxt]) { rcd = dd->rcd[ctxt];
hfi1_rcd_get(rcd);
if (rcd->del_pend)rcd = NULL;elsehfi1_rcd_get(rcd);
This probably wants to be kref_get_not_zero() - why add a confusing del_pend?
Jason
-----Original Message----- From: Jason Gunthorpe [mailto:jgg@ziepe.ca] Sent: Thursday, January 17, 2019 4:12 PM To: Dalessandro, Dennis dennis.dalessandro@intel.com Cc: dledford@redhat.com; linux-rdma@vger.kernel.org; Ruhl, Michael J michael.j.ruhl@intel.com; Marciniszyn, Mike mike.marciniszyn@intel.com; stable@vger.kernel.org Subject: Re: [PATCH for-rc 5/7] IB/hfi1: Close race condition on user context disable and close
On Thu, Jan 17, 2019 at 12:41:54PM -0800, Dennis Dalessandro wrote:
From: Michael J. Ruhl michael.j.ruhl@intel.com
When disabling and removing a receive context, it is possible for an asynchronous event (i.e IRQ) to occur. Because of this there is a race between cleaning up the context, and the context being used by the asynchronous event.
cpu 0 (context cleanup) rc->ref_count-- (ref_count == 0) hfi1_rcd_free() cpu 1 (IRQ (with rcd index)) rcd_get_by_index() lock ref_count+++ <-- reference count race (WARNING) return rcd unlock cpu 0 hfi1_free_ctxtdata() <-- incorrect free location lock remove rcd from array unlock free rcd
This race will cause the following WARNING trace:
WARNING: CPU: 0 PID: 175027 at include/linux/kref.h:52
hfi1_rcd_get_by_index+0x84/0xa0 [hfi1]
CPU: 0 PID: 175027 Comm: IMB-MPI1 Kdump: loaded Tainted: G OE -----------
- 3.10.0-957.el7.x86_64 #1
Hardware name: Intel Corporation S2600KP/S2600KP, BIOS
SE5C610.86B.11.01.0076.C4.111920150602 11/19/2015
Call Trace: dump_stack+0x19/0x1b __warn+0xd8/0x100 warn_slowpath_null+0x1d/0x20 hfi1_rcd_get_by_index+0x84/0xa0 [hfi1] is_rcv_urgent_int+0x24/0x90 [hfi1] general_interrupt+0x1b6/0x210 [hfi1] __handle_irq_event_percpu+0x44/0x1c0 handle_irq_event_percpu+0x32/0x80 handle_irq_event+0x3c/0x60 handle_edge_irq+0x7f/0x150 handle_irq+0xe4/0x1a0 do_IRQ+0x4d/0xf0 common_interrupt+0x162/0x162
The race can also lead to a use after free which could be similar to:
general protection fault: 0000 1 SMP CPU: 71 PID: 177147 Comm: IMB-MPI1 Kdump: loaded Tainted: G W OE ------
------ 3.10.0-957.el7.x86_64 #1
Hardware name: Intel Corporation S2600KP/S2600KP, BIOS
SE5C610.86B.11.01.0076.C4.111920150602 11/19/2015
task: ffff9962a8098000 ti: ffff99717a508000 task.ti: ffff99717a508000
__kmalloc+0x94/0x230
Call Trace: ? hfi1_user_sdma_process_request+0x9c8/0x1250 [hfi1] hfi1_user_sdma_process_request+0x9c8/0x1250 [hfi1] hfi1_aio_write+0xba/0x110 [hfi1] do_sync_readv_writev+0x7b/0xd0 do_readv_writev+0xce/0x260 ? handle_mm_fault+0x39d/0x9b0 ? pick_next_task_fair+0x5f/0x1b0 ? sched_clock_cpu+0x85/0xc0 ? __schedule+0x13a/0x890 vfs_writev+0x35/0x60 SyS_writev+0x7f/0x110 system_call_fastpath+0x22/0x27
Introduce a context enable flag to indicate if the context is enabled.
Reorder context cleanup to ensure context removal before cleanup occurs correctly.
Cc: stable@vger.kernel.org # v4.14.0+ Fixes: f683c80ca68e ("IB/hfi1: Resolve kernel panics by reference counting
receive contexts")
Reviewed-by: Mike Marciniszyn mike.marciniszyn@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/file_ops.c | 2 ++ drivers/infiniband/hw/hfi1/hfi.h | 3 ++- drivers/infiniband/hw/hfi1/init.c | 8 +++++--- 3 files changed, 9 insertions(+), 4 deletions(-)
diff --git a/drivers/infiniband/hw/hfi1/file_ops.c
b/drivers/infiniband/hw/hfi1/file_ops.c
index c22ebc7..0ba0cf5 100644 +++ b/drivers/infiniband/hw/hfi1/file_ops.c @@ -671,6 +671,8 @@ static int hfi1_file_close(struct inode *inode, struct
file *fp)
} spin_unlock_irqrestore(&dd->uctxt_lock, flags);
- uctxt->del_pend = 1;
Not going to get good results if you update shared data outside the lock that is reading it..
@@ -326,7 +325,10 @@ struct hfi1_ctxtdata *hfi1_rcd_get_by_index(struct
hfi1_devdata *dd, u16 ctxt)
spin_lock_irqsave(&dd->uctxt_lock, flags); if (dd->rcd[ctxt]) { rcd = dd->rcd[ctxt];
hfi1_rcd_get(rcd);
if (rcd->del_pend)rcd = NULL;elsehfi1_rcd_get(rcd);This probably wants to be kref_get_not_zero() - why add a confusing del_pend?
kref_get_unless_zero()?
/sigh. I missed that function completely.
I will examine this some more and work up a new patch.
M
Jason
From: Michael J. Ruhl michael.j.ruhl@intel.com
Applications that use the stack for execution purposes cause PSM jobs to fail during mmap().
Both Fortran (non-standard format parsing) and C (callback functions located in the stack) applications can be written such that stack execution is required.
Because of this the EXECSTACK bit can be automatically set at link time for any application.
On application load, the ELF loader evaluates the EXECSTACK bit for the application and it's linked libraries. It will set the process VM flags to allow the stack to include the VM_EXEC bit if the EXECSTACK bit is set. This flag is propagated to the driver during the mmap() call in the vma flag bits.
Checking for this bit and failing the request with EPERM is overly conservative and will break any PSM application that has the bit set.
Remove the VM_EXEC flag from the check.
Cc: stable@vger.kernel.org #v4.14+ Fixes: 12220267645c ("IB/hfi: Protect against writable mmap") Reviewed-by: Mike Marciniszyn mike.marciniszyn@intel.com Reviewed-by: Dennis Dalessandro dennis.dalessandro@intel.com Reviewed-by: Ira Weiny ira.weiny@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/file_ops.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/drivers/infiniband/hw/hfi1/file_ops.c b/drivers/infiniband/hw/hfi1/file_ops.c index 0ba0cf5..68c397a 100644 --- a/drivers/infiniband/hw/hfi1/file_ops.c +++ b/drivers/infiniband/hw/hfi1/file_ops.c @@ -488,7 +488,7 @@ static int hfi1_file_mmap(struct file *fp, struct vm_area_struct *vma) vmf = 1; break; case STATUS: - if (flags & (unsigned long)(VM_WRITE | VM_EXEC)) { + if (flags & VM_WRITE) { ret = -EPERM; goto done; }
On Thu, Jan 17, 2019 at 12:42:04PM -0800, Dennis Dalessandro wrote:
From: Michael J. Ruhl michael.j.ruhl@intel.com
Applications that use the stack for execution purposes cause PSM jobs to fail during mmap().
Both Fortran (non-standard format parsing) and C (callback functions located in the stack) applications can be written such that stack execution is required.
Because of this the EXECSTACK bit can be automatically set at link time for any application.
On application load, the ELF loader evaluates the EXECSTACK bit for the application and it's linked libraries. It will set the process VM flags to allow the stack to include the VM_EXEC bit if the EXECSTACK bit is set. This flag is propagated to the driver during the mmap() call in the vma flag bits.
Checking for this bit and failing the request with EPERM is overly conservative and will break any PSM application that has the bit set.
Remove the VM_EXEC flag from the check.
Cc: stable@vger.kernel.org #v4.14+ Fixes: 12220267645c ("IB/hfi: Protect against writable mmap") Reviewed-by: Mike Marciniszyn mike.marciniszyn@intel.com Reviewed-by: Dennis Dalessandro dennis.dalessandro@intel.com Reviewed-by: Ira Weiny ira.weiny@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/file_ops.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
applied to for-next
Thanks, Jason
On Fri, Jan 18, 2019 at 02:02:35PM -0700, Jason Gunthorpe wrote:
On Thu, Jan 17, 2019 at 12:42:04PM -0800, Dennis Dalessandro wrote:
From: Michael J. Ruhl michael.j.ruhl@intel.com
Applications that use the stack for execution purposes cause PSM jobs to fail during mmap().
Both Fortran (non-standard format parsing) and C (callback functions located in the stack) applications can be written such that stack execution is required.
Because of this the EXECSTACK bit can be automatically set at link time for any application.
On application load, the ELF loader evaluates the EXECSTACK bit for the application and it's linked libraries. It will set the process VM flags to allow the stack to include the VM_EXEC bit if the EXECSTACK bit is set. This flag is propagated to the driver during the mmap() call in the vma flag bits.
Checking for this bit and failing the request with EPERM is overly conservative and will break any PSM application that has the bit set.
Remove the VM_EXEC flag from the check.
Cc: stable@vger.kernel.org #v4.14+ Fixes: 12220267645c ("IB/hfi: Protect against writable mmap") Reviewed-by: Mike Marciniszyn mike.marciniszyn@intel.com Reviewed-by: Dennis Dalessandro dennis.dalessandro@intel.com Reviewed-by: Ira Weiny ira.weiny@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/file_ops.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
applied to for-next
sorry for-rc
Jason
From: Mike Marciniszyn mike.marciniszyn@intel.com
Fix potential memory corruption and panic in loopback for IB_WR_SEND variants.
The code blindly assumes the posted length will fit in the fetched rwqe, which is not a valid assumption.
Fix by adding a limit test, and triggering the appropriate send completion and putting the QP in an error state. This mimics the handling for non-loopback QPs.
Fixes: 15703461533a ("IB/{hfi1, qib, rdmavt}: Move ruc_loopback to rdmavt") Cc: stable@vger.kernel.org #v4.20+ Reviewed-by: Michael J. Ruhl michael.j.ruhl@intel.com Signed-off-by: Mike Marciniszyn mike.marciniszyn@intel.com Signed-off-by: Dennis Dalessandro dennis.dalessandro@intel.com --- drivers/infiniband/sw/rdmavt/qp.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-)
diff --git a/drivers/infiniband/sw/rdmavt/qp.c b/drivers/infiniband/sw/rdmavt/qp.c index a1bd8cf..c6cc3e4 100644 --- a/drivers/infiniband/sw/rdmavt/qp.c +++ b/drivers/infiniband/sw/rdmavt/qp.c @@ -2910,6 +2910,8 @@ void rvt_ruc_loopback(struct rvt_qp *sqp) goto op_err; if (!ret) goto rnr_nak; + if (wqe->length > qp->r_len) + goto inv_err; break;
case IB_WR_RDMA_WRITE_WITH_IMM: @@ -3078,7 +3080,10 @@ void rvt_ruc_loopback(struct rvt_qp *sqp) goto err;
inv_err: - send_status = IB_WC_REM_INV_REQ_ERR; + send_status = + sqp->ibqp.qp_type == IB_QPT_RC ? + IB_WC_REM_INV_REQ_ERR : + IB_WC_SUCCESS; wc.status = IB_WC_LOC_QP_OP_ERR; goto err;
On Thu, Jan 17, 2019 at 12:42:16PM -0800, Dennis Dalessandro wrote:
From: Mike Marciniszyn mike.marciniszyn@intel.com
Fix potential memory corruption and panic in loopback for IB_WR_SEND variants.
The code blindly assumes the posted length will fit in the fetched rwqe, which is not a valid assumption.
Fix by adding a limit test, and triggering the appropriate send completion and putting the QP in an error state. This mimics the handling for non-loopback QPs.
Fixes: 15703461533a ("IB/{hfi1, qib, rdmavt}: Move ruc_loopback to rdmavt") Cc: stable@vger.kernel.org #v4.20+ Reviewed-by: Michael J. Ruhl michael.j.ruhl@intel.com Signed-off-by: Mike Marciniszyn mike.marciniszyn@intel.com Signed-off-by: Dennis Dalessandro dennis.dalessandro@intel.com
drivers/infiniband/sw/rdmavt/qp.c | 7 ++++++- 1 files changed, 6 insertions(+), 1 deletions(-)
Applied to for-rc
Thanks, Jason
linux-stable-mirror@lists.linaro.org
-
Dennis Dalessandro -
Jason Gunthorpe -
Ruhl, Michael J