On Thu, May 15, 2025 at 11:36:52AM +0200, Borislav Petkov wrote:
On Wed, May 14, 2025 at 04:30:22PM -0700, Pawan Gupta wrote:
This was discussed during the mitigation, and pr_warn() was chosen because it was not obvious that srso mitigation also mitigates retbleed. (On a retrospect, there should have been a comment about it).
Why is that important?
There are 4 mitigations that currently use return thunks, retbleed=stuff(Call Depth Tracking), retbleed=unret, SRSO and ITS. They all set the return thunks they want without checking if return thunks are already set by another mitigation. I understand the SRSO mitigates retbleed(BTC), but the same is not true for retbleed(RSB underflow mitigated by CDT) and ITS. If ITS overrides CDT return thunk, it will make CDT ineffective.
We have multiple cases where a mitigation strategy addresses multiple attacks.
Agree, but here we are talking about the opposite case where a mitigation unintentionally renders a previously set mitigation ineffective.
The conclusion was to make the srso and retbleed relationship clear and then take care of the pr_warn().
So let's ask ourselves: who is really going to see what single-line warning?
Don't know. I guess some do, hence this patch.
What are we *actually* trying to prevent here?
As I said above, a mitigation unintentionally make another mitigation ineffective.
How about a big fat splat at least if we're really trying to prevent something nasty which causes a panic on warn...?
Yes, maybe a WARN_ON() conditional to sanity checks for retbleed/SRSO.