Skip to content

Fix out-of-bounds read in shuftiDoubleExecReal tail handling#381

Open
Jongy wants to merge 1 commit intoVectorCamp:developfrom
Jongy:fix-shufti-double-overread
Open

Fix out-of-bounds read in shuftiDoubleExecReal tail handling#381
Jongy wants to merge 1 commit intoVectorCamp:developfrom
Jongy:fix-shufti-double-overread

Conversation

@Jongy
Copy link

@Jongy Jongy commented Mar 19, 2026

Commit 9e9a10a ("Fix double shufti's vector end false positive", #325) changed the tail code in shuftiDoubleExecReal to read a full vector from the current pointer (loadu(d)), which can overread past buf_end by up to S-1 bytes. When the buffer ends at a page boundary followed by unmapped memory, this causes a SIGSEGV.

Fix by reading the last S bytes backward from buf_end (matching the approach used in shuftiExecReal), and falling back to memcpy for buffers shorter than one vector width.


Full disclosure - this change is entirely AI generated (but I understand the bug it tries to fix :) So I'm unsure if this is the correct way to fix it, I'm not acquainted with this project enough. Feel free to overtake this PR, or to close it and fix it someway else.

We've hit this crash in one of our apps after upgrading to 5.4.12 and this seems to be the root cause. Due to the bug's nature I wasn't able to verify that this fix eliminates the problem (this is statistical, depends on whether we try to scan buffers that happen to end on a page boundary followed by an unmapped page).
The added test segfaults without the fix, and the code bytes provided by the kernel match the code bytes the kernel reported when our app crashed.

Mar 18 23:20:13 yonatan kernel: unit-internal[3411287]: segfault at 716770493000 ip 00005b9365d6b839 sp 00007ffc46d84d80 error 4 in unit-internal[47b839,5b936596d000+7df000] likely on CPU 3 (core 12, socket 0)
Mar 18 23:20:13 yonatan kernel: Code: ff e9 00 01 00 00 0f 1f 00 c5 cd 76 f6 48 39 fa 0f 84 1b 01 00 00 48 b8 0f 0f 0f 0f 0f 0f 0f 0f c4 e1 f9 6e c0 c4 e2 7d 59 c0 <c5> fd df 22 c5 fd db 02 c5 dd 73 d4 04 c4 62 2d 00 d0 c4 e2 75 00

Commit 9e9a10a ("Fix double shufti's vector end false positive",
VectorCamp#325) changed the tail code in shuftiDoubleExecReal to read a full
vector from the current pointer (loadu(d)), which can overread past
buf_end by up to S-1 bytes. When the buffer ends at a page boundary
followed by unmapped memory, this causes a SIGSEGV.

Fix by reading the last S bytes backward from buf_end (matching the
approach used in shuftiExecReal), and falling back to memcpy for
buffers shorter than one vector width.

Signed-off-by: Yonatan Goldschmidt <yonatan.goldschmidt@wiz.io>
@markos
Copy link

markos commented Mar 19, 2026

hi, this seems to be a duplicate of #368. I will release 5.4.13 by the end of this month. However, adding the extra unit test is not a bad idea. For small pieces of code, AI assistance is not a problem, if it was a whole component that would be a different issue and I would probably not accept it.

@markos markos added the duplicate This issue or pull request already exists label Mar 19, 2026
@Jongy
Copy link
Author

Jongy commented Mar 19, 2026

I'm not sure #368 fixes the bug. I cherry-picked this commit on top of that PR (1898ab9d) and reverted the fix in shuftiDoubleExecReal, and the test crashes

@markos
Copy link

markos commented Mar 19, 2026

ok, thanks for the extra test. I will check it myself this week.

@Jongy
Copy link
Author

Jongy commented Mar 19, 2026

Great, thanks!

@Jongy Jongy marked this pull request as ready for review March 19, 2026 09:40
@ypicchi-arm
Copy link

I've not reviewed #368 yet, but I did review this one and it looks good to me. Thanks for catching that mistake.
I've not trying building and running the patch, but assuming it build and run through the unit tests, I'm happy for it to be merged.

@byeonguk-jeong
Copy link

It is not a duplicate of #368.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

duplicate This issue or pull request already exists

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants