Re: [PATCH 2/4] hw/misc/bcm2835_property: Avoid overflow in OTP access properties

[Michael Tokarev]

02.08.2024 10:02, Michael Tokarev wrote:
> 23.07.2024 16:10, Peter Maydell wrote:
> > Coverity points out that in our handling of the property
> > RPI_FWREQ_SET_CUSTOMER_OTP we have a potential overflow.  This
> > happens because we read start_num and number from the guest as
> > unsigned 32 bit integers, but then the variable 'n' we use as a loop
> > counter as we iterate from start_num to start_num + number is only an
> > "int".  That means that if the guest passes us a very large start_num
> > we will interpret it as negative.  This will result in an assertion
> > failure inside bcm2835_otp_set_row(), which checks that we didn't
> > pass it an invalid row number.
> >
> > A similar issue applies to all the properties for accessing OTP rows
> > where we are iterating through with a start and length read from the
> > guest.
>
> This is a fun one wrt the -stable series.
>
> The code which is mentioned in the subject and above (OTP access
> properties) is introduced in v9.0.0-1812-g5d5f1b60916a " hw/misc:Implement
> mailbox properties for customer OTP and device specific private keys",
> which is not in any released version of qemu.  However, the next comment
> ("A similar issue..") tells us the same prob exists in all other
> cases in the same function.  So the fix mentioned in subject does not
> apply to -stable, while "all others" "side-fix" does :)

Okay, there's no "all other" case here, it is really all about OTP access.
This change basically only removes the 'n' variable for stable-9.0, which
isn't used since the previous patch anyway, and the build fails after the
previous patch due to this.

Still fun, but in a different way :)

I'll add removal of this `n' variable to the previous patch for -stable.

Thanks,

/mjt