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
Unknown Object (File)
Thu, Jul 3, 7:41 PM
Unknown Object (File)
Thu, Jul 3, 4:12 AM
Unknown Object (File)
Fri, Jun 27, 10:37 AM
Unknown Object (File)
Wed, Jun 25, 12:54 PM
Unknown Object (File)
Tue, Jun 24, 1:42 PM
Unknown Object (File)
Sun, Jun 22, 5:09 PM
Unknown Object (File)
Sun, Jun 22, 10:54 AM
Unknown Object (File)
Tue, Jun 17, 12:46 PM
View All 88 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 Skipped
Unit
Tests Skipped
Build Status
Buildable 53136
Build 50027: arc lint + arc unit

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
366

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
366

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
373

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/
5 lines

Diff 126091

View Options

sys/arm64/arm64/machdep.c

Loading...