Re: [RFC 1/1] swiotlb: Replace BUG_ON() with graceful error handling

[Brian Johannesmeyer]

Thank you for your feedback and your patience. Apologies for the
delayed response --- I've had some personal matters arise in the past
couple weeks.

I had initially prepared responses addressing each of your points, but
after revisiting Robin's insightful observation below, it seems this
issue has already been resolved in the latest kernel. While I’m still
happy to address any further questions or comments, it appears that
additional discussion may no longer be necessary.

On Mon, Nov 25, 2024 at 8:03 AM Robin Murphy <robin.murphy@xxxxxxx> wrote:
> Hmm, looking again, how exactly *does* this happen? To get here from
> swiotlb_unmap_single(), swiotlb_find_pool() has already determined that
> "tlb_addr" is within the range belonging to "mem", so if it is somehow
> possible for it to then convert into an out-of-bounds index, maybe that
> does actually imply some bug in SWIOTLB itself where "mem" is
> misconfigured...

Great observation. Upon reviewing the latest kernel, I realized I had
analyzed an older version of the code that lacked commit [0]. This
commit introduces swiotlb_find_pool() into swiotlb_tbl_unmap_single(),
effectively mitigating the issue. Specifically, if addr is corrupted,
swiotlb_find_pool() would return NULL, causing
swiotlb_tbl_unmap_single() to now return without invoking
__swiotlb_tbl_unmap_single(). Consequently, the failed BUG_ON in
swiotlb_release_slots() is avoided. My apologies for overlooking this
recent change and not verifying the code path in the latest kernel.

That said, converting the BUG_ON to a WARN_ON_ONCE might still be a
useful improvement, even if the immediate issue is resolved.

Sincerely,

Brian Johannesmeyer

[0] commit 7296f2301a057493e97b07739213c6e864f76891 ("swiotlb: reduce
swiotlb pool lookups")