Message ID | 1299588365-2749-2-git-send-email-dacohen@gmail.com (mailing list archive) |
---|---|
State | Superseded, archived |
Headers |
Return-path: <mchehab@pedra> Envelope-to: mchehab@pedra Delivery-date: Tue, 08 Mar 2011 09:47:13 -0300 Received: from mchehab by pedra with local (Exim 4.72) (envelope-from <mchehab@pedra>) id 1PwwJk-0005jB-Lw for mchehab@pedra; Tue, 08 Mar 2011 09:47:12 -0300 Received: from casper.infradead.org [85.118.1.10] by pedra with IMAP (fetchmail-6.3.17) for <mchehab@localhost> (single-drop); Tue, 08 Mar 2011 09:47:12 -0300 (BRT) Received: from vger.kernel.org ([209.132.180.67]) by casper.infradead.org with esmtp (Exim 4.72 #1 (Red Hat Linux)) id 1PwwIz-0001Uu-BL; Tue, 08 Mar 2011 12:46:25 +0000 Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1754199Ab1CHMqW (ORCPT <rfc822; kmpark@infradead.org> + 1 other); Tue, 8 Mar 2011 07:46:22 -0500 Received: from mail-ew0-f46.google.com ([209.85.215.46]:39130 "EHLO mail-ew0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752893Ab1CHMqU (ORCPT <rfc822; linux-media@vger.kernel.org>); Tue, 8 Mar 2011 07:46:20 -0500 Received: by ewy6 with SMTP id 6so1963462ewy.19 for <multiple recipients>; Tue, 08 Mar 2011 04:46:16 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:from:to:cc:subject:date:message-id:x-mailer :in-reply-to:references; bh=7q7eOQacs4BOza+k5IaVteuaxXPr6twpqGF+erQdG5Y=; b=CFwtlA1WMGb1fzRXrGD8iHlCG/eIDqgPgkr/jHQialIuP1rZVRyJnbYVEgSkhASd5L 08WcCUcFi4Cp9q6fr8NfNgybIAi8+T9LcS3YgWSMT+dd6xUH2v80mSppWrUs4NK74y+e Lp8z3RVp2CDZzgAgD3bWNlCMo/ZoF2Ggho8xQ= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=from:to:cc:subject:date:message-id:x-mailer:in-reply-to:references; b=TFg0l5065lZtQMlrK6yNyFNdx6aksLxN9ONoZVPBeYBowh22U4nBLMS6iG2OwUnFxx 606dRUrJ2Dns/oBOwPLNkZSvF/AtHEdQqLXt7Q3F6KAH8ZGoW5VtuHHyJxEo00mSW43J YMgxPDkKCqdCVoF8b0E5fgO4E5SGgY8l1MuvU= Received: by 10.213.36.4 with SMTP id r4mr3581758ebd.66.1299588375948; Tue, 08 Mar 2011 04:46:15 -0800 (PST) Received: from localhost.localdomain (a91-152-72-10.elisa-laajakaista.fi [91.152.72.10]) by mx.google.com with ESMTPS id x54sm609369eeh.11.2011.03.08.04.46.14 (version=TLSv1/SSLv3 cipher=OTHER); Tue, 08 Mar 2011 04:46:14 -0800 (PST) From: David Cohen <dacohen@gmail.com> To: Hiroshi.DOYU@nokia.com Cc: linux-omap@vger.kernel.org, fernando.lugo@ti.com, linux-media@vger.kernel.org, laurent.pinchart@ideasonboard.com, sakari.ailus@maxwell.research.nokia.com, Michael Jones <michael.jones@matrix-vision.de> Subject: [PATCH 1/3] omap: iovmm: disallow mapping NULL address Date: Tue, 8 Mar 2011 14:46:03 +0200 Message-Id: <1299588365-2749-2-git-send-email-dacohen@gmail.com> X-Mailer: git-send-email 1.7.0.4 In-Reply-To: <1299588365-2749-1-git-send-email-dacohen@gmail.com> References: <1299588365-2749-1-git-send-email-dacohen@gmail.com> Precedence: bulk List-ID: <linux-media.vger.kernel.org> X-Mailing-List: linux-media@vger.kernel.org Sender: <mchehab@pedra> |
Commit Message
David Cohen
March 8, 2011, 12:46 p.m. UTC
From: Michael Jones <michael.jones@matrix-vision.de> 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@matrix-vision.de> --- arch/arm/plat-omap/iovmm.c | 16 ++++++++++------ 1 files changed, 10 insertions(+), 6 deletions(-)
Comments
On Tue, Mar 8, 2011 at 6:46 AM, David Cohen <dacohen@gmail.com> wrote: > From: Michael Jones <michael.jones@matrix-vision.de> > > 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@matrix-vision.de> > --- > 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; 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. 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-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 8, 2011 at 8:06 PM, Guzman Lugo, Fernando <fernando.lugo@ti.com> wrote: > On Tue, Mar 8, 2011 at 6:46 AM, David Cohen <dacohen@gmail.com> wrote: >> From: Michael Jones <michael.jones@matrix-vision.de> >> >> 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@matrix-vision.de> >> --- >> 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" 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-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 8, 2011 at 1:06 PM, David Cohen <dacohen@gmail.com> wrote: > On Tue, Mar 8, 2011 at 8:06 PM, Guzman Lugo, Fernando > <fernando.lugo@ti.com> wrote: >> On Tue, Mar 8, 2011 at 6:46 AM, David Cohen <dacohen@gmail.com> wrote: >>> From: Michael Jones <michael.jones@matrix-vision.de> >>> >>> 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@matrix-vision.de> >>> --- >>> 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. 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-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Mar 8, 2011 at 9:53 PM, Guzman Lugo, Fernando <fernando.lugo@ti.com> wrote: > On Tue, Mar 8, 2011 at 1:06 PM, David Cohen <dacohen@gmail.com> wrote: >> On Tue, Mar 8, 2011 at 8:06 PM, Guzman Lugo, Fernando >> <fernando.lugo@ti.com> wrote: >>> On Tue, Mar 8, 2011 at 6:46 AM, David Cohen <dacohen@gmail.com> wrote: >>>> From: Michael Jones <michael.jones@matrix-vision.de> >>>> >>>> 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@matrix-vision.de> >>>> --- >>>> 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-media" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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; 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; }