Page MenuHomeFreeBSD

linuxkpi: Fix __sg_alloc_table_from_pages loop
ClosedPublic

Authored by ashafer_badland.io on Apr 17 2023, 8:17 PM.
Tags
None
Referenced Files
Unknown Object (File)
Sat, Oct 5, 1:55 PM
Unknown Object (File)
Fri, Oct 4, 2:04 AM
Unknown Object (File)
Thu, Oct 3, 3:31 PM
Unknown Object (File)
Tue, Oct 1, 11:40 AM
Unknown Object (File)
Tue, Oct 1, 4:11 AM
Unknown Object (File)
Tue, Sep 24, 4:05 PM
Unknown Object (File)
Sat, Sep 21, 11:17 PM
Unknown Object (File)
Sat, Sep 21, 4:10 PM

Details

Summary

Commit 3e0856b63fe0e375a0951e05c2ef98bb2ebd9421 updated
__sg_alloc_table_from_pages to use the same API as linux, but modified
the loop condition when going over the pages in a sg list. Part of the
change included moving the sg_next call out of the for loop and into the
body, which causes an off by one error when traversing the list. Since
sg_next is called before the loop body it will skip the first element
and read one past the last element.

This caused panics when running PRIME with nvidia-drm as the off-by-one
issue causes a NULL dereference.

Diff Detail

Repository
rG FreeBSD src repository
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 50974
Build 47865: arc lint + arc unit

Event Timeline

bz added reviewers: linuxkpi, manu, dumbbell.
bz added a subscriber: bz.

Good catch and likely something which may have been bugging me as well on other work.

sys/compat/linuxkpi/common/include/linux/scatterlist.h
382

I wonder at the same time why we switched from using for_each_sg() in first place as that would have avoided the problem, but I am probably too tired to see that...

This revision is now accepted and ready to land.Apr 17 2023, 8:36 PM
sys/compat/linuxkpi/common/include/linux/scatterlist.h
382

I think the intention was to add a check of the value of s, but I thought it looked doable to do that while still using for_each_sg(). I'm not 100% sure either.

This revision was automatically updated to reflect the committed changes.