Re: [PATCH 6/6] netfilter: nft_set_pipapo_avx2: add missing vzeroupper
Stefano Brivio <[email protected]>
| Newsgroups | org.kernel.vger.linux-raid,org.kernel.vger.linux-crypto,org.kernel.vger.linux-kernel,org.kernel.vger.netfilter-devel,org.kernel.vger.stable |
|---|---|
| Organization | Red Hat |
| Message-ID | <20260817131738.51554efe@elisabeth> |
On Sun, 16 Aug 2026 10:15:18 -0700 Eric Biggers <[email protected]> wrote: > On Sun, Aug 16, 2026 at 12:38:41PM +0200, Stefano Brivio wrote: > > Eric, thanks for taking care of this. > > > > The patch looks good to me, I just have two questions: > > > > On Sat, 15 Aug 2026 13:57:50 -0700 > > Eric Biggers <[email protected]> wrote: > > > > > Since pipapo_get_avx2() uses YMM registers, execute vzeroupper before > > > returning from it. This is needed to avoid degrading the performance of > > > any later SSE code that may happen to be executed. > > > > Out of curiosity: was this prompted by some observed latency spike in > > execution of SSE code, or it's just meant to satisfy the recommendation > > from AMD and Intel to use it while transitioning from AVX to SSE modes? > > This one was found by code review. But the latency spike has been > observed in other cases with missing vzeroupper, so it's definitely a > real effect at least on some CPUs. Ah, interesting, I've been wondering about that. > > > Fixes: 7400b063969b ("nft_set_pipapo: Introduce AVX2-based lookup implementation") > > > Cc: [email protected] > > > > Is this really stable material? Skipping vzeroupper might have a > > performance impact, but it's not an issue for correctness. > > > > The main reason why I'm asking is that, while vzeroupper might look > > harmless and obviously safe, it actually caused CVE-2023-20593 > > ("ZenBleed") on AMD Zen 2. > > > > I expect systems receiving stable kernel updates to also run the > > patched microcode by now, so I'm not overly concerned in any case. > > It's awkward to mark something as a fix, then not actually want it to be > fixed everywhere. The stable maintainers know this, and they often > apply fixes anyway regardless of 'Cc stable'. Right, I guess you have point there. > And if vzeroupper is really not safe due to ZenBleed, then why is it > used everywhere else in the kernel? This is just one of the few > exceptions that doesn't have it. If it's not safe then they should all > be alternatives that patch them out to no-ops on affected CPUs. > > But since that was never implemented, and userspace almost always uses > vzeroupper too, it seems the only real solution is the microcode. I > don't think it makes sense to have a middle ground where almost all of > userspace uses vzeroupper, ~97% of the kernel uses vzeroupper, and a > random 3% of the kernel doesn't use it. Don't get me wrong, I wasn't advocating against "fixing" this for specific paths, rather just pointing out that something seemingly harmless such as vzeroupper can have nasty side effects. I used ZenBleed as a mere example, not as a current reason to do or not do anything specific. -- Stefano