On Wed, Sep 13, 2023 at 5:30 AM Luis Gerhorst <gerhorst@xxxxxxxxx> wrote:
>
> This reverts commit d75e30dddf73449bc2d10bb8e2f1a2c446bc67a2.
>
> To mitigate Spectre v1, the verifier relies on static analysis to deduct
> constant pointer bounds, which can then be enforced by rewriting pointer
> arithmetic [1] or index masking [2]. This relies on the fact that every
> memory region to be accessed has a static upper bound and every date
> below that bound is accessible. The verifier can only rewrite pointer
> arithmetic or insert masking instructions to mitigate Spectre v1 if a
> static upper bound, below of which every access is valid, can be given.
>
> When allowing packet pointer comparisons, this introduces a way for the
> program to effectively construct an accessible pointer for which no
> static upper bound is known. Intuitively, this is obvious as a packet
> might be of any size and therefore 0 is the only statically known upper
> bound below of which every date is always accessible (i.e., none).
>
> To clarify, the problem is not that comparing two pointers can be used
> for pointer leaks in the same way in that comparing a pointer to a known
> scalar can be used for pointer leaks. That is because the "secret"
> components of the addresses cancel each other out if the pointers are
> into the same region.
>
> With [3] applied, the following malicious BPF program can be loaded into
> the kernel without CAP_PERFMON:
>
> r2 = *(u32 *)(r1 + 76) // data
> r3 = *(u32 *)(r1 + 80) // data_end
> r4 = r2
> r4 += 1
> if r4 > r3 goto exit
> r5 = *(u8 *)(r2 + 0) // speculatively read secret
> r5 &= 1 // choose bit to leak
> // ... side channel to leak secret bit
> exit:
> // ...
>
> This is jited to the following amd64 code which still contains the
> gadget:
>
> 0: endbr64
> 4: nopl 0x0(%rax,%rax,1)
> 9: xchg %ax,%ax
> b: push %rbp
> c: mov %rsp,%rbp
> f: endbr64
> 13: push %rbx
> 14: mov 0xc8(%rdi),%rsi // data
> 1b: mov 0x50(%rdi),%rdx // data_end
> 1f: mov %rsi,%rcx
> 22: add $0x1,%rcx
> 26: cmp %rdx,%rcx
> 29: ja 0x000000000000003f // branch to mispredict
> 2b: movzbq 0x0(%rsi),%r8 // speculative load of secret
> 30: and $0x1,%r8 // choose bit to leak
> 34: xor %ebx,%ebx
> 36: cmp %rbx,%r8
> 39: je 0x000000000000003f // branch based on secret
> 3b: imul $0x61,%r8,%r8 // leak using port contention side channel
> 3f: xor %eax,%eax
> 41: pop %rbx
> 42: leaveq
> 43: retq
>
> Here I'm using a port contention side channel because storing the secret
> to the stack causes the verifier to insert an lfence for unrelated
> reasons (SSB mitigation) which would terminate the speculation.
>
> As Daniel already pointed out to me, data_end is even attacker
> controlled as one could send many packets of sufficient length to train
> the branch prediction into assuming data_end >= data will never be true.
> When the attacker then sends a packet with insufficient data, the
> Spectre v1 gadget leaks the chosen bit of some value that lies behind
> data_end.
The above analysis is correct, but unlike traditional spec_v1
the attacker doesn't control data/data_end.
The attack can send many large packets to train that data + X < data_end
and then send a small packet where CPU will mispredict that branch
and data + X will speculatively read past data_end,
so the attacker can extract a bit past data_end,
but data/data_end themselves cannot be controlled.
So whether this bit 0 or 1 has no bearing.
The attack cannot be repeated for the same location.
The attacker can read one bit 8 times in a row and all of them
will be from different locations in the memory.
Same as reading 8 random bits from 8 random locations.
Hence I don't think this revert is necessary.
I don't believe you can craft an actual exploit.
Your patch 3 says:
/* Speculative access to be prevented. */
+ char secret = *((char *) iph);
+
+ /* Leak the first bit of the secret value that lies behind data_end to a
+ * SMP silbling thread that also executes imul instructions. If the bit
+ * is 1, the silbling will experience a slowdown. */
+ long long x = secret;
+ if (secret & 1) {
+ x *= 97;
+ }
the comment is correct, but speculative access alone is not enough
to leak data.
