Re: git: 40e0fa10f58d - main - Check alignment of fp in unwind_frame

From: Brooks Davis <brooks_at_freebsd.org>
Date: Tue, 15 Nov 2022 17:00:48 UTC
Sorry, that's entierly on me.  I should have done the trivial test.

-- Brooks

On Tue, Nov 15, 2022 at 12:42:03PM +0000, Andrew Turner wrote:
> I???ve reverted for now & reopened the Phabricator review.
> 
> Andrew
> 
> > On 15 Nov 2022, at 08:22, Mateusz Guzik <mjguzik@gmail.com> wrote:
> > 
> > this fails to build:
> > 
> > /usr/src/sys/riscv/riscv/unwind.c:50:7: error: implicit declaration of
> > function 'is_aligned' is invalid in C99
> > [-Werror,-Wimplicit-function-declaration]
> >        if (!is_aligned(fp, sizeof(fp)) ||
> >             ^
> > 1 error generated.
> > 
> > On 11/15/22, Brooks Davis <brooks@freebsd.org> wrote:
> >> The branch main has been updated by brooks:
> >> 
> >> URL:
> >> https://cgit.FreeBSD.org/src/commit/?id=40e0fa10f58d90744c2857b57adf0ddbce1a1e1c
> >> 
> >> commit 40e0fa10f58d90744c2857b57adf0ddbce1a1e1c
> >> Author:     Dapeng Gao <dapeng@dpgao.cn>
> >> AuthorDate: 2022-11-15 00:21:38 +0000
> >> Commit:     Brooks Davis <brooks@FreeBSD.org>
> >> CommitDate: 2022-11-15 00:25:46 +0000
> >> 
> >>    Check alignment of fp in unwind_frame
> >> 
> >>    A misaligned frame pointer is certainly not a valid frame pointer and
> >>    with strict alignment enabled (as on CHERI) can cause panics when it is
> >>    loaded from later in the code.
> >> 
> >>    Reviewed By:    jhb
> >>    Differential Revision: https://reviews.freebsd.org/D34646
> >> ---
> >> sys/arm64/arm64/unwind.c | 3 ++-
> >> sys/riscv/riscv/unwind.c | 3 ++-
> >> 2 files changed, 4 insertions(+), 2 deletions(-)
> >> 
> >> diff --git a/sys/arm64/arm64/unwind.c b/sys/arm64/arm64/unwind.c
> >> index 470b64c00540..81431e109494 100644
> >> --- a/sys/arm64/arm64/unwind.c
> >> +++ b/sys/arm64/arm64/unwind.c
> >> @@ -41,7 +41,8 @@ unwind_frame(struct thread *td, struct unwind_state
> >> *frame)
> >> 
> >> 	fp = frame->fp;
> >> 
> >> -	if (!kstack_contains(td, fp, sizeof(uintptr_t) * 2))
> >> +	if (!is_aligned(fp, sizeof(fp)) ||
> >> +	    !kstack_contains(td, fp, sizeof(fp) * 2))
> >> 		return (false);
> >> 
> >> 	/* FP to previous frame (X29) */
> >> diff --git a/sys/riscv/riscv/unwind.c b/sys/riscv/riscv/unwind.c
> >> index 9efb1fef9451..a66ffebcdc35 100644
> >> --- a/sys/riscv/riscv/unwind.c
> >> +++ b/sys/riscv/riscv/unwind.c
> >> @@ -47,7 +47,8 @@ unwind_frame(struct thread *td, struct unwind_state
> >> *frame)
> >> 
> >> 	fp = frame->fp;
> >> 
> >> -	if (!kstack_contains(td, fp - sizeof(fp) * 2, sizeof(fp) * 2))
> >> +	if (!is_aligned(fp, sizeof(fp)) ||
> >> +	    !kstack_contains(td, fp - sizeof(fp) * 2, sizeof(fp) * 2))
> >> 		return (false);
> >> 
> >> 	frame->sp = fp;
> >> 
> > 
> > 
> > -- 
> > Mateusz Guzik <mjguzik gmail.com>
> > 
>