linux-kernel - Re: [RFC 1/6] omap: iommu: generic iommu api migration

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <BANLkTineGVvmph=Om2FGR_+mkiMW6k7UAw@mail.gmail.com>
Date:	Tue, 7 Jun 2011 14:19:05 +0300
From:	Ohad Ben-Cohen <ohad@...ery.com>
To:	Laurent Pinchart <laurent.pinchart@...asonboard.com>
Cc:	linux-media@...r.kernel.org, linux-omap@...r.kernel.org,
	linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org,
	Hiroshi.DOYU@...ia.com, arnd@...db.de, davidb@...eaurora.org,
	Joerg.Roedel@....com
Subject: Re: [RFC 1/6] omap: iommu: generic iommu api migration

Hi Laurent,

On Tue, Jun 7, 2011 at 12:22 PM, Laurent Pinchart
<laurent.pinchart@...asonboard.com> wrote:
>> +     BUG_ON(!IS_ALIGNED((long)omap_domain->pgtable, IOPGD_TABLE_SIZE));
>
> Either __get_free_pages() guarantees that the allocated memory will be aligned
> on an IOPGD_TABLE_SIZE boundary, in which case the BUG_ON() is unnecessary, or
> doesn't offer such guarantee, in which case the BUG_ON() will oops randomly.

Curious, does it oops randomly today ?
(i just copied this from omap_iommu_probe, where it always existed).

It is a bit ugly though, and thinking on it again, 16KB is not that
big. We can just use kmalloc here, which does ensure the alignment
(or, better yet, kzalloc, and then ditch the memset).

> In both cases BUG_ON() should probably be avoided.

I disagree; we must check this so user data won't be harmed (hardware
requirement), and if a memory allocation API fails to meet its
requirements - that's really bad and user data is again at stake (much
more will break, not only the iommu driver).

> This leaks omap_domain->pgtable.
>
> The free_pages() call in omap_iommu_remove() should be removed, as
> omap_iommu_probe() doesn't allocate the pages table anymore.

thanks !

> You can also remove the the struct iommu::iopgd field.

No, I can't; it's used when the device is attached to an address space domain.

> You return 0 in the bogus pte/pgd cases. Is that intentional ?

Yes, that's probably the most (if any) reasonable value to return here
(all other iommu implementations are doing so too).

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