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 vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id A486AC433F5 for ; Sat, 21 May 2022 01:01:07 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1347006AbiEUBBF (ORCPT ); Fri, 20 May 2022 21:01:05 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:50714 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1350544AbiEUBBE (ORCPT ); Fri, 20 May 2022 21:01:04 -0400 Received: from bombadil.infradead.org (bombadil.infradead.org [IPv6:2607:7c80:54:e::133]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 61354515A7; Fri, 20 May 2022 18:01:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=infradead.org; s=bombadil.20210309; h=Sender:In-Reply-To:Content-Type: MIME-Version:References:Message-ID:Subject:Cc:To:From:Date:Reply-To: Content-Transfer-Encoding:Content-ID:Content-Description; bh=4pzvRwoVdtYoVcWFyyAnPE0HISlku86yFUDDkjyuoZg=; b=C/vAXd2A8NSnWf9Ia9CxZQ7HWd eC1jdZIkoq35cFPNjQB8DPs6mJdeEt29TIQKDtnkgjTYYrqS5utyu2EdsBEetVG2MYF9g3veZ6QWT WUt6QyMaT+/NTh9CEbnVWqefeoZ1ZnVkPcl/PO6MzUPEkKQgLStT20xPGZ8aCDlbE/stjtP0o3iUY FwCP/FbIfiRlr3xi5bOCpBew9Yh/5vgQsIHxVpMh3X4TrkWSNGpxpWZcOY+O0xoPid/aRQckcFURd J1liBHgnehV8y9wgUXvJiGLzAgbrRWalq0pKefjdnhl6e0XK6iJRFCeZrjRYi2iz6PtNz9au59awl O3qVzKrA==; Received: from mcgrof by bombadil.infradead.org with local (Exim 4.94.2 #2 (Red Hat Linux)) id 1nsDUH-00F5i2-1l; Sat, 21 May 2022 01:00:57 +0000 Date: Fri, 20 May 2022 18:00:57 -0700 From: Luis Chamberlain To: Song Liu , Rick Edgecombe , Arnd Bergmann Cc: linux-kernel@vger.kernel.org, bpf@vger.kernel.org, linux-mm@kvack.org, ast@kernel.org, daniel@iogearbox.net, peterz@infradead.org, torvalds@linux-foundation.org, kernel-team@fb.com Subject: Re: [PATCH v3 bpf-next 5/8] bpf: use module_alloc_huge for bpf_prog_pack Message-ID: References: <20220520031548.338934-1-song@kernel.org> <20220520031548.338934-6-song@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20220520031548.338934-6-song@kernel.org> Sender: Luis Chamberlain Precedence: bulk List-ID: X-Mailing-List: bpf@vger.kernel.org On Thu, May 19, 2022 at 08:15:45PM -0700, Song Liu wrote: > Also, remove set_vm_flush_reset_perms() from alloc_new_pack() and use > set_memory_[nx|rw] in bpf_prog_pack_free(). This is because > VM_FLUSH_RESET_PERMS does not work with huge pages yet. [1] > > [1] https://lore.kernel.org/bpf/aeeeaf0b7ec63fdba55d4834d2f524d8bf05b71b.camel@intel.com/ > Suggested-by: Rick Edgecombe > Signed-off-by: Song Liu > --- Rick, although VM_FLUSH_RESET_PERMS is rather new my concern here is we're essentially enabling sloppy users to grow without also addressing what if we have to take the leash back to support VM_FLUSH_RESET_PERMS properly? If the hack to support this on other architectures other than x86 is as simple as the one you in vm_remove_mappings() today: if (flush_reset && !IS_ENABLED(CONFIG_ARCH_HAS_SET_DIRECT_MAP)) { set_memory_nx(addr, area->nr_pages); set_memory_rw(addr, area->nr_pages); } then I suppose this isn't a big deal. I'm just concerned here this being a slippery slope of sloppiness leading to something which we will regret later. My intution tells me this shouldn't be a big issue, but I just want to confirm. Luis > diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c > index cacd8684c3c4..b64d91fcb0ba 100644 > @@ -949,6 +947,8 @@ static void bpf_prog_pack_free(struct bpf_binary_header *hdr) > > mutex_lock(&pack_mutex); > if (hdr->size > bpf_prog_pack_size) { > + set_memory_nx((unsigned long)hdr, hdr->size / PAGE_SIZE); > + set_memory_rw((unsigned long)hdr, hdr->size / PAGE_SIZE); > module_memfree(hdr); > goto out; > } > @@ -975,6 +975,8 @@ static void bpf_prog_pack_free(struct bpf_binary_header *hdr) > if (bitmap_find_next_zero_area(pack->bitmap, bpf_prog_chunk_count(), 0, > bpf_prog_chunk_count(), 0) == 0) { > list_del(&pack->list); > + set_memory_nx((unsigned long)pack->ptr, bpf_prog_pack_size / PAGE_SIZE); > + set_memory_rw((unsigned long)pack->ptr, bpf_prog_pack_size / PAGE_SIZE); > module_memfree(pack->ptr); > kfree(pack); > } > -- > 2.30.2 >