[Bug 284587] integer wrap and invalid read in scsi_sa saloadtimeouts()

[email protected] Wed, 05 Feb 2025 19:52:05 +0000
Newsgroups gmane.os.freebsd.devel.scsi
Message-ID <[email protected]/bugzilla/>
https://bugs.freebsd.org/bugzilla/show_bug.cgi?id=3D284587

Warner Losh <[email protected]> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |[email protected],
                   |                            |[email protected]

--- Comment #1 from Warner Losh <[email protected]> ---
I'll note that we completely believe the device's data in a huge number of
places. And a bunch of other places we basically trust it, but with minimal
guardrails for out-of-bounds access.

But in this case, I think that
        if ((avail_len !=3D 0)
         && (avail_len > valid_len)) {
                xpt_print(softc->periph->path, "WARNING: available timeout "
                    "descriptor len %zu > valid len %u\n",
avail_len,valid_len);
        }

is wrong. First, it should bail entirely on the timeout descriptor (but the=
re's
no good way to pass a failure here back. So 'return;' seems missing.

Second, (avail_len =3D=3D 0 || avail_len > valid_len) should be the test.  =
If hdr->
is -4 (or rather 0xfffb) then we'll be zero. If it's a bigger negative numb=
er,
the other part of the if will trigger since things are unsigned....

The td->length tests as well as changing them also need some better care,
especially when it is updating used_len as you noted...

Adding ken@ for comments as well. But this has to be just the tiny tip of t=
he
iceberg.

--=20
You are receiving this mail because:
You are the assignee for the bug.=