From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 680EEC433F5 for ; Thu, 21 Apr 2022 18:24:44 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id DD9316B0072; Thu, 21 Apr 2022 14:24:43 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id D88226B0073; Thu, 21 Apr 2022 14:24:43 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id C51646B0074; Thu, 21 Apr 2022 14:24:43 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (relay.a.hostedemail.com [64.99.140.24]) by kanga.kvack.org (Postfix) with ESMTP id B728B6B0072 for ; Thu, 21 Apr 2022 14:24:43 -0400 (EDT) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay06.hostedemail.com (Postfix) with ESMTP id 8E3F527A19 for ; Thu, 21 Apr 2022 18:24:43 +0000 (UTC) X-FDA: 79381712046.03.1F3F1B3 Received: from mail-pl1-f177.google.com (mail-pl1-f177.google.com [209.85.214.177]) by imf02.hostedemail.com (Postfix) with ESMTP id E1D008002B for ; Thu, 21 Apr 2022 18:24:41 +0000 (UTC) Received: by mail-pl1-f177.google.com with SMTP id n8so5649435plh.1 for ; Thu, 21 Apr 2022 11:24:42 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=iCbcFDFbn2mhKynuYB02KhdbVXr72r1cIVRt2FI2Ets=; b=HqvvWWyYs5DFKktWicH2EF49fqzUee2aliWRQKcrrunRjNuGvLFTBvT7Y1V4ZxXT+G xV4tsLbFEw/uFKZ3cb2jWPYkl6CVhSz+bRhr5h2vVASWouwqqirPrCSpq706jU4Q+RUa 3obsQQ9UXtPRvl+MZnj0mdegShcWv4zU7R9QaIUxdCXDEj7ZE1JTjO2vT8yqh3UcZHNJ Nfd0hgTnaCz7eUTysQaP4C3enbN0omZ6IOugtbZmPxNlNJLbMvYu5faNCdTeN8KNhO03 72ry4qCftQTgxBWSJ2zeu8Y9gu51D5dUM431GOLzbmwnLA6Li8+IjxXwHOCptdLYg2yD jovg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=iCbcFDFbn2mhKynuYB02KhdbVXr72r1cIVRt2FI2Ets=; b=BtrwlXvihHs4k1s4wezWPBbAVNwujqeoutAScXCKYlRe0uq5HZ4z02CWwWGPETuOt/ G3YVd/rY12zSroCS4qU37IzfFW9rAX6fyN3YV/BfA9OqsRlnh/ihLvgNx3U2dkckO+f/ WmwgtvpzL4Qv+lS1SluCp4JyidgNM7Bo3ElSgrJqsBUmMpSFQoyLmliy8xHUMnkBjZ+6 KOjo/kctZGPMUizQcdlkzJvjXLeHUFc2MMFWTKqGT9VPrveLpM48CUYky5sQ6Wb9Qxzy wXMZRE/HXafr730SBeWIjZAkGwp0sI0msG5jZ7wp2KKzmhhHIocPcBy596JLbutEZJod TK4w== X-Gm-Message-State: AOAM533BOYgVqcDsn5wK33tsx+vuBvBqErZVnS6i5LbzEf+ELCXNioZr 5+PaRLHQzn/bI0RlfIy2GGfbjWdXx/5MwTubvKo= X-Google-Smtp-Source: ABdhPJwYBNWuVtwoUeeocV7PLCWhSWkNZkv2HQVwVmYSf5Sz3RNhIY+aKyczpvducm6w2n3P2pO4FfHViXzUIYvxX40= X-Received: by 2002:a17:90a:8591:b0:1b9:da10:2127 with SMTP id m17-20020a17090a859100b001b9da102127mr11902949pjn.13.1650565482039; Thu, 21 Apr 2022 11:24:42 -0700 (PDT) MIME-Version: 1.0 References: <20220421072212.608884-1-song@kernel.org> In-Reply-To: From: Alexei Starovoitov Date: Thu, 21 Apr 2022 11:24:31 -0700 Message-ID: Subject: Re: [PATCH bpf] bpf: invalidate unused part of bpf_prog_pack To: Linus Torvalds Cc: Song Liu , bpf , Linux-MM , Linux Kernel Mailing List , Alexei Starovoitov , Daniel Borkmann , Kernel Team , Andrew Morton , "Edgecombe, Rick P" , Christoph Hellwig , Andrii Nakryiko Content-Type: text/plain; charset="UTF-8" Authentication-Results: imf02.hostedemail.com; dkim=pass header.d=gmail.com header.s=20210112 header.b=HqvvWWyY; dmarc=pass (policy=none) header.from=gmail.com; spf=pass (imf02.hostedemail.com: domain of alexei.starovoitov@gmail.com designates 209.85.214.177 as permitted sender) smtp.mailfrom=alexei.starovoitov@gmail.com X-Rspam-User: X-Rspamd-Server: rspam08 X-Rspamd-Queue-Id: E1D008002B X-Stat-Signature: ui6dwtr7mjguiutyfn9adjdki4txmx1q X-HE-Tag: 1650565481-821688 X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, Apr 21, 2022 at 10:09 AM Linus Torvalds wrote: > > On Thu, Apr 21, 2022 at 12:27 AM Song Liu wrote: > > > > --- a/arch/x86/net/bpf_jit_comp.c > > +++ b/arch/x86/net/bpf_jit_comp.c > > @@ -228,6 +228,28 @@ static void jit_fill_hole(void *area, unsigned int size) > > memset(area, 0xcc, size); > > } > > > > +#define INVALID_BUF_SIZE PAGE_SIZE > > +static char invalid_insn_buf[INVALID_BUF_SIZE]; > > + > > +static int __init bpf_init_invalid_insn_buf(void) > > +{ > > + jit_fill_hole(invalid_insn_buf, INVALID_BUF_SIZE); > > + return 0; > > +} > > +pure_initcall(bpf_init_invalid_insn_buf); > > + > > +void bpf_arch_invalidate_text(void *dst, size_t len) > > +{ > > + size_t i = 0; > > + > > + while (i < len) { > > + size_t s = min_t(size_t, len - i, INVALID_BUF_SIZE); > > + > > + bpf_arch_text_copy(dst + i, invalid_insn_buf, s); > > + i += s; > > + } > > +} > > Why do we need this new infrastructure? > > Why bpf_arch_invalidate_text()? > > Why not jit_fill_hole() unconditionally? > > It seems a bit pointless to have page buffer for containing this data, > when we already have a (trivial) function to fill an area with invalid > instructions. > > On x86, it's literally just "memset(0xcc)" (ie all 'int3' instructions). > > And on most RISC architectures, it would be some variation of > "memset32(TRAP_INSN)". > > And all bpf targets should already have that nicely as that > jit_fill_hole() function, no? > > The pack-allocator bpf code already *does* that, and is already passed > that function. > > But it's just that it does it too late. Instead of doing it when > allocating a new pack, it does it in the sub-allocator. > > Afaik the code in bpf/core.c already has all the information it needs, > and already has that jit_fill_hole() function pointer, but is applying > it at the wrong point. > > So I think the fix should be to just pass in that 'bpf_fill_ill_insns' > function pointer all the way to alloc_new_pack(), instead of using it > in bpf_jit_binary_alloc(). jit_fill_hole is an overkill here. Long ago when jit spraying attack was fixed there was a concern that memset(0) essentially populates the code page with valid 'add BYTE PTR [rax],al' instructions. Jumping anywhere in the zero page with a valid address in rax will eventually lead to execution of the first insn in jit-ed bpf prog. So memset(0xcc) was added to make it a bit harder to guess the start address. jit spraying is only a concern for archs that can jump in the middle of the instruction and cpus will interpret the byte stream differently. The existing bpf_prog_pack code still does memset(0xcc) a random range of bytes before and after jit-ed bpf code. So doing memset(0xcc) for the whole huge page is not necessary at all. Just memset(0) of a huge page at init time and memset(0) when prog is freed is enough. Jumping into zero page of 'valid' insns the cpu will eventually stumble on 0xcc before reaching the first insn. Let's not complicate the logic by dragging jit_fill_hole further into generic allocation.