Re: CVE-2023-52920: bpf: support non-r10 register spill/fill to/from stack in precision tracking

Re: CVE-2023-52920: bpf: support non-r10 register spill/fill to/from stack in precision tracking

[Shung-Hsi Yu]

Hi Andrii and Eduard,

I'm trying to determine the security implication of CVE-2023-52920, or
more specifically, what does commit 41f6f64e6999 ("bpf: support non-r10
register spill/fill to/from stack in precision tracking") fix.
Superficially this looks more like an improvement to the verifier.

On Tue, Nov 05, 2024 at 11:09:19AM GMT, Greg Kroah-Hartman wrote:
> Description
> ===========
> 
> In the Linux kernel, the following vulnerability has been resolved:
> 
> bpf: support non-r10 register spill/fill to/from stack in precision tracking
...

Before this commit, precise tracking of stack spills/fills was only
supported if the frame pointer register r10 was directly used (and any
offset to the frame pointer had to be presented through insn->off).

In the case where r10 is indirectly used (e.g. using r6 where r6 = r10 -
16), backtrack_insn() would simply return 0, implying that precision
backtracking has completed; which technically isn't correct. However,
since any register that is stored to the stack is already marked as
precise in check_stack_write_fixed_off(), the verifier would not miss
marking registers as precise - it would just be much more conservative
than needed by marking more registers as precise than necessary.

In other words, even without commit 41f6f64e6999, it shouldn't be
possible to craft a BPF program that tricks the verifier into
incorrectly omitting precision marks, as the verifier would err on the
side of being more strict.

Without this commit, the most significant impact would appear to be that
a BPF program could trigger warnings as reported by Tao Lyu[1].

Does that assessment sound correct?


Thanks,
Shung-Hsi Yu

1: https://lore.kernel.org/all/20231020220216.263948-1-tao.lyu@epfl.ch/

Re: CVE-2023-52920: bpf: support non-r10 register spill/fill to/from stack in precision tracking

[Eduard Zingerman]

On Fri, 2024-11-08 at 12:42 +0800, Shung-Hsi Yu wrote:
> Hi Andrii and Eduard,

Hi Shung-Hsi,

> I'm trying to determine the security implication of CVE-2023-52920, or
> more specifically, what does commit 41f6f64e6999 ("bpf: support non-r10
> register spill/fill to/from stack in precision tracking") fix.
> Superficially this looks more like an improvement to the verifier.

It is my understanding as well, that this commit is an optimization to
avoid some precision marks. I do not see any security implications.

Thanks,
Eduard

[...]

Re: CVE-2023-52920: bpf: support non-r10 register spill/fill to/from stack in precision tracking

[Tao Lyu]

>> Hi Andrii and Eduard,
>
> Hi Shung-Hsi,
>
>> I'm trying to determine the security implication of CVE-2023-52920, or
>> more specifically, what does commit 41f6f64e6999 ("bpf: support non-r10
>> register spill/fill to/from stack in precision tracking") fix.
>> Superficially this looks more like an improvement to the verifier.
>
> It is my understanding as well, that this commit is an optimization to
> avoid some precision marks. I do not see any security implications.
>
> Thanks,
> Eduard
> 
> [...]

Hi Andrii, Eduard, and Shung-Hsi,

The previous discussion is here: https://lore.kernel.org/bpf/20231020155842.130257-1-tao.lyu@epfl.ch/ 

In short, without this patch, the verifier will miss checking an execution path that can have out-of-bound access,
which eventually leads to privilege escalation, like obtaining the root privilege.

Best,
Tao

Re: CVE-2023-52920: bpf: support non-r10 register spill/fill to/from stack in precision tracking

[Shung-Hsi Yu]

On Fri, Nov 08, 2024 at 07:59:22AM GMT, Tao Lyu wrote:
...
> >> I'm trying to determine the security implication of CVE-2023-52920, or
> >> more specifically, what does commit 41f6f64e6999 ("bpf: support non-r10
> >> register spill/fill to/from stack in precision tracking") fix.
> >> Superficially this looks more like an improvement to the verifier.
> >
> > It is my understanding as well, that this commit is an optimization to
> > avoid some precision marks. I do not see any security implications.
...
> Hi Andrii, Eduard, and Shung-Hsi,
> 
> The previous discussion is here: https://lore.kernel.org/bpf/20231020155842.130257-1-tao.lyu@epfl.ch/ 
> 
> In short, without this patch, the verifier will miss checking an execution path that can have out-of-bound access,
> which eventually leads to privilege escalation, like obtaining the root privilege.

Thank you both for looking into this, and Tao's pointer to previous
discussion. I'll try to have this backported to at least stable 6.6.y.

Best,
Shung-Hsi