From mboxrd@z Thu Jan 1 00:00:00 1970 From: "Gustavo A. R. Silva" Subject: Re: [PATCH] net: core: Fix Spectre v1 vulnerability Date: Sat, 22 Dec 2018 20:53:40 -0600 Message-ID: References: <20181221204901.GA30045@embeddedor> <20181222.150722.1493687829239836271.davem@davemloft.net> <20181222235952.keue7a336sg7jfim@ast-mbp.dhcp.thefacebook.com> <20181222.184051.718127928973898182.davem@davemloft.net> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit Cc: ast@kernel.org, daniel@iogearbox.net, netdev@vger.kernel.org, linux-kernel@vger.kernel.org To: David Miller , alexei.starovoitov@gmail.com Return-path: Received: from gateway33.websitewelcome.com ([192.185.146.97]:39606 "EHLO gateway33.websitewelcome.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1730536AbeLWCxn (ORCPT ); Sat, 22 Dec 2018 21:53:43 -0500 Received: from cm16.websitewelcome.com (cm16.websitewelcome.com [100.42.49.19]) by gateway33.websitewelcome.com (Postfix) with ESMTP id B6CF155144 for ; Sat, 22 Dec 2018 20:53:41 -0600 (CST) In-Reply-To: <20181222.184051.718127928973898182.davem@davemloft.net> Content-Language: en-US Sender: netdev-owner@vger.kernel.org List-ID: Hi, On 12/22/18 8:40 PM, David Miller wrote: > From: Alexei Starovoitov > Date: Sat, 22 Dec 2018 15:59:54 -0800 > >> On Sat, Dec 22, 2018 at 03:07:22PM -0800, David Miller wrote: >>> From: "Gustavo A. R. Silva" >>> Date: Fri, 21 Dec 2018 14:49:01 -0600 >>> >>>> flen is indirectly controlled by user-space, hence leading to >>>> a potential exploitation of the Spectre variant 1 vulnerability. >>>> >>>> This issue was detected with the help of Smatch: >>>> >>>> net/core/filter.c:1101 bpf_check_classic() warn: potential spectre issue 'filter' [w] >>>> >>>> Fix this by sanitizing flen before using it to index filter at line 1101: >>>> >>>> switch (filter[flen - 1].code) { >>>> >>>> and through pc at line 1040: >>>> >>>> const struct sock_filter *ftest = &filter[pc]; >>>> >>>> Notice that given that speculation windows are large, the policy is >>>> to kill the speculation on the first load and not worry if it can be >>>> completed with a dependent load/store [1]. >>>> >>>> [1] https://marc.info/?l=linux-kernel&m=152449131114778&w=2 >>>> >>>> Signed-off-by: Gustavo A. R. Silva >>> >>> BPF folks, I'll take this directly. >>> >>> Applied and queued up for -stable, thanks. >> >> hmm. what was the rush? >> I think it is unnecessary change. >> Though fp is passed initially from user space >> it's copied into kernel struct first. >> There is no way user space can force kernel to mispredict >> branch in for (pc = 0; pc < flen; pc++) loop. The following piece of code is the one that can be mispredicted, not the for loop: 1013 if (flen == 0 || flen > BPF_MAXINSNS) 1014 return false; Instead of calling array_index_nospec() inside bpf_check_basics_ok(), I decided to place the call close to the code that could be compromised. This is when accessing filter[].