* [PATCH for-rc 0/2] IB/hfi1: Fixes for rc or next if too late @ 2018-05-31 18:29 ` Dennis Dalessandro 0 siblings, 0 replies; 8+ messages in thread From: Dennis Dalessandro @ 2018-05-31 18:29 UTC (permalink / raw) To: jgg, dledford Cc: linux-rdma, Michael J. Ruhl, Mike Marciniszyn, stable, Kaike Wan Hi Doug and Jason, We have two more late breaking fix up patches. The DMA_RTAIL fix is the more serious of the two. I realize we are at the tail end of 4.17 so I would not be against holding off till 4.18 for these, but if there is another rdma pull request we may want to tack these on. --- Kaike Wan (1): IB/hfi1: Ensure VL index is within bounds Mike Marciniszyn (1): IB/hfi1: Fix user context tail allocation for DMA_RTAIL drivers/infiniband/hw/hfi1/chip.c | 8 ++++---- drivers/infiniband/hw/hfi1/file_ops.c | 2 +- drivers/infiniband/hw/hfi1/init.c | 9 ++++----- drivers/infiniband/hw/hfi1/sdma.c | 12 +++--------- 4 files changed, 12 insertions(+), 19 deletions(-) ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH for-rc 0/2] IB/hfi1: Fixes for rc or next if too late @ 2018-05-31 18:29 ` Dennis Dalessandro 0 siblings, 0 replies; 8+ messages in thread From: Dennis Dalessandro @ 2018-05-31 18:29 UTC (permalink / raw) To: jgg, dledford Cc: linux-rdma, Michael J. Ruhl, Mike Marciniszyn, stable, Kaike Wan Hi Doug and Jason, We have two more late breaking fix up patches. The DMA_RTAIL fix is the more serious of the two. I realize we are at the tail end of 4.17 so I would not be against holding off till 4.18 for these, but if there is another rdma pull request we may want to tack these on. --- Kaike Wan (1): IB/hfi1: Ensure VL index is within bounds Mike Marciniszyn (1): IB/hfi1: Fix user context tail allocation for DMA_RTAIL drivers/infiniband/hw/hfi1/chip.c | 8 ++++---- drivers/infiniband/hw/hfi1/file_ops.c | 2 +- drivers/infiniband/hw/hfi1/init.c | 9 ++++----- drivers/infiniband/hw/hfi1/sdma.c | 12 +++--------- 4 files changed, 12 insertions(+), 19 deletions(-) -- -Denny ^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCH for-rc 1/2] IB/hfi1: Fix user context tail allocation for DMA_RTAIL 2018-05-31 18:29 ` Dennis Dalessandro (?) @ 2018-05-31 18:30 ` Dennis Dalessandro -1 siblings, 0 replies; 8+ messages in thread From: Dennis Dalessandro @ 2018-05-31 18:30 UTC (permalink / raw) To: jgg, dledford; +Cc: linux-rdma, Michael J. Ruhl, Mike Marciniszyn, stable From: Mike Marciniszyn <mike.marciniszyn@intel.com> The following code fails to allocate a buffer for the tail address that the hardware DMAs into when the user context DMA_RTAIL is set. if (HFI1_CAP_KGET_MASK(rcd->flags, DMA_RTAIL)) { rcd->rcvhdrtail_kvaddr = dma_zalloc_coherent( &dd->pcidev->dev, PAGE_SIZE, &dma_hdrqtail, gfp_flags); if (!rcd->rcvhdrtail_kvaddr) goto bail_free; rcd->rcvhdrqtailaddr_dma = dma_hdrqtail; } So the rcvhdrtail_kvaddr would then be NULL. The mmap logic fails to check for a NULL rcvhdrtail_kvaddr. The fix is to test for both user and kernel DMA_TAIL options during the allocation as well as testing for a NULL rcvhdrtail_kvaddr during the mmap processing. Additionally, all downstream testing of the capmask for DMA_RTAIL have been eliminated in favor of testing rcvhdrtail_kvaddr. Cc: <stable@vger.kernel.org> # 4.9.x 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/hw/hfi1/chip.c | 8 ++++---- drivers/infiniband/hw/hfi1/file_ops.c | 2 +- drivers/infiniband/hw/hfi1/init.c | 9 ++++----- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/drivers/infiniband/hw/hfi1/chip.c b/drivers/infiniband/hw/hfi1/chip.c index 9b1f97f..ccbdce2 100644 --- a/drivers/infiniband/hw/hfi1/chip.c +++ b/drivers/infiniband/hw/hfi1/chip.c @@ -6841,7 +6841,7 @@ static void rxe_kernel_unfreeze(struct hfi1_devdata *dd) } rcvmask = HFI1_RCVCTRL_CTXT_ENB; /* HFI1_RCVCTRL_TAILUPD_[ENB|DIS] needs to be set explicitly */ - rcvmask |= HFI1_CAP_KGET_MASK(rcd->flags, DMA_RTAIL) ? + rcvmask |= rcd->rcvhdrtail_kvaddr ? HFI1_RCVCTRL_TAILUPD_ENB : HFI1_RCVCTRL_TAILUPD_DIS; hfi1_rcvctrl(dd, rcvmask, rcd); hfi1_rcd_put(rcd); @@ -8367,7 +8367,7 @@ static inline int check_packet_present(struct hfi1_ctxtdata *rcd) u32 tail; int present; - if (!HFI1_CAP_IS_KSET(DMA_RTAIL)) + if (!rcd->rcvhdrtail_kvaddr) present = (rcd->seq_cnt == rhf_rcv_seq(rhf_to_cpu(get_rhf_addr(rcd)))); else /* is RDMA rtail */ @@ -11843,7 +11843,7 @@ void hfi1_rcvctrl(struct hfi1_devdata *dd, unsigned int op, /* reset the tail and hdr addresses, and sequence count */ write_kctxt_csr(dd, ctxt, RCV_HDR_ADDR, rcd->rcvhdrq_dma); - if (HFI1_CAP_KGET_MASK(rcd->flags, DMA_RTAIL)) + if (rcd->rcvhdrtail_kvaddr) write_kctxt_csr(dd, ctxt, RCV_HDR_TAIL_ADDR, rcd->rcvhdrqtailaddr_dma); rcd->seq_cnt = 1; @@ -11923,7 +11923,7 @@ void hfi1_rcvctrl(struct hfi1_devdata *dd, unsigned int op, rcvctrl |= RCV_CTXT_CTRL_INTR_AVAIL_SMASK; if (op & HFI1_RCVCTRL_INTRAVAIL_DIS) rcvctrl &= ~RCV_CTXT_CTRL_INTR_AVAIL_SMASK; - if (op & HFI1_RCVCTRL_TAILUPD_ENB && rcd->rcvhdrqtailaddr_dma) + if ((op & HFI1_RCVCTRL_TAILUPD_ENB) && rcd->rcvhdrtail_kvaddr) rcvctrl |= RCV_CTXT_CTRL_TAIL_UPD_SMASK; if (op & HFI1_RCVCTRL_TAILUPD_DIS) { /* See comment on RcvCtxtCtrl.TailUpd above */ diff --git a/drivers/infiniband/hw/hfi1/file_ops.c b/drivers/infiniband/hw/hfi1/file_ops.c index c9d23c3..0fc4aa9 100644 --- a/drivers/infiniband/hw/hfi1/file_ops.c +++ b/drivers/infiniband/hw/hfi1/file_ops.c @@ -505,7 +505,7 @@ static int hfi1_file_mmap(struct file *fp, struct vm_area_struct *vma) ret = -EINVAL; goto done; } - if (flags & VM_WRITE) { + if ((flags & VM_WRITE) || !uctxt->rcvhdrtail_kvaddr) { ret = -EPERM; goto done; } diff --git a/drivers/infiniband/hw/hfi1/init.c b/drivers/infiniband/hw/hfi1/init.c index 8265551..f2a0b03 100644 --- a/drivers/infiniband/hw/hfi1/init.c +++ b/drivers/infiniband/hw/hfi1/init.c @@ -1838,7 +1838,6 @@ int hfi1_create_rcvhdrq(struct hfi1_devdata *dd, struct hfi1_ctxtdata *rcd) u64 reg; if (!rcd->rcvhdrq) { - dma_addr_t dma_hdrqtail; gfp_t gfp_flags; /* @@ -1863,13 +1862,13 @@ int hfi1_create_rcvhdrq(struct hfi1_devdata *dd, struct hfi1_ctxtdata *rcd) goto bail; } - if (HFI1_CAP_KGET_MASK(rcd->flags, DMA_RTAIL)) { + if (HFI1_CAP_KGET_MASK(rcd->flags, DMA_RTAIL) || + HFI1_CAP_UGET_MASK(rcd->flags, DMA_RTAIL)) { rcd->rcvhdrtail_kvaddr = dma_zalloc_coherent( - &dd->pcidev->dev, PAGE_SIZE, &dma_hdrqtail, - gfp_flags); + &dd->pcidev->dev, PAGE_SIZE, + &rcd->rcvhdrqtailaddr_dma, gfp_flags); if (!rcd->rcvhdrtail_kvaddr) goto bail_free; - rcd->rcvhdrqtailaddr_dma = dma_hdrqtail; } rcd->rcvhdrq_size = amt; ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCH for-rc 0/2] IB/hfi1: Fixes for rc or next if too late 2018-05-31 18:29 ` Dennis Dalessandro (?) (?) @ 2018-05-31 18:47 ` Doug Ledford 2018-05-31 19:08 ` Dennis Dalessandro -1 siblings, 1 reply; 8+ messages in thread From: Doug Ledford @ 2018-05-31 18:47 UTC (permalink / raw) To: Dennis Dalessandro, jgg Cc: linux-rdma, Michael J. Ruhl, Mike Marciniszyn, stable, Kaike Wan [-- Attachment #1: Type: text/plain, Size: 1202 bytes --] On Thu, 2018-05-31 at 11:29 -0700, Dennis Dalessandro wrote: > Hi Doug and Jason, > > We have two more late breaking fix up patches. The DMA_RTAIL fix is the more > serious of the two. I realize we are at the tail end of 4.17 so I would not be > against holding off till 4.18 for these, but if there is another rdma > pull request we may want to tack these on. > > --- > > Kaike Wan (1): > IB/hfi1: Ensure VL index is within bounds > > Mike Marciniszyn (1): > IB/hfi1: Fix user context tail allocation for DMA_RTAIL > > > drivers/infiniband/hw/hfi1/chip.c | 8 ++++---- > drivers/infiniband/hw/hfi1/file_ops.c | 2 +- > drivers/infiniband/hw/hfi1/init.c | 9 ++++----- > drivers/infiniband/hw/hfi1/sdma.c | 12 +++--------- > 4 files changed, 12 insertions(+), 19 deletions(-) > > -- > -Denny Hi Denny, These two patches look fine in terms of the patches themselves. In terms of whether to put them in for-rc or for-next, what's the consequences of hitting each of these bugs? -- 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 --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH for-rc 0/2] IB/hfi1: Fixes for rc or next if too late 2018-05-31 18:47 ` [PATCH for-rc 0/2] IB/hfi1: Fixes for rc or next if too late Doug Ledford @ 2018-05-31 19:08 ` Dennis Dalessandro 2018-05-31 20:21 ` Jason Gunthorpe 0 siblings, 1 reply; 8+ messages in thread From: Dennis Dalessandro @ 2018-05-31 19:08 UTC (permalink / raw) To: Doug Ledford, jgg Cc: linux-rdma, Michael J. Ruhl, Mike Marciniszyn, stable, Kaike Wan On 5/31/2018 2:47 PM, Doug Ledford wrote: > On Thu, 2018-05-31 at 11:29 -0700, Dennis Dalessandro wrote: >> Hi Doug and Jason, >> >> We have two more late breaking fix up patches. The DMA_RTAIL fix is the more >> serious of the two. I realize we are at the tail end of 4.17 so I would not be >> against holding off till 4.18 for these, but if there is another rdma >> pull request we may want to tack these on. >> >> --- >> >> Kaike Wan (1): >> IB/hfi1: Ensure VL index is within bounds >> >> Mike Marciniszyn (1): >> IB/hfi1: Fix user context tail allocation for DMA_RTAIL >> >> >> drivers/infiniband/hw/hfi1/chip.c | 8 ++++---- >> drivers/infiniband/hw/hfi1/file_ops.c | 2 +- >> drivers/infiniband/hw/hfi1/init.c | 9 ++++----- >> drivers/infiniband/hw/hfi1/sdma.c | 12 +++--------- >> 4 files changed, 12 insertions(+), 19 deletions(-) >> >> -- >> -Denny > > Hi Denny, > > These two patches look fine in terms of the patches themselves. In > terms of whether to put them in for-rc or for-next, what's the > consequences of hitting each of these bugs? > The VL index, could be bad because it would jump beyond the end of the array. However, we won't actually hit that with the code the way it currently is because of the way we validate the VL in other areas of the code. This is more of a we better fix it before we do end up with a problem sort of thing. In the other one, the DMA_RTAIL one, the driver ends up mmaping NULL and handing that user space. This only happens though if users muck with the CAP_MASK and enable the dma of the rtail. Which is not the default. Mike found this through code inspection I believe. So they do fix serious flaws, but the likelihood of actually hitting them is very slim. Based on the stable tag on Mike's patch we have had this since 4.9. -Denny ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH for-rc 0/2] IB/hfi1: Fixes for rc or next if too late 2018-05-31 19:08 ` Dennis Dalessandro @ 2018-05-31 20:21 ` Jason Gunthorpe 2018-05-31 20:29 ` Dennis Dalessandro 0 siblings, 1 reply; 8+ messages in thread From: Jason Gunthorpe @ 2018-05-31 20:21 UTC (permalink / raw) To: Dennis Dalessandro Cc: Doug Ledford, linux-rdma, Michael J. Ruhl, Mike Marciniszyn, stable, Kaike Wan On Thu, May 31, 2018 at 03:08:56PM -0400, Dennis Dalessandro wrote: > On 5/31/2018 2:47 PM, Doug Ledford wrote: > >On Thu, 2018-05-31 at 11:29 -0700, Dennis Dalessandro wrote: > >>Hi Doug and Jason, > >> > >>We have two more late breaking fix up patches. The DMA_RTAIL fix is the more > >>serious of the two. I realize we are at the tail end of 4.17 so I would not be > >>against holding off till 4.18 for these, but if there is another rdma > >>pull request we may want to tack these on. > >> > >> > >>Kaike Wan (1): > >> IB/hfi1: Ensure VL index is within bounds > >> > >>Mike Marciniszyn (1): > >> IB/hfi1: Fix user context tail allocation for DMA_RTAIL > >> > >> > >> drivers/infiniband/hw/hfi1/chip.c | 8 ++++---- > >> drivers/infiniband/hw/hfi1/file_ops.c | 2 +- > >> drivers/infiniband/hw/hfi1/init.c | 9 ++++----- > >> drivers/infiniband/hw/hfi1/sdma.c | 12 +++--------- > >> 4 files changed, 12 insertions(+), 19 deletions(-) > >> > > > >Hi Denny, > > > >These two patches look fine in terms of the patches themselves. In > >terms of whether to put them in for-rc or for-next, what's the > >consequences of hitting each of these bugs? > > > > The VL index, could be bad because it would jump beyond the end of the > array. However, we won't actually hit that with the code the way it > currently is because of the way we validate the VL in other areas of the > code. This is more of a we better fix it before we do end up with a problem > sort of thing. Theoretical future bugs are not rc or stable material > In the other one, the DMA_RTAIL one, the driver ends up mmaping NULL and > handing that user space. This only happens though if users muck with the > CAP_MASK and enable the dma of the rtail. Which is not the default. Mike > found this through code inspection I believe. > So they do fix serious flaws, but the likelihood of actually hitting them is > very slim. Based on the stable tag on Mike's patch we have had this since > 4.9. I think it is too late for more -rc stuff.. The last -rc (assuming rc7 is the end) pull request needs to go tomorrow morning and we like it to have -rc stuff sit in -next for at least a day before sending to Linus :\ Jason ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH for-rc 0/2] IB/hfi1: Fixes for rc or next if too late 2018-05-31 20:21 ` Jason Gunthorpe @ 2018-05-31 20:29 ` Dennis Dalessandro 0 siblings, 0 replies; 8+ messages in thread From: Dennis Dalessandro @ 2018-05-31 20:29 UTC (permalink / raw) To: Jason Gunthorpe Cc: Doug Ledford, linux-rdma, Michael J. Ruhl, Mike Marciniszyn, stable, Kaike Wan On 5/31/2018 4:21 PM, Jason Gunthorpe wrote: > On Thu, May 31, 2018 at 03:08:56PM -0400, Dennis Dalessandro wrote: >> On 5/31/2018 2:47 PM, Doug Ledford wrote: >>> On Thu, 2018-05-31 at 11:29 -0700, Dennis Dalessandro wrote: >>>> Hi Doug and Jason, >>>> >>>> We have two more late breaking fix up patches. The DMA_RTAIL fix is the more >>>> serious of the two. I realize we are at the tail end of 4.17 so I would not be >>>> against holding off till 4.18 for these, but if there is another rdma >>>> pull request we may want to tack these on. >>>> >>>> >>>> Kaike Wan (1): >>>> IB/hfi1: Ensure VL index is within bounds >>>> >>>> Mike Marciniszyn (1): >>>> IB/hfi1: Fix user context tail allocation for DMA_RTAIL >>>> >>>> >>>> drivers/infiniband/hw/hfi1/chip.c | 8 ++++---- >>>> drivers/infiniband/hw/hfi1/file_ops.c | 2 +- >>>> drivers/infiniband/hw/hfi1/init.c | 9 ++++----- >>>> drivers/infiniband/hw/hfi1/sdma.c | 12 +++--------- >>>> 4 files changed, 12 insertions(+), 19 deletions(-) >>>> >>> >>> Hi Denny, >>> >>> These two patches look fine in terms of the patches themselves. In >>> terms of whether to put them in for-rc or for-next, what's the >>> consequences of hitting each of these bugs? >>> >> >> The VL index, could be bad because it would jump beyond the end of the >> array. However, we won't actually hit that with the code the way it >> currently is because of the way we validate the VL in other areas of the >> code. This is more of a we better fix it before we do end up with a problem >> sort of thing. > > Theoretical future bugs are not rc or stable material The VL index one is not marked as stable, however the other one is. That is because it is a real bug, as Mike said to me earlier, we provide the tools to play, the concern is if someone does. So I think it meets stable bar, but not necessarily this late of an -rc. >> In the other one, the DMA_RTAIL one, the driver ends up mmaping NULL and >> handing that user space. This only happens though if users muck with the >> CAP_MASK and enable the dma of the rtail. Which is not the default. Mike >> found this through code inspection I believe. > >> So they do fix serious flaws, but the likelihood of actually hitting them is >> very slim. Based on the stable tag on Mike's patch we have had this since >> 4.9. > > I think it is too late for more -rc stuff.. > > The last -rc (assuming rc7 is the end) pull request needs to go > tomorrow morning and we like it to have -rc stuff sit in -next for at > least a day before sending to Linus :\ Agree, and fine with me. -Denny ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCH for-rc 0/2] IB/hfi1: Fixes for rc or next if too late 2018-05-31 18:29 ` Dennis Dalessandro ` (2 preceding siblings ...) (?) @ 2018-06-04 19:34 ` Jason Gunthorpe -1 siblings, 0 replies; 8+ messages in thread From: Jason Gunthorpe @ 2018-06-04 19:34 UTC (permalink / raw) To: Dennis Dalessandro Cc: dledford, linux-rdma, Michael J. Ruhl, Mike Marciniszyn, stable, Kaike Wan On Thu, May 31, 2018 at 11:29:59AM -0700, Dennis Dalessandro wrote: > Hi Doug and Jason, > > We have two more late breaking fix up patches. The DMA_RTAIL fix is the more > serious of the two. I realize we are at the tail end of 4.17 so I would not be > against holding off till 4.18 for these, but if there is another rdma > pull request we may want to tack these on. > > > Kaike Wan (1): > IB/hfi1: Ensure VL index is within bounds > > Mike Marciniszyn (1): > IB/hfi1: Fix user context tail allocation for DMA_RTAIL applied to for-next since the merge window is open now Thanks, Jason ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-06-04 19:34 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-05-31 18:29 [PATCH for-rc 0/2] IB/hfi1: Fixes for rc or next if too late Dennis Dalessandro 2018-05-31 18:29 ` Dennis Dalessandro 2018-05-31 18:30 ` [PATCH for-rc 1/2] IB/hfi1: Fix user context tail allocation for DMA_RTAIL Dennis Dalessandro 2018-05-31 18:47 ` [PATCH for-rc 0/2] IB/hfi1: Fixes for rc or next if too late Doug Ledford 2018-05-31 19:08 ` Dennis Dalessandro 2018-05-31 20:21 ` Jason Gunthorpe 2018-05-31 20:29 ` Dennis Dalessandro 2018-06-04 19:34 ` Jason Gunthorpe
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.