linux-hardening - Re: How large is the xtensa pt_regs::areg array supposed to be?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAMo8BfKQuG4JarZEcB9eT3B3DOutyioq2h-VQuVZ05KrwD6Mnw@mail.gmail.com>
Date:   Thu, 3 Feb 2022 16:05:45 -0800
From:   Max Filippov <jcmvbkbc@...il.com>
To:     Kees Cook <keescook@...omium.org>
Cc:     Chris Zankel <chris@...kel.net>,
        "open list:TENSILICA XTENSA PORT (xtensa)" 
        <linux-xtensa@...ux-xtensa.org>, linux-hardening@...r.kernel.org
Subject: Re: How large is the xtensa pt_regs::areg array supposed to be?

On Thu, Feb 3, 2022 at 1:56 PM Kees Cook <keescook@...omium.org> wrote:
> > > What should be happening here?
> >
> > pt_regs::areg is the current register window, but when we enter
> > the kernel from the userspace additional valid registers up the call
> > stack are saved here:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/xtensa/kernel/entry.S?h=v5.16#n204
> > This is done because when the kernel is built with windowed ABI
> > we cannot have a mix of user and kernel registers in the physical
> > register file.
>
> This is a bit opaque for me to read, but it looks like it's a loop of
> 4-at-a-time of the "extra" registers?

Correct. That's because the register window may move 4, 8 or 12
registers at a time, so it's always a multiple of 4 registers.

> > The space for these registers is reserved here:
> > https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/xtensa/kernel/entry.S?h=v5.16#n2102
> > by adding PT_REGS_OFFSET, which accounts for the
> > XCHAL_NUM_AREGS value, to the task stack address.
>
> Okay, so it seems like there are two "views" of the registers from an
> ABI perspective, the userspace view (struct pt_regs), and the kernel
> view which is struct pt_regs + more.

I'd say that's the different kinds of stack frames on the kernel stack.
I don't think we expose the structure itself to the userspace anymore.

> > For some reason during xtensa port development it was chosen
> > to have pt_regs::areg only cover the current register window. I guess
> > this was done as a common denominator for the user and kernel
> > stack frames and to avoid an exposure of XCHAL_NUM_AREGS
> > constant in the user-visible headers. Chris may have additional details.
>
> Right, that makes sense: pt_regs should be the shared user/kernel view.
>
> The compiler is mad about trying to access the extra registers from the
> pt_reg struct, so maybe a kernel-only struct could be used here?

I agree. Let me try to do this change.

-- 
Thanks.
-- Max