Bug in vme subsystem (vme.c)

[ternaryd at gmail.com]

On Sat, 23 Feb 2013 22:39:34 +0300
Dan Carpenter <dan.carpenter at oracle.com> wrote:

> I looked at the assembly and I think the second test is evaluated.

Yes, sorry, it is evaluated, if size is smaller than VME_A16_MAX,
which is not my case.

> It looks like it's testing for integer overflow, but not correctly.
> A large value of size (which comes directly from the user in
> vme_user_ioctl()) could make vme_base + size wrap to a low number.
> 
> It should be:
> 
> 	if (size > VME_A16_MAX || vme_base > VME_A16_MAX ||
> 	    size + vme_base > VME_A16_MAX)
> 		retval = -EFAULT;
> 
> That way it can't overflow.  Presumably only root can call
> vme_master_set() so this isn't a security bug.

All three, vme_base, size and VME_A16_MAX, are 64-bit unsigned integers.
Addresses are added to vme_base and are the only thing needing to fit
into 16-bit. Also, the sum vme_base + address does not have this limit.
VME_A16_MAX can not be compared with vme_base. But there actually are
alignment restrictions which should be tested for.

> > As slave windows must not overlap, this means that there can not be
> > more than one window in this address space on any VME bus member,
> > because the only valid base address would be 0x0.

> I'm not sure how vme works so hopefully someone else can comment on
> this.

Again, I forgot to mention that it must be a full-size window for my
statement to be correct. The insight, that slave windows must not
overlap, comes from Mr. Welch:

        Master windows can overlap, two devices can read from the same
        "peripheral!" device at different times. Slave windows cannot
        overlap.

So, slave windows can not overlap. But there can be more than one
full-sized slave window in the same address space of the same VME bus.

regards,

-- 
Christoph