MTD pointer alignment

[Gavin Lambert]

Quoth Artem Bityutskiy [dedekind at infradead.org]:
> On Wed, 2006-11-29 at 13:09 +1300, Gavin Lambert wrote:
>> Given that mtdchar assumes that it can mess with the low 2 bits of
>> the MTD pointer it keeps around with impunity, add_mtd_device in
>> mtdcore.c should probably refuse to add any MTD device pointer that
>> doesn't fall on a 4-byte boundary.
> 
> Hmm, I personally I do not understand what is this about.

In mtdchar.c, when an MTD character device is opened (mtd_open), its
pointer value is assigned to file->private_data.  All subsequent
operations then use the TO_MTD macro, which is defined as:

#define TO_MTD(file) (struct mtd_info *)((long)((file)->private_data) &
~3L)

(ie. masking off the lower two bits).  There's even a comment above that
those bits have been hijacked for OTP modes, and it's expecting that
alignment of the pointer is at least 32 bits.

While this is true for a standalone kalloc'd MTD structure, if the MTD
structure is embedded within another structure then it is not
necessarily the case.  Since struct mtd_info starts with a byte-sized
field, the default padding rules say that the structure as a whole is
allowed to start on a byte boundary (although on many arches it'll get
16-bit aligned regardless).  Hence the 32-bit alignment is most
definitely *not* guaranteed.

If you're going to hijack the bits then you should at the very least
test your assertion in either mtd_open or add_mtd_device, by making it
reject MTD devices that aren't aligned as you'd expect.  Otherwise it
leads to a whole raft of weird bugs.

As I said before, though, I'm looking at the MTD sources as of kernel
2.6.15.  It's possible this has been sorted out since then.  If so, I
apologise.