On Tue, Mar 8, 2011 at 9:53 PM, Guzman Lugo, Fernando
<fernando.lugo@xxxxxx> wrote:
> On Tue, Mar 8, 2011 at 1:06 PM, David Cohen <dacohen@xxxxxxxxx> wrote:
>> On Tue, Mar 8, 2011 at 8:06 PM, Guzman Lugo, Fernando
>> <fernando.lugo@xxxxxx> wrote:
>>> On Tue, Mar 8, 2011 at 6:46 AM, David Cohen <dacohen@xxxxxxxxx> wrote:
>>>> From: Michael Jones <michael.jones@xxxxxxxxxxxxxxxx>
>>>>
>>>> commit c7f4ab26e3bcdaeb3e19ec658e3ad9092f1a6ceb allowed mapping
>>>> the NULL address if da_start==0, which would then not get unmapped.
>>>> Disallow this again. ÂAnd spell variable 'alignment' correctly.
>>>>
>>>> Signed-off-by: Michael Jones <michael.jones@xxxxxxxxxxxxxxxx>
>>>> ---
>>>> Âarch/arm/plat-omap/iovmm.c | Â 16 ++++++++++------
>>>> Â1 files changed, 10 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/arch/arm/plat-omap/iovmm.c b/arch/arm/plat-omap/iovmm.c
>>>> index 6dc1296..11c9b76 100644
>>>> --- a/arch/arm/plat-omap/iovmm.c
>>>> +++ b/arch/arm/plat-omap/iovmm.c
>>>> @@ -271,20 +271,24 @@ static struct iovm_struct *alloc_iovm_area(struct iommu *obj, u32 da,
>>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â size_t bytes, u32 flags)
>>>> Â{
>>>> Â Â Â Âstruct iovm_struct *new, *tmp;
>>>> - Â Â Â u32 start, prev_end, alignement;
>>>> + Â Â Â u32 start, prev_end, alignment;
>>>>
>>>> Â Â Â Âif (!obj || !bytes)
>>>> Â Â Â Â Â Â Â Âreturn ERR_PTR(-EINVAL);
>>>>
>>>> Â Â Â Âstart = da;
>>>> - Â Â Â alignement = PAGE_SIZE;
>>>> + Â Â Â alignment = PAGE_SIZE;
>>>>
>>>> Â Â Â Âif (flags & IOVMF_DA_ANON) {
>>>> - Â Â Â Â Â Â Â start = obj->da_start;
>>>> + Â Â Â Â Â Â Â /* Don't map address 0 */
>>>> + Â Â Â Â Â Â Â if (obj->da_start)
>>>> + Â Â Â Â Â Â Â Â Â Â Â start = obj->da_start;
>>>> + Â Â Â Â Â Â Â else
>>>> + Â Â Â Â Â Â Â Â Â Â Â start = obj->da_start + alignment;
>>>
>>> else part obj->da_star is always 0, so why not?
>>> start = alignment;
>>
>> The patch is not from me, but I can fix it if Michael agrees.
>>
>>>
>>> so, why I understand, it now it is possible mapp address 0x0 only if
>>> IOVMF_DA_ANON is not set, right? maybe that would be mention in the
>>> patch.
>>
>> Indeed address 0x0 was never meant to be mapped. The mentioned commit
>> on the patch's description did that without noticing. But the new
>> change is documented in the code by the comment "Don't map address 0"
>
> yeah, but it only applies when "flags & IOVMF_DA_ANON", So if I use
> IOVMF_DA_FIXED and da_start == 0, I will be able to map some area
> which starts from address 0x0, right? Âif so, that looks good to me
> and the description should mention that if is disallowing mapping
> address 0x0 when IOVMF_DA_ANON is used.
Yes, that's the case. I will update the comment. I hope Michael does
not complain about the changes. :)
Br,
David
>
> Regards,
> Fernando.
>
>> and it's also mentioned on the patch description. Is it OK for you?
>>
>> Regards,
>>
>> David Cohen
>>
>>>
>>> Regards,
>>> Fernando.
>>>
>>>>
>>>> Â Â Â Â Â Â Â Âif (flags & IOVMF_LINEAR)
>>>> - Â Â Â Â Â Â Â Â Â Â Â alignement = iopgsz_max(bytes);
>>>> - Â Â Â Â Â Â Â start = roundup(start, alignement);
>>>> + Â Â Â Â Â Â Â Â Â Â Â alignment = iopgsz_max(bytes);
>>>> + Â Â Â Â Â Â Â start = roundup(start, alignment);
>>>> Â Â Â Â} else if (start < obj->da_start || start > obj->da_end ||
>>>> Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Â Âobj->da_end - start < bytes) {
>>>> Â Â Â Â Â Â Â Âreturn ERR_PTR(-EINVAL);
>>>> @@ -304,7 +308,7 @@ static struct iovm_struct *alloc_iovm_area(struct iommu *obj, u32 da,
>>>> Â Â Â Â Â Â Â Â Â Â Â Âgoto found;
>>>>
>>>> Â Â Â Â Â Â Â Âif (tmp->da_end >= start && flags & IOVMF_DA_ANON)
>>>> - Â Â Â Â Â Â Â Â Â Â Â start = roundup(tmp->da_end + 1, alignement);
>>>> + Â Â Â Â Â Â Â Â Â Â Â start = roundup(tmp->da_end + 1, alignment);
>>>>
>>>> Â Â Â Â Â Â Â Âprev_end = tmp->da_end;
>>>> Â Â Â Â}
>>>> --
>>>> 1.7.0.4
>>>>
>>>>
>>>
>>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-omap" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
