An off-by-one error in a switch case in onenand_read() allowed
a misbehaving guest to read off the end of a block of memory.

NB: the onenand device is used only by the "n800" and "n810"
machines, which are usable only with TCG, not KVM, so this is
not a security issue.

Reported-by: Thomas Huth <thuth@redhat.com>
Suggested-by: Richard Henderson <richard.henderson@linaro.org>
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
---
I tweaked RTH's suggested fix to use an 0xbffe offset so
we don't overrun on an access to 0xbfff either.

 hw/block/onenand.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/block/onenand.c b/hw/block/onenand.c
index 0cb8d7fa135..49ef68c9b14 100644
--- a/hw/block/onenand.c
+++ b/hw/block/onenand.c
@@ -608,7 +608,7 @@ static uint64_t onenand_read(void *opaque, hwaddr addr,
     int offset = addr >> s->shift;
 
     switch (offset) {
-    case 0x0000 ... 0xc000:
+    case 0x0000 ... 0xbffe:
         return lduw_le_p(s->boot[0] + addr);
 
     case 0xf000:	/* Manufacturer ID */
-- 
2.19.1

On 11/15/18 3:35 PM, Peter Maydell wrote:
> An off-by-one error in a switch case in onenand_read() allowed
> a misbehaving guest to read off the end of a block of memory.
> 
> NB: the onenand device is used only by the "n800" and "n810"
> machines, which are usable only with TCG, not KVM, so this is
> not a security issue.
> 
> Reported-by: Thomas Huth <thuth@redhat.com>
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I tweaked RTH's suggested fix to use an 0xbffe offset so
> we don't overrun on an access to 0xbfff either.
> 
>  hw/block/onenand.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Richard Henderson <richard.henderson@linaro.org>


r~

On 15/11/18 15:35, Peter Maydell wrote:
> An off-by-one error in a switch case in onenand_read() allowed
> a misbehaving guest to read off the end of a block of memory.
> 
> NB: the onenand device is used only by the "n800" and "n810"
> machines, which are usable only with TCG, not KVM, so this is
> not a security issue.
> 
> Reported-by: Thomas Huth <thuth@redhat.com>
> Suggested-by: Richard Henderson <richard.henderson@linaro.org>
> Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
> ---
> I tweaked RTH's suggested fix to use an 0xbffe offset so
> we don't overrun on an access to 0xbfff either.

Correct.

> 
>   hw/block/onenand.c | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/hw/block/onenand.c b/hw/block/onenand.c
> index 0cb8d7fa135..49ef68c9b14 100644
> --- a/hw/block/onenand.c
> +++ b/hw/block/onenand.c
> @@ -608,7 +608,7 @@ static uint64_t onenand_read(void *opaque, hwaddr addr,
>       int offset = addr >> s->shift;
>   
>       switch (offset) {
> -    case 0x0000 ... 0xc000:
> +    case 0x0000 ... 0xbffe:
>           return lduw_le_p(s->boot[0] + addr);
>   
>       case 0xf000:	/* Manufacturer ID */
> 

Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>