Timur Tabi
2022-May-11 16:37 UTC
[Nouveau] [PATCH] drm/nouveau: fix another off-by-one in nvbios_addr
This check determines whether a given address is part of
image 0 or image 1. Image 1 starts at offset image0_size,
so that address should be included.
Fixes: 4d4e9907ff572 ("drm/nouveau/bios: guard against out-of-bounds
accesses to image")
Signed-off-by: Timur Tabi <ttabi at nvidia.com>
---
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 64e423dddd9e..6c318e41bde0 100644
--- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/base.c
+++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/base.c
@@ -33,7 +33,7 @@ nvbios_addr(struct nvkm_bios *bios, u32 *addr, u8 size)
{
u32 p = *addr;
- if (*addr > bios->image0_size && bios->imaged_addr) {
+ if (*addr >= bios->image0_size && bios->imaged_addr) {
*addr -= bios->image0_size;
*addr += bios->imaged_addr;
}
--
2.36.0
Karol Herbst
2022-May-11 16:41 UTC
[Nouveau] [PATCH] drm/nouveau: fix another off-by-one in nvbios_addr
On Wed, May 11, 2022 at 6:37 PM Timur Tabi <ttabi at nvidia.com> wrote:> > This check determines whether a given address is part of > image 0 or image 1. Image 1 starts at offset image0_size, > so that address should be included. > > Fixes: 4d4e9907ff572 ("drm/nouveau/bios: guard against out-of-bounds accesses to image") > Signed-off-by: Timur Tabi <ttabi at nvidia.com> > --- > 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 64e423dddd9e..6c318e41bde0 100644 > --- a/drivers/gpu/drm/nouveau/nvkm/subdev/bios/base.c > +++ b/drivers/gpu/drm/nouveau/nvkm/subdev/bios/base.c > @@ -33,7 +33,7 @@ nvbios_addr(struct nvkm_bios *bios, u32 *addr, u8 size) > { > u32 p = *addr; > > - if (*addr > bios->image0_size && bios->imaged_addr) { > + if (*addr >= bios->image0_size && bios->imaged_addr) { > *addr -= bios->image0_size; > *addr += bios->imaged_addr; > } > -- > 2.36.0 >Reviewed-by: Karol Herbst <kherbst at redhat.com> I don't like how this could fail for values spanning both images, but at least this looks more correct. I think in the future we should keep a copy without that gap on the host side to make access to the vbios image cheaper. What do you say Ben?