If it’s correct I’d like to see it make its way upstream. I have no idea what the submission procedure is.

From: Ilia Mirkin
Sent: Thursday, January 20, 2022 11:24 AM
To: Nick Lopez
Cc: nouveau@lists.freedesktop.org
Subject: Re: [Nouveau] Off-by-one or bad BIOS image? Apple eMac 800 GeForce 2MX

This stuff is always so confusing. Let's think this through.... if
bios size is 4, and we're trying to read a 4-byte thing starting at
address 0, that _ought_ to work, I think. So in my strawman case,
bios->size == 4, and size == 4. So we should only error if size >
bios->size, not if they're ==. Looks like your patch is right.

Want to make a linux kernel patch submission with this? (i.e.
including changelog, signoff, etc?)

Cheers,

  -ilia

On Thu, Jan 20, 2022 at 1:17 PM Nick Lopez <nick@glowingmonkey.org> wrote:
>
> Because I watch too much retro YouTube I decided it was a good idea to try installing Adelie Linux on my old G4/800 eMac, but the Live installer would freeze. By blacklisting nouveau I was able to get it booted and manually installed and, after hours and hours of compiling, get a working kernel tree to poke at. After only a few iterations with dump_stack() and nvkm_debug and the output of envytools/nvbios I worked out at the last initscript instruction was stored in the last byte of the ROM. I think the bounds check in the nvbios_addr() function is miscalculating the limit as one byte short, that’s why I was seeing this in the syslog:
>
>
>
> nouveau 0000:00:10.0: bios: OOB 1 000007b2 000007b2
>
> nouveau 0000:00:10.0: devinit: 0x000007b2[ ]: unknown opcode 0x00
>
> nouveau 0000:00:10.0: preinit failed with -22
>
> nouveau: DRM-master:00000000:00000080: init failed with -22
>
> nouveau 0000:00:10.0: DRM-master: Device allocation failed: -22
>
> nouveau: probe of 0000:00:10.0 failed with error -22
>
>
>
> After I changed the limit check from:
>
> if (unlikely(*addr + size >= bios->size)) {
>
> to:
>
> if (unlikely(*addr + size > bios->size)) {
>
>
>
> it initialized the card properly, brought up the fbconsole and even seems to be working in X with DRI. So the question is: was the bounds check wrong, or is the NVDA,BMP image provided by OpenFirmware truncated? I’m guess this doesn’t turn up elsewhere because the ROM images read through any of the other methods are the size of flash chip they’re stored on so there’s always unused space at the end and they never use the last byte where the NVDA,BMP provided by OpenFirmware is just the active section.
>
>
>
> The patch is against the Adelie easy-kernel patch 5.4 tree, but it looks like that code is still there in the current upstream torvalds/linux git.