Re: [Qemu-devel] [PATVH v2] net: ne2000: fix bounds check in ioport operations

[Jason Wang]

On 12/31/2015 01:56 PM, P J P wrote:
> +-- On Thu, 31 Dec 2015, Jason Wang wrote --+
> | > -        (addr >= NE2000_PMEM_START && addr < NE2000_MEM_SIZE)) {
> | > +    if (addr < 32 || (addr >= NE2000_PMEM_START && addr < 
> NE2000_MEM_SIZE)) {
> | 
> | The change is unnecessary.
>
>   Okay.
>  
> | > +    if (addr < 32
> | > +        || (addr >= NE2000_PMEM_START
> | > +            && addr + sizeof(uint16_t) < NE2000_MEM_SIZE)) {
> | 
> | I think you mean '<=' instead of '<' here? (And for the other checks below).
>
>   I think <= would lead to an off-by-one, no?

The real byte we could touch is in fact addr + sizeof(uint16_t) -1 here.

Consider we should allow double bytes access at NE2000_MEM_SIZE - 2, but
this patch forbids this.

Btw, looking at ne2000_mem_writew(), it has:

addr &= ~1;

at the beginning, so looks like we are really safe,  Need only to care
about writel?

>  As the last array index would be 
> one less than the size; Same as ne2000_mem_readb() above.
>
> Thank you.
> --
> Prasad J Pandit / Red Hat Product Security Team
> 47AF CE69 3A90 54AA 9045 1053 DD13 3D32 FE5B 041F