Page MenuHomeFreeBSD

linuxkpi: Fix iteration in __sg_alloc_table_from_pages again
ClosedPublic

Authored by ashafer_badland.io on Aug 24 2023, 1:51 PM.
Tags
None
Referenced Files
Unknown Object (File)
Wed, Nov 6, 4:40 PM
Unknown Object (File)
Wed, Oct 16, 2:22 PM
Unknown Object (File)
Mon, Oct 14, 7:33 PM
Unknown Object (File)
Mon, Oct 14, 7:33 PM
Unknown Object (File)
Mon, Oct 14, 7:04 AM
Unknown Object (File)
Sun, Oct 13, 6:34 AM
Unknown Object (File)
Sat, Oct 12, 7:27 AM
Unknown Object (File)
Thu, Oct 10, 11:32 PM

Details

Summary

Commit 3f686532c9b4 tried to fix an issue with not properly starting
at the first page in the sg list to prevent a panic. This worked but
with the side effect of incrementing "s" during the final iteration,
causing it to be NULL since the list had ended.

This also causes a panic with drm-5.15, since s is null when we later
pass it to sg_mark_end. This change decouples the iteration sg from
s so that s is never incremented past the final page in the chain.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

This is part of a fix for https://github.com/amshafer/nvidia-driver/issues/20

I don't think I noticed this problem with my fix the first time since the sg_mark_end(s) call is only hit in drm 5.15, I'm guessing I tested with 5.10.

bz added inline comments.
sys/compat/linuxkpi/common/include/linux/scatterlist.h
384

Please make this a C-style comment. /* .. */

bz added subscribers: wulf, manu, dumbbell.

@dumbbell @manu @wulf unless someone else will review it, I'll likely commit it the next days so it doesn't get lost.

@ashafer_badland.io whe you suggest in your commit message that we panic later passing a NULL, I assume you only test with a non-debug kernel and never hit the KASSERT after the loop which should catch exactly that?

In D41574#951132, @bz wrote:

@dumbbell @manu @wulf unless someone else will review it, I'll likely commit it the next days so it doesn't get lost.

@ashafer_badland.io whe you suggest in your commit message that we panic later passing a NULL, I assume you only test with a non-debug kernel and never hit the KASSERT after the loop which should catch exactly that?

Sorry, forgot about this review, please do commit it so it's in 14.0

This revision is now accepted and ready to land.Sep 6 2023, 6:57 AM

Off the top of my head I thought I tested with GENERIC but because that KASSERT didn't fire I'm assuming I had GENERIC-NODEBUG installed, which I normally do on half my machines.