linux-kernel - Re: [PATCH] USB: ehci: use packed,aligned(4) instead of removing the packed attribute

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: <20110620205559.GM26089@n2100.arm.linux.org.uk>
Date:	Mon, 20 Jun 2011 21:55:59 +0100
From:	Russell King - ARM Linux <linux@....linux.org.uk>
To:	Arnd Bergmann <arnd@...db.de>
Cc:	linux-arm-kernel@...ts.infradead.org,
	Alan Stern <stern@...land.harvard.edu>,
	linux-usb@...r.kernel.org, Nicolas Pitre <nico@...xnic.net>,
	gregkh@...e.de, lkml <linux-kernel@...r.kernel.org>,
	Rabin Vincent <rabin@....in>,
	Alexander Holler <holler@...oftware.de>
Subject: Re: [PATCH] USB: ehci: use packed,aligned(4) instead of removing
	the packed attribute

On Mon, Jun 20, 2011 at 10:26:37PM +0200, Arnd Bergmann wrote:
> * We already need a compiler barrier in the non-_relaxed() versions of
>   the I/O accessors, which will force a reload of the base address
>   in a lot of cases, so the code is already suboptimal. Yes, we don't
>   have the barrier today without CONFIG_ARM_DMA_MEM_BUFFERABLE, but that
>   is a bug, because it lets the compiler move accesses to DMA buffers
>   around readl/writel.

You're now being obtuse there.  You don't need compiler barriers to
guarantee order - that's what volatile does there.

Before you start quoting stuff about volatile, look at the
volatile-considered-harmful.txt document:

  - The above-mentioned accessor functions might use volatile on
    architectures where direct I/O memory access does work.  Essentially,
    each accessor call becomes a little critical section on its own and
    ensures that the access happens as expected by the programmer.

which is what we're doing here.  And because each accessor is its own
little critical section, there's no need for a compiler barrier.

> > If it is the case that these structures do not require packing to get
> > their desired layout, then they don't require packing, and the packed
> > attribute should be dropped.
> 
> Yes. But are you going to audit every other use of __packed in the kernel
> to check if it is used on __iomem pointers?

As I've said, using __packed on __iomem pointers is fraught for many
reasons, and ignoring the other reasons and just concentrating on the
IO accessor problem is bad news in any case.

So yes, __packed needs to be solved _irrespective_ of the IO accessor
issue.
--
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/