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)
Sun, Sep 13, 9:00 AM
Unknown Object (File)
Thu, Sep 10, 9:52 PM
Unknown Object (File)
Wed, Sep 9, 3:26 AM
Unknown Object (File)
Sun, Sep 6, 11:58 PM
Unknown Object (File)
Sun, Sep 6, 7:25 PM
Unknown Object (File)
Sun, Sep 6, 2:15 PM
Unknown Object (File)
Sun, Sep 6, 2:10 PM
Unknown Object (File)
Sun, Sep 6, 1:48 PM
View All 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...