[PATCH]: iommu fix potential overflow in alloc_iommu() - Kernel

This is a discussion on [PATCH]: iommu fix potential overflow in alloc_iommu() - Kernel ; (This didn't appear on LKML or any of the mirrors ... trying again) It is possible that alloc_iommu()'s boundary_size overflows as dma_get_seg_boundary can return 0xffffffff. In that case, further usage of boundary_size triggers a BUG_ON() in the iommu code. Signed-off-by: ...

+ Reply to Thread
Results 1 to 7 of 7

Thread: [PATCH]: iommu fix potential overflow in alloc_iommu()

  1. [PATCH]: iommu fix potential overflow in alloc_iommu()

    (This didn't appear on LKML or any of the mirrors ... trying again)

    It is possible that alloc_iommu()'s boundary_size overflows as
    dma_get_seg_boundary can return 0xffffffff. In that case, further usage of
    boundary_size triggers a BUG_ON() in the iommu code.

    Signed-off-by: Prarit Bhargava

    diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c
    index faf3229..baecb47 100644
    --- a/arch/x86/kernel/pci-gart_64.c
    +++ b/arch/x86/kernel/pci-gart_64.c
    @@ -91,7 +91,7 @@ static unsigned long alloc_iommu(struct device *dev, int size)

    base_index = ALIGN(iommu_bus_base & dma_get_seg_boundary(dev),
    PAGE_SIZE) >> PAGE_SHIFT;
    - boundary_size = ALIGN(dma_get_seg_boundary(dev) + 1,
    + boundary_size = ALIGN((unsigned long long)dma_get_seg_boundary(dev) + 1,
    PAGE_SIZE) >> PAGE_SHIFT;

    spin_lock_irqsave(&iommu_bitmap_lock, flags);
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  2. [PATCH]: PCI: GART iommu alignment fixes

    pci_alloc_consistent/dma_alloc_coherent is supposed to return size aligned
    addresses.

    From Documentation/DMA-mapping.txt:

    "pci_alloc_consistent returns two values: the virtual address which you
    can use to access it from the CPU and dma_handle which you pass to the
    card.

    The cpu return address and the DMA bus master address are both
    guaranteed to be aligned to the smallest PAGE_SIZE order which
    is greater than or equal to the requested size. This invariant
    exists (for example) to guarantee that if you allocate a chunk
    which is smaller than or equal to 64 kilobytes, the extent of the
    buffer you receive will not cross a 64K boundary."

    Fix the GART's alloc_iommu code to return a size aligned address. Also fix
    an incorrect alignment calculation in the iommu-helper code.

    Signed-off-by: Prarit Bhargava

    diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c
    index faf3229..329718e 100644
    --- a/arch/x86/kernel/pci-gart_64.c
    +++ b/arch/x86/kernel/pci-gart_64.c
    @@ -96,11 +96,12 @@ static unsigned long alloc_iommu(struct device *dev, int size)

    spin_lock_irqsave(&iommu_bitmap_lock, flags);
    offset = iommu_area_alloc(iommu_gart_bitmap, iommu_pages, next_bit,
    - size, base_index, boundary_size, 0);
    + size, base_index, boundary_size, size - 1);
    if (offset == -1) {
    need_flush = 1;
    offset = iommu_area_alloc(iommu_gart_bitmap, iommu_pages, 0,
    - size, base_index, boundary_size, 0);
    + size, base_index, boundary_size,
    + size - 1);
    }
    if (offset != -1) {
    set_bit_string(iommu_gart_bitmap, offset, size);
    diff --git a/lib/iommu-helper.c b/lib/iommu-helper.c
    index a3b8d4c..b970f1b 100644
    --- a/lib/iommu-helper.c
    +++ b/lib/iommu-helper.c
    @@ -16,7 +16,7 @@ again:
    index = find_next_zero_bit(map, size, start);

    /* Align allocation */
    - index = (index + align_mask) & ~align_mask;
    + index = (index + align_mask + 1) & ~align_mask;

    end = index + nr;
    if (end >= size)
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  3. Re: [PATCH]: PCI: GART iommu alignment fixes

    On Mon, 21 Jul 2008 10:15:27 -0400
    Prarit Bhargava wrote:

    > pci_alloc_consistent/dma_alloc_coherent is supposed to return size aligned
    > addresses.
    >
    > From Documentation/DMA-mapping.txt:
    >
    > "pci_alloc_consistent returns two values: the virtual address which you
    > can use to access it from the CPU and dma_handle which you pass to the
    > card.
    >
    > The cpu return address and the DMA bus master address are both
    > guaranteed to be aligned to the smallest PAGE_SIZE order which
    > is greater than or equal to the requested size. This invariant
    > exists (for example) to guarantee that if you allocate a chunk
    > which is smaller than or equal to 64 kilobytes, the extent of the
    > buffer you receive will not cross a 64K boundary."
    >
    > Fix the GART's alloc_iommu code to return a size aligned address. Also fix
    > an incorrect alignment calculation in the iommu-helper code.
    >
    > Signed-off-by: Prarit Bhargava
    >
    > diff --git a/arch/x86/kernel/pci-gart_64.c b/arch/x86/kernel/pci-gart_64.c
    > index faf3229..329718e 100644
    > --- a/arch/x86/kernel/pci-gart_64.c
    > +++ b/arch/x86/kernel/pci-gart_64.c
    > @@ -96,11 +96,12 @@ static unsigned long alloc_iommu(struct device *dev, int size)
    >
    > spin_lock_irqsave(&iommu_bitmap_lock, flags);
    > offset = iommu_area_alloc(iommu_gart_bitmap, iommu_pages, next_bit,
    > - size, base_index, boundary_size, 0);
    > + size, base_index, boundary_size, size - 1);
    > if (offset == -1) {
    > need_flush = 1;
    > offset = iommu_area_alloc(iommu_gart_bitmap, iommu_pages, 0,
    > - size, base_index, boundary_size, 0);
    > + size, base_index, boundary_size,
    > + size - 1);
    > }


    This affects map_sg and map_single unnecessarily.


    > if (offset != -1) {
    > set_bit_string(iommu_gart_bitmap, offset, size);
    > diff --git a/lib/iommu-helper.c b/lib/iommu-helper.c
    > index a3b8d4c..b970f1b 100644
    > --- a/lib/iommu-helper.c
    > +++ b/lib/iommu-helper.c
    > @@ -16,7 +16,7 @@ again:
    > index = find_next_zero_bit(map, size, start);
    >
    > /* Align allocation */
    > - index = (index + align_mask) & ~align_mask;
    > + index = (index + align_mask + 1) & ~align_mask;


    Doesn't look right. What we do here is __ALIGN_MASK().
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  4. Re: [PATCH]: iommu fix potential overflow in alloc_iommu()

    On Mon, 21 Jul 2008 10:15:22 -0400
    Prarit Bhargava wrote:

    > (This didn't appear on LKML or any of the mirrors ... trying again)
    >
    > It is possible that alloc_iommu()'s boundary_size overflows as
    > dma_get_seg_boundary can return 0xffffffff. In that case, further usage of
    > boundary_size triggers a BUG_ON() in the iommu code.


    Did you actually hit this? pci-gart_64.c is used only by X86_64.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  5. Re: [PATCH]: iommu fix potential overflow in alloc_iommu()



    FUJITA Tomonori wrote:
    > On Mon, 21 Jul 2008 10:15:22 -0400
    > Prarit Bhargava wrote:
    >
    >
    >> (This didn't appear on LKML or any of the mirrors ... trying again)
    >>
    >> It is possible that alloc_iommu()'s boundary_size overflows as
    >> dma_get_seg_boundary can return 0xffffffff. In that case, further usage of
    >> boundary_size triggers a BUG_ON() in the iommu code.
    >>

    >
    > Did you actually hit this? pci-gart_64.c is used only by X86_64.
    >


    I hit this by declaring a device struct and not declaring a value for
    dev->dma_parms->segment_boundary_mask.

    I was attempting to alloc out of the IOMMU and the code then dies on the
    bugcheck in iommu_is_span_boundary() because boundary_size = 0.

    P.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  6. Re: [PATCH]: iommu fix potential overflow in alloc_iommu()

    On Mon, 21 Jul 2008 11:21:39 -0400
    Prarit Bhargava wrote:

    >
    >
    > FUJITA Tomonori wrote:
    > > On Mon, 21 Jul 2008 10:15:22 -0400
    > > Prarit Bhargava wrote:
    > >
    > >
    > >> (This didn't appear on LKML or any of the mirrors ... trying again)
    > >>
    > >> It is possible that alloc_iommu()'s boundary_size overflows as
    > >> dma_get_seg_boundary can return 0xffffffff. In that case, further usage of
    > >> boundary_size triggers a BUG_ON() in the iommu code.
    > >>

    > >
    > > Did you actually hit this? pci-gart_64.c is used only by X86_64.
    > >

    >
    > I hit this by declaring a device struct and not declaring a value for
    > dev->dma_parms->segment_boundary_mask.


    What do you mean? You set dev->dma_params but does't set
    dev->dma_parms->segment_boundary_mask? If so, you need to fix your
    code. If you set dev->dma_parms, dma_parms needs to be initialized
    properly.
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

  7. Re: [PATCH]: iommu fix potential overflow in alloc_iommu()



    FUJITA Tomonori wrote:
    > On Mon, 21 Jul 2008 11:21:39 -0400
    > Prarit Bhargava wrote:
    >
    >
    >> FUJITA Tomonori wrote:
    >>
    >>> On Mon, 21 Jul 2008 10:15:22 -0400
    >>> Prarit Bhargava wrote:
    >>>
    >>>
    >>>
    >>>> (This didn't appear on LKML or any of the mirrors ... trying again)
    >>>>
    >>>> It is possible that alloc_iommu()'s boundary_size overflows as
    >>>> dma_get_seg_boundary can return 0xffffffff. In that case, further usage of
    >>>> boundary_size triggers a BUG_ON() in the iommu code.
    >>>>
    >>>>
    >>> Did you actually hit this? pci-gart_64.c is used only by X86_64.
    >>>
    >>>

    >> I hit this by declaring a device struct and not declaring a value for
    >> dev->dma_parms->segment_boundary_mask.
    >>

    >
    > What do you mean? You set dev->dma_params but does't set
    > dev->dma_parms->segment_boundary_mask? If so, you need to fix your
    > code. If you set dev->dma_parms, dma_parms needs to be initialized
    > properly.
    >


    No, I didn't set dev->dma_params.

    P.

    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

+ Reply to Thread