Page MenuHomeFreeBSD
Differential D41485

arm64 makectx: Fix overflow of tf_x array
ClosedPublic
Actions

Authored by jhb on Aug 16 2023, 7:30 PM.
Tags
None
Referenced Files
F102190735: D41485.diff
Fri, Nov 8, 5:53 PM
Unknown Object (File)
Fri, Nov 1, 1:33 AM
Unknown Object (File)
Fri, Nov 1, 1:33 AM
Unknown Object (File)
Fri, Nov 1, 1:32 AM
Unknown Object (File)
Fri, Nov 1, 1:31 AM
Unknown Object (File)
Fri, Nov 1, 1:31 AM
Unknown Object (File)
Fri, Nov 1, 1:16 AM
Unknown Object (File)
Fri, Oct 25, 5:54 PM
View All 50 Files
Subscribers
emaste
imp

Details

Reviewers
andrew
markj
jrtc27
manu
Commits
rG91d0876a20ce: arm64 makectx: Fix overflow of tf_x array
Summary

PCB_LR isn't stored in tf_x, so trying to store it as pcb_x[PCB_LR] =
tf->tf_x[PCB_LR + PCB_X_START] overflowed the tf_x array.

Reported by: Morello (bounds check crash)
Obtained from: CheriBSD
Sponsored by: DARPA

Diff Detail

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

Event Timeline

jhb created this revision.Aug 16 2023, 7:30 PM
Herald added a reviewer: manu. · View Herald TranscriptAug 16 2023, 7:30 PM
Herald added subscribers: emaste, imp. · View Herald Transcript
jhb requested review of this revision.Aug 16 2023, 7:30 PM
Harbormaster completed remote builds in B53136: Diff 126091.Aug 16 2023, 7:30 PM
jhb added a comment.Aug 16 2023, 7:31 PM

An alternate fix (that might be cleaner) without breaking the KBI would be to rename pcb_x[PCB_LR] back to pcb_lr.

jrtc27 added inline comments.Aug 16 2023, 7:34 PM
sys/arm64/arm64/machdep.c
363

Copying my comment on the CheriBSD PR: could move the tf->tf_elr read in here?

jrtc27 added a comment.Aug 16 2023, 7:46 PM
In D41485#944895, @jhb wrote:

An alternate fix (that might be cleaner) without breaking the KBI would be to rename pcb_x[PCB_LR] back to pcb_lr.

Eh, assembly relies on them being adjacent and LR is an X register

markj accepted this revision.Aug 16 2023, 8:09 PM
This revision is now accepted and ready to land.Aug 16 2023, 8:09 PM
jhb added inline comments.Aug 16 2023, 9:01 PM
sys/arm64/arm64/machdep.c
363

Yeah, that's not a bad idea, and allows the comment to be moved so all the PCB_LR handling is in one place.

jhb updated this revision to Diff 126099.Aug 16 2023, 11:00 PM

Simplify

This revision now requires review to proceed.Aug 16 2023, 11:00 PM
Harbormaster completed remote builds in B53141: Diff 126099.Aug 16 2023, 11:00 PM
jrtc27 added inline comments.Aug 17 2023, 12:18 AM
sys/arm64/arm64/machdep.c
365

My personal taste would be to use i here rather than propagating the constant, as the check's there to change how you read from tf, not how you store to pcb_x.

jhb updated this revision to Diff 126144.Aug 17 2023, 6:33 PM

Tweak

Harbormaster completed remote builds in B53158: Diff 126144.Aug 17 2023, 6:34 PM
jhb marked an inline comment as done.Aug 17 2023, 6:39 PM
jrtc27 accepted this revision.Aug 17 2023, 7:40 PM
This revision is now accepted and ready to land.Aug 17 2023, 7:40 PM
andrew accepted this revision.Aug 17 2023, 7:55 PM
Closed by commit rG91d0876a20ce: arm64 makectx: Fix overflow of tf_x array (authored by jhb). · Explain WhyAug 17 2023, 10:27 PM
This revision was automatically updated to reflect the committed changes.
jhb added a commit: rG91d0876a20ce: arm64 makectx: Fix overflow of tf_x array.

Revision Contents
Changeset List

PathSize
sys/
arm64/
arm64/
11 lines

Diff 126163

View Options

sys/arm64/arm64/machdep.c

Loading...