[PATCH] arm: dma-mapping: Fix mapping size value

[Ritesh Harjani]

Ok thanks Marek,

I was about to send a new patch (as I had now got hold of my system).
Anyways, I will add this discussion of cleaning up this variable in my
to-do list.


Thanks
Ritesh

On Wed, Apr 23, 2014 at 6:47 PM, Marek Szyprowski
<m.szyprowski at samsung.com> wrote:
> Hello,
>
>
> On 2014-04-23 12:04, Ritesh Harjani wrote:
>>
>> Hi Marek/Will
>>
>> On Wed, Apr 23, 2014 at 3:00 PM, Laurent Pinchart
>> <laurent.pinchart at ideasonboard.com> wrote:
>> > Hi Marek,
>> >
>> > On Wednesday 23 April 2014 10:53:24 Marek Szyprowski wrote:
>> >> On 2014-04-22 10:53, Will Deacon wrote:
>> >> > On Mon, Apr 21, 2014 at 07:47:27AM +0100, Ritesh Harjani wrote:
>> >> > > 68efd7d2fb("arm: dma-mapping: remove order parameter from
>> >> > > arm_iommu_create_mapping()") is causing kernel panic
>> >> > > because it wrongly sets the value of mapping->size:
>> >> > >
>> >> > > Unable to handle kernel NULL pointer dereference at virtual
>> >> > > address 000000a0
>> >> > > pgd = e7a84000
>> >> > > [000000a0] *pgd=00000000
>> >> > > ...
>> >> > > PC is at bitmap_clear+0x48/0xd0
>> >> > > LR is at __iommu_remove_mapping+0x130/0x164
>> >> > >
>> >> > > Fix it by correcting mapping->size value.
>> >> > >
>> >> > > Signed-off-by: Ritesh Harjani <ritesh.harjani at gmail.com>
>> >> > > Acked-by: Laurent Pinchart <laurent.pinchart at ideasonboard.com>
>> >> > > ---
>> >> > >
>> >> > >  arch/arm/mm/dma-mapping.c | 2 +-
>> >> > >  1 file changed, 1 insertion(+), 1 deletion(-)
>> >> > >
>> >> > > diff --git a/arch/arm/mm/dma-mapping.c b/arch/arm/mm/dma-mapping.c
>> >> > > index f62aa06..6b00be1 100644
>> >> > > --- a/arch/arm/mm/dma-mapping.c
>> >> > > +++ b/arch/arm/mm/dma-mapping.c
>> >> > > @@ -1963,8 +1963,8 @@ arm_iommu_create_mapping(struct bus_type
>> >> > > *bus,
>> >> > > dma_addr_t base, size_t size)> >
>> >> > >   mapping->nr_bitmaps = 1;
>> >> > >   mapping->extensions = extensions;
>> >> > >   mapping->base = base;
>> >> > >
>> >> > > - mapping->size = bitmap_size << PAGE_SHIFT;
>> >> > >
>> >> > >   mapping->bits = BITS_PER_BYTE * bitmap_size;
>> >> > >
>> >> > > + mapping->size = mapping->bits << PAGE_SHIFT;
>> >> >
>> >> > Ok, but given that mapping->size is derived from mapping->bits, do we
>> >> > really need both of these fields in struct dma_iommu_mapping?
>> >>
>> >> You are right. I didn't notice this while I was refactoring the code.
>> >> Ritesh, could you update your patch and simply replace all references
>> >> of
>> >> mapping->size with (mapping->bits << PAGE_SHIFT), probably with some
>> >> temporary variable to make the code easier to understand? I've didn't
>> >> apply
>> >> your patch yet.
>> >
>> > As this patch fixes a v3.15 regression, shouldn't it be applied as-is
>> > and
>> > ASAP, with the cleanup that removes mapping->size coming in a later,
>> > less
>> > urgent patch ?
>>
>> I agree with Laurent. Anyway this cleanup can be taken care when we will
>> be
>> doing refactoring of common code to lib/iommu-helper.c.
>>
>> Anyways, if you still insist I can prepare and submit the patch. Let me
>> know
>> again on this.
>
>
> Ok, I've merged the patch as is and I will send pull request soon.
> Please include the above discussed cleanup while refactoring common code to
> lib.
>
>
> Best regards
> --
> Marek Szyprowski, PhD
> Samsung R&D Institute Poland
>