| Message ID | 1346780259-9781-1-git-send-email-sw@weilnetz.de |
|---|---|
| State | Accepted |
| Headers | show |
On 4 September 2012 18:37, Stefan Weil <sw@weilnetz.de> wrote: > Report from smatch: > > mcf5206.c:384 m5206_mbar_readb(7) error: buffer overflow 'm5206_mbar_width' 128 <= 128 > mcf5206.c:403 m5206_mbar_readw(8) error: buffer overflow 'm5206_mbar_width' 128 <= 128 > mcf5206.c:427 m5206_mbar_readl(8) error: buffer overflow 'm5206_mbar_width' 128 <= 128 > mcf5206.c:451 m5206_mbar_writeb(9) error: buffer overflow 'm5206_mbar_width' 128 <= 128 > mcf5206.c:475 m5206_mbar_writew(9) error: buffer overflow 'm5206_mbar_width' 128 <= 128 > mcf5206.c:503 m5206_mbar_writel(9) error: buffer overflow 'm5206_mbar_width' 128 <= 128 > > m5206_mbar_width has 0x80 elements and supports 0 <= offset < 0x200. > > Signed-off-by: Stefan Weil <sw@weilnetz.de> Checked against the data sheet -- last documented register is at offset $1F0, so correcting the offset check rather than the array length is the correct fix. Reviewed-by: Peter Maydell <peter.maydell@linaro.org> -- PMM
Am 04.09.2012 19:57, schrieb Peter Maydell: > On 4 September 2012 18:37, Stefan Weil <sw@weilnetz.de> wrote: >> Report from smatch: >> >> mcf5206.c:384 m5206_mbar_readb(7) error: buffer overflow 'm5206_mbar_width' 128 <= 128 >> mcf5206.c:403 m5206_mbar_readw(8) error: buffer overflow 'm5206_mbar_width' 128 <= 128 >> mcf5206.c:427 m5206_mbar_readl(8) error: buffer overflow 'm5206_mbar_width' 128 <= 128 >> mcf5206.c:451 m5206_mbar_writeb(9) error: buffer overflow 'm5206_mbar_width' 128 <= 128 >> mcf5206.c:475 m5206_mbar_writew(9) error: buffer overflow 'm5206_mbar_width' 128 <= 128 >> mcf5206.c:503 m5206_mbar_writel(9) error: buffer overflow 'm5206_mbar_width' 128 <= 128 >> >> m5206_mbar_width has 0x80 elements and supports 0 <= offset < 0x200. >> >> Signed-off-by: Stefan Weil <sw@weilnetz.de> > Checked against the data sheet -- last documented register is at offset $1F0, > so correcting the offset check rather than the array length is the correct > fix. > > Reviewed-by: Peter Maydell <peter.maydell@linaro.org> > > -- PMM Then m5206_mbar_width should be shortened to 124 elements (0x1f0 / 4) _and_ the offset check needs a correction. -- sw
Am 04.09.2012 20:12, schrieb Stefan Weil: > Am 04.09.2012 19:57, schrieb Peter Maydell: >> On 4 September 2012 18:37, Stefan Weil <sw@weilnetz.de> wrote: >>> Report from smatch: >>> >>> mcf5206.c:384 m5206_mbar_readb(7) error: buffer overflow >>> 'm5206_mbar_width' 128 <= 128 >>> mcf5206.c:403 m5206_mbar_readw(8) error: buffer overflow >>> 'm5206_mbar_width' 128 <= 128 >>> mcf5206.c:427 m5206_mbar_readl(8) error: buffer overflow >>> 'm5206_mbar_width' 128 <= 128 >>> mcf5206.c:451 m5206_mbar_writeb(9) error: buffer overflow >>> 'm5206_mbar_width' 128 <= 128 >>> mcf5206.c:475 m5206_mbar_writew(9) error: buffer overflow >>> 'm5206_mbar_width' 128 <= 128 >>> mcf5206.c:503 m5206_mbar_writel(9) error: buffer overflow >>> 'm5206_mbar_width' 128 <= 128 >>> >>> m5206_mbar_width has 0x80 elements and supports 0 <= offset < 0x200. >>> >>> Signed-off-by: Stefan Weil <sw@weilnetz.de> >> Checked against the data sheet -- last documented register is at >> offset $1F0, >> so correcting the offset check rather than the array length is the >> correct >> fix. >> >> Reviewed-by: Peter Maydell <peter.maydell@linaro.org> >> >> -- PMM > > Then m5206_mbar_width should be shortened to 124 elements > (0x1f0 / 4) _and_ the offset check needs a correction. > > -- sw Sorry, 125 elements, of course. Or are there undocumented registers at 0x1f4, 0x1f8 and 0x1fc? - sw
On 4 September 2012 19:16, Stefan Weil <sw@weilnetz.de> wrote: > Am 04.09.2012 20:12, schrieb Stefan Weil: >> Am 04.09.2012 19:57, schrieb Peter Maydell: >>> Checked against the data sheet -- last documented register is at >>> offset $1F0, so correcting the offset check rather than the array >>> length is the correct fix. >> Then m5206_mbar_width should be shortened to 124 elements >> (0x1f0 / 4) _and_ the offset check needs a correction. Why bother? The relevant offsets will hit the hw_error() cases in m5206_mbar_read() and m5206_mbar_write() anyway, the same as for the other cases where there are "holes" in the register space. The only reason we're doing these checks here is to avoid overrunning the width array... > Sorry, 125 elements, of course. Or are there undocumented > registers at 0x1f4, 0x1f8 and 0x1fc? If there were, I wouldn't know, because they aren't documented :-) -- PMM
On Tue, Sep 04, 2012 at 07:37:39PM +0200, Stefan Weil wrote: > Report from smatch: > > mcf5206.c:384 m5206_mbar_readb(7) error: buffer overflow 'm5206_mbar_width' 128 <= 128 > mcf5206.c:403 m5206_mbar_readw(8) error: buffer overflow 'm5206_mbar_width' 128 <= 128 > mcf5206.c:427 m5206_mbar_readl(8) error: buffer overflow 'm5206_mbar_width' 128 <= 128 > mcf5206.c:451 m5206_mbar_writeb(9) error: buffer overflow 'm5206_mbar_width' 128 <= 128 > mcf5206.c:475 m5206_mbar_writew(9) error: buffer overflow 'm5206_mbar_width' 128 <= 128 > mcf5206.c:503 m5206_mbar_writel(9) error: buffer overflow 'm5206_mbar_width' 128 <= 128 > > m5206_mbar_width has 0x80 elements and supports 0 <= offset < 0x200. > > Signed-off-by: Stefan Weil <sw@weilnetz.de> > --- > hw/mcf5206.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/hw/mcf5206.c b/hw/mcf5206.c > index 539b391..27753e2 100644 > --- a/hw/mcf5206.c > +++ b/hw/mcf5206.c > @@ -378,7 +378,7 @@ static uint32_t m5206_mbar_readb(void *opaque, target_phys_addr_t offset) > { > m5206_mbar_state *s = (m5206_mbar_state *)opaque; > offset &= 0x3ff; > - if (offset > 0x200) { > + if (offset >= 0x200) { > hw_error("Bad MBAR read offset 0x%x", (int)offset); > } > if (m5206_mbar_width[offset >> 2] > 1) { > @@ -397,7 +397,7 @@ static uint32_t m5206_mbar_readw(void *opaque, target_phys_addr_t offset) > m5206_mbar_state *s = (m5206_mbar_state *)opaque; > int width; > offset &= 0x3ff; > - if (offset > 0x200) { > + if (offset >= 0x200) { > hw_error("Bad MBAR read offset 0x%x", (int)offset); > } > width = m5206_mbar_width[offset >> 2]; > @@ -421,7 +421,7 @@ static uint32_t m5206_mbar_readl(void *opaque, target_phys_addr_t offset) > m5206_mbar_state *s = (m5206_mbar_state *)opaque; > int width; > offset &= 0x3ff; > - if (offset > 0x200) { > + if (offset >= 0x200) { > hw_error("Bad MBAR read offset 0x%x", (int)offset); > } > width = m5206_mbar_width[offset >> 2]; > @@ -445,7 +445,7 @@ static void m5206_mbar_writeb(void *opaque, target_phys_addr_t offset, > m5206_mbar_state *s = (m5206_mbar_state *)opaque; > int width; > offset &= 0x3ff; > - if (offset > 0x200) { > + if (offset >= 0x200) { > hw_error("Bad MBAR write offset 0x%x", (int)offset); > } > width = m5206_mbar_width[offset >> 2]; > @@ -469,7 +469,7 @@ static void m5206_mbar_writew(void *opaque, target_phys_addr_t offset, > m5206_mbar_state *s = (m5206_mbar_state *)opaque; > int width; > offset &= 0x3ff; > - if (offset > 0x200) { > + if (offset >= 0x200) { > hw_error("Bad MBAR write offset 0x%x", (int)offset); > } > width = m5206_mbar_width[offset >> 2]; > @@ -497,7 +497,7 @@ static void m5206_mbar_writel(void *opaque, target_phys_addr_t offset, > m5206_mbar_state *s = (m5206_mbar_state *)opaque; > int width; > offset &= 0x3ff; > - if (offset > 0x200) { > + if (offset >= 0x200) { > hw_error("Bad MBAR write offset 0x%x", (int)offset); > } > width = m5206_mbar_width[offset >> 2]; > -- > 1.7.10 > Thanks, applied.
diff --git a/hw/mcf5206.c b/hw/mcf5206.c index 539b391..27753e2 100644 --- a/hw/mcf5206.c +++ b/hw/mcf5206.c @@ -378,7 +378,7 @@ static uint32_t m5206_mbar_readb(void *opaque, target_phys_addr_t offset) { m5206_mbar_state *s = (m5206_mbar_state *)opaque; offset &= 0x3ff; - if (offset > 0x200) { + if (offset >= 0x200) { hw_error("Bad MBAR read offset 0x%x", (int)offset); } if (m5206_mbar_width[offset >> 2] > 1) { @@ -397,7 +397,7 @@ static uint32_t m5206_mbar_readw(void *opaque, target_phys_addr_t offset) m5206_mbar_state *s = (m5206_mbar_state *)opaque; int width; offset &= 0x3ff; - if (offset > 0x200) { + if (offset >= 0x200) { hw_error("Bad MBAR read offset 0x%x", (int)offset); } width = m5206_mbar_width[offset >> 2]; @@ -421,7 +421,7 @@ static uint32_t m5206_mbar_readl(void *opaque, target_phys_addr_t offset) m5206_mbar_state *s = (m5206_mbar_state *)opaque; int width; offset &= 0x3ff; - if (offset > 0x200) { + if (offset >= 0x200) { hw_error("Bad MBAR read offset 0x%x", (int)offset); } width = m5206_mbar_width[offset >> 2]; @@ -445,7 +445,7 @@ static void m5206_mbar_writeb(void *opaque, target_phys_addr_t offset, m5206_mbar_state *s = (m5206_mbar_state *)opaque; int width; offset &= 0x3ff; - if (offset > 0x200) { + if (offset >= 0x200) { hw_error("Bad MBAR write offset 0x%x", (int)offset); } width = m5206_mbar_width[offset >> 2]; @@ -469,7 +469,7 @@ static void m5206_mbar_writew(void *opaque, target_phys_addr_t offset, m5206_mbar_state *s = (m5206_mbar_state *)opaque; int width; offset &= 0x3ff; - if (offset > 0x200) { + if (offset >= 0x200) { hw_error("Bad MBAR write offset 0x%x", (int)offset); } width = m5206_mbar_width[offset >> 2]; @@ -497,7 +497,7 @@ static void m5206_mbar_writel(void *opaque, target_phys_addr_t offset, m5206_mbar_state *s = (m5206_mbar_state *)opaque; int width; offset &= 0x3ff; - if (offset > 0x200) { + if (offset >= 0x200) { hw_error("Bad MBAR write offset 0x%x", (int)offset); } width = m5206_mbar_width[offset >> 2];
Report from smatch: mcf5206.c:384 m5206_mbar_readb(7) error: buffer overflow 'm5206_mbar_width' 128 <= 128 mcf5206.c:403 m5206_mbar_readw(8) error: buffer overflow 'm5206_mbar_width' 128 <= 128 mcf5206.c:427 m5206_mbar_readl(8) error: buffer overflow 'm5206_mbar_width' 128 <= 128 mcf5206.c:451 m5206_mbar_writeb(9) error: buffer overflow 'm5206_mbar_width' 128 <= 128 mcf5206.c:475 m5206_mbar_writew(9) error: buffer overflow 'm5206_mbar_width' 128 <= 128 mcf5206.c:503 m5206_mbar_writel(9) error: buffer overflow 'm5206_mbar_width' 128 <= 128 m5206_mbar_width has 0x80 elements and supports 0 <= offset < 0x200. Signed-off-by: Stefan Weil <sw@weilnetz.de> --- hw/mcf5206.c | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-)