linux-kernel - Re: [PATCH v2 6/6] riscv: vector: initialize vlenb on the first context switch

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] [day] [month] [year] [list]

Message-ID: <CAFTtA3NW=8JDnr=JN2X9q9YwYLihsF3zE06VTGSzg7kVjDLJZQ@mail.gmail.com>
Date: Tue, 21 Oct 2025 17:07:55 -0500
From: Andy Chiu <andybnac@...il.com>
To: Sergey Matyukevich <geomatsi@...il.com>
Cc: linux-riscv@...ts.infradead.org, linux-kselftest@...r.kernel.org, 
	linux-kernel@...r.kernel.org, Paul Walmsley <pjw@...nel.org>, 
	Palmer Dabbelt <palmer@...belt.com>, Albert Ou <aou@...s.berkeley.edu>, 
	Alexandre Ghiti <alex@...ti.fr>, Oleg Nesterov <oleg@...hat.com>, Shuah Khan <shuah@...nel.org>, 
	Jisheng Zhang <jszhang@...nel.org>, Thomas Gleixner <tglx@...utronix.de>, Thomas Huth <thuth@...hat.com>, 
	Charlie Jenkins <charlie@...osinc.com>, Han Gao <rabenda.cn@...il.com>, 
	Samuel Holland <samuel.holland@...ive.com>, Nam Cao <namcao@...utronix.de>, 
	Joel Granados <joel.granados@...nel.org>, Clément Léger <cleger@...osinc.com>, 
	Conor Dooley <conor.dooley@...rochip.com>
Subject: Re: [PATCH v2 6/6] riscv: vector: initialize vlenb on the first
 context switch

On Sun, Oct 19, 2025 at 4:43 PM Sergey Matyukevich <geomatsi@...il.com> wrote:
>
> On Wed, Oct 15, 2025 at 02:54:39PM -0500, Andy Chiu wrote:
> > Hi Sergey,
> >
> > On Tue, Oct 7, 2025 at 6:58 AM Sergey Matyukevich <geomatsi@...il.com> wrote:
> > >
> > > The vstate in thread_struct is zeroed when the vector context is
> > > initialized. That includes read-only register vlenb, which holds
> > > the vector register length in bytes. This zeroed state persists
> > > until mstatus.VS becomes 'dirty' and a context switch saves the
> > > actual hardware values.
> > >
> > > This can expose the zero vlenb value to the user-space in early
> > > debug scenarios, e.g. when ptrace attaches to a traced process
> > > early, before any vector instruction except the first one was
> > > executed.
> > >
> > > Fix this by forcing the vector context save on the first context switch.
> > >
> > > Signed-off-by: Sergey Matyukevich <geomatsi@...il.com>
> > > ---
> > >  arch/riscv/kernel/vector.c | 4 ++++
> > >  1 file changed, 4 insertions(+)
> > >
> > > diff --git a/arch/riscv/kernel/vector.c b/arch/riscv/kernel/vector.c
> > > index 901e67adf576..3dd22a71aa18 100644
> > > --- a/arch/riscv/kernel/vector.c
> > > +++ b/arch/riscv/kernel/vector.c
> > > @@ -120,6 +120,7 @@ static int riscv_v_thread_zalloc(struct kmem_cache *cache,
> > >
> > >         ctx->datap = datap;
> > >         memset(ctx, 0, offsetof(struct __riscv_v_ext_state, datap));
> > > +
> > >         return 0;
> > >  }
> > >
> > > @@ -216,8 +217,11 @@ bool riscv_v_first_use_handler(struct pt_regs *regs)
> > >                 force_sig(SIGBUS);
> > >                 return true;
> > >         }
> > > +
> > >         riscv_v_vstate_on(regs);
> > >         riscv_v_vstate_set_restore(current, regs);
> > > +       set_tsk_thread_flag(current, TIF_RISCV_V_FORCE_SAVE);
> > > +
> >
> > I am afraid that this approach can result in a security issue where a
> > context switch happens before the v-restore part of the current
> > process, cheating the kernel to store stale v-regs onto the current
> > context memory. Please note that this handler is run with irq enabled
> > so preemption is allowed.
> >
> > I would expect simply initializing the vleb in riscv_v_thread_zalloc,
> > perhaps dropping the "z" in the name to prevent confusion.
>
> Ok, so we can just set 'ctx->vlenb = riscv_v_vsize / 32' in the renamed
> riscv_v_thread_alloc function. But note, that w/o forced context save
> we implicitly reset the vector configuration to 'all zeros', overwriting
> the hardware defaults.

Resetting all vregs to zero is desired as otherwise we may
unintentionally leak stale states from other users or the kernel to
the user process.

>
> By the way, could you please elaborate a little bit more about your security
> concerns with the TIF_RISCV_V_FORCE_SAVE approach ? The atomic and per-process
> flag modification looks safe to me, so I'd like to understand what I am
> missing.
>

The concern is information leak. A context switch can happen right
after the FORCE_SAVE bit is set. At this point the kernel saves live
vregs on the machine to the context memory (vstate) of that process.
The content of live registers may come from another process, or stale
value of in-kernel Vector uses, since we don't flush registers at
every ownership change. When we switch back to the original process
and return to the user space, the saved stale content is restored back
to registers. As a result, the user space can read Vector registers
from other contexts.

Thanks,
Andy