* Re: [PATCH 5.4 10/36] bpf: Fix tnum constraints for 32-bit comparisons [not found] ` <20200407101455.655552813@linuxfoundation.org> @ 2020-04-07 10:45 ` Daniel Borkmann 2020-04-07 14:42 ` Greg Kroah-Hartman 0 siblings, 1 reply; 2+ messages in thread From: Daniel Borkmann @ 2020-04-07 10:45 UTC (permalink / raw) To: Greg Kroah-Hartman, linux-kernel Cc: stable, Jann Horn, Alexei Starovoitov, Sasha Levin, bpf Hey Sasha, hey Greg, On 4/7/20 12:21 PM, Greg Kroah-Hartman wrote: > From: Jann Horn <jannh@google.com> > > [ Upstream commit 604dca5e3af1db98bd123b7bfc02b017af99e3a0 ] > > The BPF verifier tried to track values based on 32-bit comparisons by > (ab)using the tnum state via 581738a681b6 ("bpf: Provide better register > bounds after jmp32 instructions"). The idea is that after a check like > this: > > if ((u32)r0 > 3) > exit > > We can't meaningfully constrain the arithmetic-range-based tracking, but > we can update the tnum state to (value=0,mask=0xffff'ffff'0000'0003). > However, the implementation from 581738a681b6 didn't compute the tnum > constraint based on the fixed operand, but instead derives it from the > arithmetic-range-based tracking. This means that after the following > sequence of operations: > > if (r0 >= 0x1'0000'0001) > exit > if ((u32)r0 > 7) > exit > > The verifier assumed that the lower half of r0 is in the range (0, 0) > and apply the tnum constraint (value=0,mask=0xffff'ffff'0000'0000) thus > causing the overall tnum to be (value=0,mask=0x1'0000'0000), which was > incorrect. Provide a fixed implementation. > > Fixes: 581738a681b6 ("bpf: Provide better register bounds after jmp32 instructions") > Signed-off-by: Jann Horn <jannh@google.com> > Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> > Signed-off-by: Alexei Starovoitov <ast@kernel.org> > Link: https://lore.kernel.org/bpf/20200330160324.15259-3-daniel@iogearbox.net > Signed-off-by: Sasha Levin <sashal@kernel.org> We've already addressed this issue (CVE-2020-8835) on 5.4/5.5/5.6 kernels through the following backports: https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-5.4.y&id=8d62a8c7489a68b5738390b008134a644aa9b383 https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-5.5.y&id=0ebc01466d98d016eb6a3780ec8edb0c86fa48bc https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-5.6.y&id=6797143df51c8ae259aa4bfe4e99c832b20bde8a Given the severity of the issue, we concluded that revert-only is the best and most straight forward way to address it for stable. Was this selected via Sasha's ML mechanism? Should there be a commit tag to opt-out for some commits being selected? E.g. this one 581738a681b6 ("bpf: Provide better register bounds after jmp32 instructions") already fell through our radar and wrongly made its way into 5.4 where it should have never landed. :/ Thanks, Daniel ^ permalink raw reply [flat|nested] 2+ messages in thread
* Re: [PATCH 5.4 10/36] bpf: Fix tnum constraints for 32-bit comparisons 2020-04-07 10:45 ` [PATCH 5.4 10/36] bpf: Fix tnum constraints for 32-bit comparisons Daniel Borkmann @ 2020-04-07 14:42 ` Greg Kroah-Hartman 0 siblings, 0 replies; 2+ messages in thread From: Greg Kroah-Hartman @ 2020-04-07 14:42 UTC (permalink / raw) To: Daniel Borkmann Cc: linux-kernel, stable, Jann Horn, Alexei Starovoitov, Sasha Levin, bpf On Tue, Apr 07, 2020 at 12:45:23PM +0200, Daniel Borkmann wrote: > Hey Sasha, hey Greg, > > On 4/7/20 12:21 PM, Greg Kroah-Hartman wrote: > > From: Jann Horn <jannh@google.com> > > > > [ Upstream commit 604dca5e3af1db98bd123b7bfc02b017af99e3a0 ] > > > > The BPF verifier tried to track values based on 32-bit comparisons by > > (ab)using the tnum state via 581738a681b6 ("bpf: Provide better register > > bounds after jmp32 instructions"). The idea is that after a check like > > this: > > > > if ((u32)r0 > 3) > > exit > > > > We can't meaningfully constrain the arithmetic-range-based tracking, but > > we can update the tnum state to (value=0,mask=0xffff'ffff'0000'0003). > > However, the implementation from 581738a681b6 didn't compute the tnum > > constraint based on the fixed operand, but instead derives it from the > > arithmetic-range-based tracking. This means that after the following > > sequence of operations: > > > > if (r0 >= 0x1'0000'0001) > > exit > > if ((u32)r0 > 7) > > exit > > > > The verifier assumed that the lower half of r0 is in the range (0, 0) > > and apply the tnum constraint (value=0,mask=0xffff'ffff'0000'0000) thus > > causing the overall tnum to be (value=0,mask=0x1'0000'0000), which was > > incorrect. Provide a fixed implementation. > > > > Fixes: 581738a681b6 ("bpf: Provide better register bounds after jmp32 instructions") > > Signed-off-by: Jann Horn <jannh@google.com> > > Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> > > Signed-off-by: Alexei Starovoitov <ast@kernel.org> > > Link: https://lore.kernel.org/bpf/20200330160324.15259-3-daniel@iogearbox.net > > Signed-off-by: Sasha Levin <sashal@kernel.org> > > We've already addressed this issue (CVE-2020-8835) on 5.4/5.5/5.6 kernels through > the following backports: > > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-5.4.y&id=8d62a8c7489a68b5738390b008134a644aa9b383 > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-5.5.y&id=0ebc01466d98d016eb6a3780ec8edb0c86fa48bc > https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-5.6.y&id=6797143df51c8ae259aa4bfe4e99c832b20bde8a > > Given the severity of the issue, we concluded that revert-only is the best and > most straight forward way to address it for stable. > > Was this selected via Sasha's ML mechanism? Should there be a commit tag to opt-out > for some commits being selected? E.g. this one 581738a681b6 ("bpf: Provide better > register bounds after jmp32 instructions") already fell through our radar and wrongly > made its way into 5.4 where it should have never landed. :/ Oops, yeah, I think this came from Sasha's simple "Fixes:" script, and wasn't aware that it was already resolved. I'll go drop these patches now, thanks for catching this. greg k-h ^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-04-07 14:42 UTC | newest] Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- [not found] <20200407101454.281052964@linuxfoundation.org> [not found] ` <20200407101455.655552813@linuxfoundation.org> 2020-04-07 10:45 ` [PATCH 5.4 10/36] bpf: Fix tnum constraints for 32-bit comparisons Daniel Borkmann 2020-04-07 14:42 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).