[PATCH] drm/nouveau: fix off by one in BIOS boundry checking

[PATCH] drm/nouveau: fix off by one in BIOS boundry checking

[Nick Lopez]

Bounds checking when parsing init scripts embedded in the BIOS reject
access to the last byte. This causes driver initialization to fail on
Apple eMac's with GeForce 2 MX GPUs, leaving the system with no working
console.

This is probably only seen on OpenFirmware machines like PowerPC Macs
because the BIOS image provides by OF is only the used parts of the ROM,
not a power-of-two blocks read from PCI directly so PCs always have
empty bytes at the end that are never accesseed.

Signed-off-by: Nick Lopez <github@glowingmonkey.org>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/bios/base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/base.c
index d0f52d59fc2f..64e423dddd9e 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/base.c
@@ -38,7 +38,7 @@ nvbios_addr(struct nvkm_bios *bios, u32 *addr, u8 size)
 		*addr += bios->imaged_addr;
 	}
 
-	if (unlikely(*addr + size >= bios->size)) {
+	if (unlikely(*addr + size > bios->size)) {
 		nvkm_error(&bios->subdev, "OOB %d %08x %08x\n", size, p, *addr);
 		return false;
 	}
-- 
2.30.2

[Nouveau] [PATCH] drm/nouveau: fix off by one in BIOS boundry checking

[Nick Lopez]

Bounds checking when parsing init scripts embedded in the BIOS reject
access to the last byte. This causes driver initialization to fail on
Apple eMac's with GeForce 2 MX GPUs, leaving the system with no working
console.

This is probably only seen on OpenFirmware machines like PowerPC Macs
because the BIOS image provides by OF is only the used parts of the ROM,
not a power-of-two blocks read from PCI directly so PCs always have
empty bytes at the end that are never accesseed.

Signed-off-by: Nick Lopez <github@glowingmonkey.org>
---
 drivers/gpu/drm/nouveau/nvkm/subdev/bios/base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/base.c
index d0f52d59fc2f..64e423dddd9e 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/base.c
@@ -38,7 +38,7 @@ nvbios_addr(struct nvkm_bios *bios, u32 *addr, u8 size)
 		*addr += bios->imaged_addr;
 	}
 
-	if (unlikely(*addr + size >= bios->size)) {
+	if (unlikely(*addr + size > bios->size)) {
 		nvkm_error(&bios->subdev, "OOB %d %08x %08x\n", size, p, *addr);
 		return false;
 	}
-- 
2.30.2

Re: [Nouveau] [PATCH] drm/nouveau: fix off by one in BIOS boundry checking

[Karol Herbst]

On Sat, Jan 22, 2022 at 11:44 AM Nick Lopez <github@glowingmonkey.org> wrote:
>
> Bounds checking when parsing init scripts embedded in the BIOS reject
> access to the last byte. This causes driver initialization to fail on
> Apple eMac's with GeForce 2 MX GPUs, leaving the system with no working
> console.
>
> This is probably only seen on OpenFirmware machines like PowerPC Macs
> because the BIOS image provides by OF is only the used parts of the ROM,
> not a power-of-two blocks read from PCI directly so PCs always have
> empty bytes at the end that are never accesseed.
>

small typo nitpicks: provided and accessed

Also, I think it makes sense to add

Fixes: 4d4e9907ff572 "drm/nouveau/bios: guard against out-of-bounds
accesses to image"
Cc: <stable@vger.kernel.org> # v4.10+

so it gets automatically backported to applicable stable kernels

Anyway, whoever picks the patch can make those adjustments as well.

Reviewed-by: Karol Herbst <kherbst@redhat.com>


> Signed-off-by: Nick Lopez <github@glowingmonkey.org>
> ---
>  drivers/gpu/drm/nouveau/nvkm/subdev/bios/base.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/base.c
> index d0f52d59fc2f..64e423dddd9e 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/base.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/base.c
> @@ -38,7 +38,7 @@ nvbios_addr(struct nvkm_bios *bios, u32 *addr, u8 size)
>                 *addr += bios->imaged_addr;
>         }
>
> -       if (unlikely(*addr + size >= bios->size)) {
> +       if (unlikely(*addr + size > bios->size)) {
>                 nvkm_error(&bios->subdev, "OOB %d %08x %08x\n", size, p, *addr);
>                 return false;
>         }
> --
> 2.30.2
>

Re: [PATCH] drm/nouveau: fix off by one in BIOS boundry checking

[Karol Herbst]

On Sat, Jan 22, 2022 at 11:44 AM Nick Lopez <github@glowingmonkey.org> wrote:
>
> Bounds checking when parsing init scripts embedded in the BIOS reject
> access to the last byte. This causes driver initialization to fail on
> Apple eMac's with GeForce 2 MX GPUs, leaving the system with no working
> console.
>
> This is probably only seen on OpenFirmware machines like PowerPC Macs
> because the BIOS image provides by OF is only the used parts of the ROM,
> not a power-of-two blocks read from PCI directly so PCs always have
> empty bytes at the end that are never accesseed.
>

small typo nitpicks: provided and accessed

Also, I think it makes sense to add

Fixes: 4d4e9907ff572 "drm/nouveau/bios: guard against out-of-bounds
accesses to image"
Cc: <stable@vger.kernel.org> # v4.10+

so it gets automatically backported to applicable stable kernels

Anyway, whoever picks the patch can make those adjustments as well.

Reviewed-by: Karol Herbst <kherbst@redhat.com>


> Signed-off-by: Nick Lopez <github@glowingmonkey.org>
> ---
>  drivers/gpu/drm/nouveau/nvkm/subdev/bios/base.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/base.c b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/base.c
> index d0f52d59fc2f..64e423dddd9e 100644
> --- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/base.c
> +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/base.c
> @@ -38,7 +38,7 @@ nvbios_addr(struct nvkm_bios *bios, u32 *addr, u8 size)
>                 *addr += bios->imaged_addr;
>         }
>
> -       if (unlikely(*addr + size >= bios->size)) {
> +       if (unlikely(*addr + size > bios->size)) {
>                 nvkm_error(&bios->subdev, "OOB %d %08x %08x\n", size, p, *addr);
>                 return false;
>         }
> --
> 2.30.2
>