Received: by 2002:a25:c593:0:0:0:0:0 with SMTP id v141csp6030504ybe; Tue, 10 Sep 2019 12:24:59 -0700 (PDT) X-Google-Smtp-Source: APXvYqz3pMJjXZPZVqlQd5pQyOEzlv3U2WnWnXhUUwndOWwxTn2grXTkY4rVt+kxvrl9Pr1xict4 X-Received: by 2002:a17:906:134e:: with SMTP id x14mr26453743ejb.280.1568143499417; Tue, 10 Sep 2019 12:24:59 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1568143499; cv=none; d=google.com; s=arc-20160816; b=d13bLSqmrsgOCFo0+FEWhscUBF++T6Cj5+00IzDmTM48K8yCM0YwQ0HYL9ehdVnj0v C20/yeyT2lqPQVT3o7KrqbhzuEFQHiE8yzfs3mgvt2UUS1n51veQOQie5GSh8JFndikV i2zpgfVSXRk8cgt+h2jwMSJvKymyUwWdAS+wHHxSIkdPQUglQhfswnw91fFBEDX85wNK 5qYKz83wOVxBgTc3Ashzw3cQ84Bdtd9yp8zNpDR3zg7zitpolAsOGIS4aft0U9YXo6LJ WRxUSHnViCTE2f7jtiHTkAHAJx7c80SafHhrrhRvfs1II2to6oHd4GOq3ziYnToP5Lq6 Vf3g== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:sender:user-agent:in-reply-to :content-disposition:mime-version:references:message-id:subject:cc :to:from:date:dkim-signature; bh=WytUsbUCueuTaHp+R3kiieTTsIcIBaJbRPUzOY9zmMs=; b=kHQiwpbrio3+o2XMpB25oko+fv6EJdxfe8bVe4wbJYYxSqETnULBRYtMULjQjXzlKS PLRsz9EG5DThgZkfjq9uB8qbRatR0iWx5UHcmb0BooO8BUhQx7plaYDpBQtMki6xKDGc Q1HZgqubfp6Jn06X0prF3dwXYytOLLy82py5D2HB+tgH9jKLOgZvezRDAANMsWpnVSVX FyuI6GFkIxQNsk7WqT0uGJOXGabIDGUap5Rv/y72lstBXDBqPMJSMdeuTFw6QPcYDf3/ gDfT+DUThxth+kqmSqf73td6ORPd3JZ1GCl1/h7VtoxrYoDXliNUy71/ggD4S1ZTPVAM w8AQ== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=g8eBFeJU; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Return-Path: Received: from vger.kernel.org (vger.kernel.org. [209.132.180.67]) by mx.google.com with ESMTP id w2si11994125edc.386.2019.09.10.12.24.34; Tue, 10 Sep 2019 12:24:59 -0700 (PDT) Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67; Authentication-Results: mx.google.com; dkim=pass header.i=@google.com header.s=20161025 header.b=g8eBFeJU; spf=pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=REJECT sp=REJECT dis=NONE) header.from=google.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S2437012AbfIJRXA (ORCPT + 99 others); Tue, 10 Sep 2019 13:23:00 -0400 Received: from mail-pf1-f194.google.com ([209.85.210.194]:46511 "EHLO mail-pf1-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1729580AbfIJRXA (ORCPT ); Tue, 10 Sep 2019 13:23:00 -0400 Received: by mail-pf1-f194.google.com with SMTP id q5so11902538pfg.13 for ; Tue, 10 Sep 2019 10:23:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to:user-agent; bh=WytUsbUCueuTaHp+R3kiieTTsIcIBaJbRPUzOY9zmMs=; b=g8eBFeJURPLmM7K80XMyv6QCx9j8YlqaXas7oh2yW8WK40QJIDwACb+8Fed1j4td8M rDgIx0hMCLUGE2ZLv0+DhqEU6GEAD9EgjaTcKC2f7slxN9qUxOR3H7UikEyiSWfyziwL lzkvnZGQUh93SlN+8AXzBgLvJqZ2UCH0Zeycm3EFae21/p/tcstsMHN7DgvmMYWoSa/E xEz6R5wNLvHQWPJ3Bb6JtYorQKBqOqedQneR/BXDcedQqQEDA50N8N9h36EL9837Y5G0 WlDnCjhRt6OajXG+J5ay44IycrumkvdYDUnesB54fFvPe3v8pBv4tcVLn8H/X/zSOSne RDzQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to:user-agent; bh=WytUsbUCueuTaHp+R3kiieTTsIcIBaJbRPUzOY9zmMs=; b=KsbMj6cphLFNyCo2HR9+DGalil4OG8Y3EuWNlldK3yjYUBmkFaP2RuO9etKaTSB2eU mCkb3zN+ksCy6eUXdsSoykCdvkv0xva+k9ivT98310tORySavh93UGq17adTrSCvMyDw PAtA301MWoJ9mYsvuVv7N6LwG++HxEbH0vXKpE8yOZ1lfBNoCCOf8impCmrrvjF66AE9 n/ImQ4oe/MrRHpc84DhQ+UCjccIU/Qqd8L28VXV/gNeiYvKjimJJTQ0Ntg9unw9zjtF2 YEoKJDPudLVkEmBq49Z0d7a4K4jNpL5UDdVrcY7gLVzGE1Uoy9NcukBQg3GveUwWSshw 8OuQ== X-Gm-Message-State: APjAAAWMib4i+hybAWqtE9hH/YSxyxWB9Ksd8y9m+AG2sCZ7wt/Wh+pw Du/8x2MkKieEQEjirmdWiAcGQQ== X-Received: by 2002:a63:dd0c:: with SMTP id t12mr27742480pgg.82.1568136178740; Tue, 10 Sep 2019 10:22:58 -0700 (PDT) Received: from google.com ([2620:15c:201:2:ce90:ab18:83b0:619]) by smtp.gmail.com with ESMTPSA id j2sm19138413pfe.130.2019.09.10.10.22.57 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 10 Sep 2019 10:22:58 -0700 (PDT) Date: Tue, 10 Sep 2019 10:22:53 -0700 From: Sami Tolvanen To: Yonghong Song Cc: Alexei Starovoitov , Daniel Borkmann , Kees Cook , Martin Lau , Song Liu , "netdev@vger.kernel.org" , "bpf@vger.kernel.org" , "linux-kernel@vger.kernel.org" Subject: Re: [PATCH] bpf: validate bpf_func when BPF_JIT is enabled Message-ID: <20190910172253.GA164966@google.com> References: <20190909223236.157099-1-samitolvanen@google.com> <4f4136f5-db54-f541-2843-ccb35be25ab4@fb.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <4f4136f5-db54-f541-2843-ccb35be25ab4@fb.com> User-Agent: Mutt/1.10.1 (2018-07-13) Sender: linux-kernel-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Tue, Sep 10, 2019 at 08:37:19AM +0000, Yonghong Song wrote: > You did not mention BPF_BINARY_HEADER_MAGIC and added member > of `magic` in bpf_binary_header. Could you add some details > on what is the purpose for this `magic` member? Sure, I'll add a description to the next version. The magic is a random number used to identify bpf_binary_header in memory. The purpose of this patch is to limit the possible call targets for the function pointer and checking for the magic helps ensure we are jumping to a page that contains a jited function, instead of allowing calls to arbitrary targets. This is particularly useful when combined with the compiler-based Control-Flow Integrity (CFI) mitigation, which Google started shipping in Pixel kernels last year. The compiler injects checks to all indirect calls, but cannot obviously validate jumps to dynamically generated code. > > +unsigned int bpf_call_func(const struct bpf_prog *prog, const void *ctx) > > +{ > > + const struct bpf_binary_header *hdr = bpf_jit_binary_hdr(prog); > > + > > + if (!IS_ENABLED(CONFIG_BPF_JIT_ALWAYS_ON) && !prog->jited) > > + return prog->bpf_func(ctx, prog->insnsi); > > + > > + if (unlikely(hdr->magic != BPF_BINARY_HEADER_MAGIC || > > + !arch_bpf_jit_check_func(prog))) { > > + WARN(1, "attempt to jump to an invalid address"); > > + return 0; > > + } > > + > > + return prog->bpf_func(ctx, prog->insnsi); > > +} > The above can be rewritten as > if (IS_ENABLED(CONFIG_BPF_JIT_ALWAYS_ON) || prog->jited || > hdr->magic != BPF_BINARY_HEADER_MAGIC || > !arch_bpf_jit_check_func(prog))) { > WARN(1, "attempt to jump to an invalid address"); > return 0; > } That doesn't look quite equivalent, but yes, this can be rewritten as a single if statement like this: if ((IS_ENABLED(CONFIG_BPF_JIT_ALWAYS_ON) || prog->jited) && (hdr->magic != BPF_BINARY_HEADER_MAGIC || !arch_bpf_jit_check_func(prog))) I think splitting the interpreter and JIT paths would be more readable, but I can certainly change this if you prefer. > BPF_PROG_RUN() will be called during xdp fast path. > Have you measured how much slowdown the above change could > cost for the performance? I have not measured the overhead, but it shouldn't be significant. Is there a particular benchmark you'd like me to run? Sami