Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
Pre-existing issues: - [High] Latent NULL pointer dereference in `pvr_mmu_op_context_unmap_curr_page()` when unmapping missing page tables, due to `-ENXIO` being improperly swallowed. -- commit 9512cb5ae9264ab1bc6ea5bf3b2a374fe88ffc0e Author: Brajesh Gupta <[email protected]> drm/imagination: Propagate map failures correctly from pvr_mmu_map_sgl() This commit ensures that map failures from pvr_mmu_map_sgl() are correctly returned to the caller, preventing the pvr_mmu_map() interface from continuing incorrectly. It also adds logging for page unmapping failures during cleanup. > diff --git a/drivers/gpu/drm/imagination/pvr_mmu.c > b/drivers/gpu/drm/imagination/pvr_mmu.c > index 3cac482e10347..175f0ba4d993f 100644 > --- a/drivers/gpu/drm/imagination/pvr_mmu.c > +++ b/drivers/gpu/drm/imagination/pvr_mmu.c [ ... ] > @@ -2553,7 +2554,9 @@ pvr_mmu_map_sgl(struct pvr_mmu_op_context *op_ctx, > struct scatterlist *sgl, > > err_destroy_pages: > memcpy(&op_ctx->curr_page, &ptr_copy, sizeof(op_ctx->curr_page)); > - err = pvr_mmu_op_context_unmap_curr_page(op_ctx, page); > + if (pvr_mmu_op_context_unmap_curr_page(op_ctx, page)) [Severity: High] This is a pre-existing issue, but could this unmap call trigger a NULL pointer dereference under certain conditions? If an unmap operation spans an L1 boundary into an unmapped area where the L0 table is missing, pvr_mmu_op_context_next_page() clears op_ctx->curr_page.l0_table to NULL and calls pvr_mmu_op_context_load_tables(). In pvr_mmu_op_context_load_tables(), the -ENXIO error indicating a missing L0 table is swallowed: if (err == -ENXIO) err = 0; Because it returns 0 instead of -ENXIO, the loop inside pvr_mmu_op_context_unmap_curr_page() misses its guard: err = pvr_mmu_op_context_next_page(op_ctx, false); if (err == -ENXIO) continue; else if (err) return err; pvr_page_destroy(op_ctx); This causes it to unconditionally proceed to pvr_page_destroy(), which then dereferences the NULL l0_table when checking if the entry is valid. Since this patch modifies the error handling around unmapping pages, is there a way to address the swallowed -ENXIO so that sparse unmap requests or rollbacks that cross into a missing L0 table skip properly instead of crashing? > + drm_err(from_pvr_device(op_ctx->mmu_ctx->pvr_dev), "%s : > Failure in unmapping pages\n", > + __func__); > > return err; > } -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
