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 47F21C4167B for ; Thu, 7 Dec 2023 08:37:31 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id D0C1F6B007B; Thu, 7 Dec 2023 03:37:30 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id C955C6B007E; Thu, 7 Dec 2023 03:37:30 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B35666B0080; Thu, 7 Dec 2023 03:37:30 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id A1FE76B007B for ; Thu, 7 Dec 2023 03:37:30 -0500 (EST) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay02.hostedemail.com (Postfix) with ESMTP id 6F8D612013E for ; Thu, 7 Dec 2023 08:37:30 +0000 (UTC) X-FDA: 81539368260.03.C824AC3 Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf03.hostedemail.com (Postfix) with ESMTP id 6C04020014 for ; Thu, 7 Dec 2023 08:37:28 +0000 (UTC) Authentication-Results: imf03.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=Rgod5W5N; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf03.hostedemail.com: domain of omosnace@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=omosnace@redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1701938248; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=Ql1Ghzu6pTeMcHMjS2o9I9cOZlV0c5FGtb0DiNg5B/Y=; b=aftQw9PBVU/Wl6uvUoj7LOhULTfv2rdaD10ixXtWqxq8G328kgRHLbPmmE695zx8vCHrNV Q46n58TpHTMy+oHbBprvYSnT0n9UxHwGrczHZ4Kxu23r+08ay3WmAffpdbUp3qXjVgTPp8 gViEPbcHLiNhpNaQy3GwDmnULnjEJ+g= ARC-Authentication-Results: i=1; imf03.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=Rgod5W5N; dmarc=pass (policy=none) header.from=redhat.com; spf=pass (imf03.hostedemail.com: domain of omosnace@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=omosnace@redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1701938248; a=rsa-sha256; cv=none; b=iD6eYSxV+bJUF8BB4UnjyGhBmf+PZJeyd5neXoPIHOlf/jGMEUrTOeXWZeQBo17uvUggAG oguGETTv/Txa0/oukxXPYzj3LDR4becK8sCcCXAE2+SgK3Xa/6Wfotc+53WHgNHrPFDs4T a4xjtOz3caGA3ikBohYwYpYrMZkhcFs= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1701938247; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=Ql1Ghzu6pTeMcHMjS2o9I9cOZlV0c5FGtb0DiNg5B/Y=; b=Rgod5W5NfEG7c9/2GiBYY2GkUnUzKoGf9NrBBL4a2r8Rzxd7U1+gY7MSuvYQir7q1iaWpX 7tmr0/oH6qeH0IwgJsRlj+L8jnGRX23nZQQjJCpfA2gRarM3YUrhP1j7bx4vBAkaJ35ieK nO0iqiXl6IP17h/y4QdLk/DHmvugrZ4= Received: from mail-pj1-f70.google.com (mail-pj1-f70.google.com [209.85.216.70]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-689-iEtU2jD1M_C_21YnB1IVfw-1; Thu, 07 Dec 2023 03:37:26 -0500 X-MC-Unique: iEtU2jD1M_C_21YnB1IVfw-1 Received: by mail-pj1-f70.google.com with SMTP id 98e67ed59e1d1-2865681dcb4so693288a91.3 for ; Thu, 07 Dec 2023 00:37:25 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1701938244; x=1702543044; h=content-transfer-encoding:cc:to:subject:message-id:date:from :in-reply-to:references:mime-version:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=Ql1Ghzu6pTeMcHMjS2o9I9cOZlV0c5FGtb0DiNg5B/Y=; b=X+hImtn2YHJHatK8BBnjEG4gqmZT+auiLHjYrtHYN0xz9p5nOdl7btBHDcEBic/is/ KFwvPxQRj1UgKRhe7BLOA7NpewfMler9duTcwRwQiRg4VNBXGFY0EuscM70emFos+FB3 tHSeuoxUI1t8/lro21f6mUV8x0zUTTCAQZABCL0Mt0b7wvZH0GJ53jhzYt71UrZ9pU+f Qb1oT7IZ2eFRqrpEocNK/UykRaoNXwQKICSSCFlGVU5mqCi3+rIx4LvcKMq+6JsTFHeZ /AyM8YWaC6Cs1vA+5/bWLj+V9gHrYJQFbUxJO8VgJe11/KVBGI+qvDLFI1ZWfTKANaK4 M+qQ== X-Gm-Message-State: AOJu0Yya/Rq18dQeUXGH0y44NHkbRIzVoh754sFzjAdhWNx+nyP+V0c5 zCIoGVSCotrmZ9r2MiKXxaQNT4cV5v8Ah6QosNLLwwsDt2aYQ5xtLPk86Xs8naPprx5pBYYsLS1 7zP+a9RgLOyMxKa88bBB/ynveS4w= X-Received: by 2002:a17:90b:4a8f:b0:286:b6bf:e6d5 with SMTP id lp15-20020a17090b4a8f00b00286b6bfe6d5mr1779184pjb.14.1701938244492; Thu, 07 Dec 2023 00:37:24 -0800 (PST) X-Google-Smtp-Source: AGHT+IGstCK0Dkasup6i8nnE06yQpc/uNh0Tu7+7VEU7OCd1DWWkXX6Dq53Ha8GjO7pxz/6aqAHli0YdroCP7JK/3sI= X-Received: by 2002:a17:90b:4a8f:b0:286:b6bf:e6d5 with SMTP id lp15-20020a17090b4a8f00b00286b6bfe6d5mr1779174pjb.14.1701938244171; Thu, 07 Dec 2023 00:37:24 -0800 (PST) MIME-Version: 1.0 References: <20230728050043.59880-1-wangkefeng.wang@huawei.com> <20230728050043.59880-4-wangkefeng.wang@huawei.com> <79f6131f-7d69-408d-917a-f0ca6fccc5b2@huawei.com> In-Reply-To: <79f6131f-7d69-408d-917a-f0ca6fccc5b2@huawei.com> From: Ondrej Mosnacek Date: Thu, 7 Dec 2023 09:37:13 +0100 Message-ID: Subject: Re: [PATCH v3 3/4] selinux: use vma_is_initial_stack() and vma_is_initial_heap() To: Kefeng Wang Cc: Paul Moore , Stephen Smalley , SElinux list , "open list:MEMORY MANAGEMENT" , David Hildenbrand , peterz@infradead.org, Andrew Morton X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Rspam-User: X-Stat-Signature: ipbzpigfrz1rpm9zyywiarw8cxbmrwnh X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: 6C04020014 X-HE-Tag: 1701938248-507317 X-HE-Meta: U2FsdGVkX1+MIB5P6Jdfe9WGeAZeSjgT2NpLB8DdBeUZtJsaBjxDg3BEA7H7uDBbtRDqiM1mE+60m0M0BswBSpA1J4TluRnwJHm7D86KoVg4p3OqzJVwikFdn/9SA3ZWR+KHfIdvtklhTQbu/cNtFanI/cSDv9YEn9+QemdZGjzoQEzXh06lyCayU0eTOG4rI6NeZ/io+/m8T2DL8ElTIRecglgIvKuUy4zf1oNbYuSSYVFHR1J5ItpzCIO/Oa1aRIZqH7tuNR8laSj/5Dl3K6uuaSV4X7o1hHpYf6WG6SnBsRT7Tg0g4K7vzFfJCC8cVvJvy7rfzr5YnVx70Q28ffmHxYu/KDZM4V1WDRSMbgw9Voxwf96tE3qt79ThAI+FZMKcuhGh1t4kla5NpXSf5p9EXpOrnXRegxhxd3v9pfT4EelHpFpdZMLH6WS1fTI7LTQdFb7EvrMEHV+gvkk2Z5GUpiZiYdLpc1UaEnWmwAXbsaz5fsAuiFtnFtSuRm7V8Q6BRZnnFFmVlPGjczV/HYsl+0v5ki26rkY7AJL1jo+CM6qyCkjRdkVflXLkn17R75xle8UFca2QYgqkfuJOMtBzVn1pbLNX8K4hv41TTaj/Hbo/NEPa/vciWShwWY+g69sGdtq/lDyl7gRma9vU92vmtkcnSiNA2OYZX2SJCyv31L9QnEYi0kLtfkPiowYaQ5k3KItkyfbfU1L4+IY+DNpaLQZiUBs3oy+4CIeA06slywkTcsf/hU29amMuamXZ1kBexCLhfrEYWGw7stuJAjQtfBCFPuzyZEjhPTswcIBp2++TywrA7si72m5p/KJnaNXKu9l3pYwGtScdTEWlFP7W/nCPy8IsSKSoggUv6BPGw4y7QclQ6DjyLrQqzjwAqR7vqiNR2f6DTQ1lbxTWTTXbyVcs4yG3jLminrLGA4Cr7HCXg9tWZQHwLdm392SCURxGC5ftCeDbIDSby+Q D/3pyooa dIcUDtF+8Profm0soi3IfAoZEUmukWWPqc7y7RfRlYWTnbxdPHG/pqikzpt+w2BoXRFdVhgnuGBjrJZFZnXeXj+2siSPh1F0nNEsovC6Hku5Pdvo2kcrKNLkITAjgo30axbyLG6nnHSJ2hIZuVVbzzMP8XoeHM8carEing07p3bWNR+GjjTQXxXhSWFMEs1NBk09FY64N0YCKYdZ6uwiYFGSB6e9weSE+JAWEB68iuchUJtLQ1hG4z+W3lHIVbpFPg83lIWmC0a6zX16zlhXxtgpMhKjkGpifVX2AkP78hgirNKyCI8P8LeNX9DNuzWo48i5+irOg1Pk0fzdJ1Hp8wwGbMWA4RHaNPMuQAzYNKjuOa0gpEfehXUzEXy00uRjaQZJrlFdx4A9YPWW1G/2+DjHg9sykwoeadRarEz3pRFc7nGmmN0jvtC+kzLtIL+h8/m+R0w2ZVRVM4HtfObxBb4EO1mzty9FRX6/8hDVSMGugRoc+B/abZPtsgoQwXLH8Holw/k7orWXi4+nzpl15RKM11RBacrDS1pamN8NIiZhHuY7xJLwY91vfr9cZe4B5zOoa 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: List-Subscribe: List-Unsubscribe: On Thu, Dec 7, 2023 at 5:50=E2=80=AFAM Kefeng Wang wrote: > > > > On 2023/12/7 4:47, Paul Moore wrote: > > On Wed, Dec 6, 2023 at 9:22=E2=80=AFAM Ondrej Mosnacek wrote: > >> > >> (Restoring part of the Cc list to include more relevant lists & > >> people... If you are lost, the original email is here: > >> https://lore.kernel.org/selinux/20230728050043.59880-4-wangkefeng.wang= @huawei.com/) > >> > >> On Tue, Aug 1, 2023 at 1:08=E2=80=AFAM Paul Moore wrote: > >>> On Mon, Jul 31, 2023 at 4:02=E2=80=AFPM Stephen Smalley > >>> wrote: > >>>> On Mon, Jul 31, 2023 at 12:19=E2=80=AFPM Paul Moore wrote: > >>>>> > >>>>> On Mon, Jul 31, 2023 at 10:26=E2=80=AFAM Stephen Smalley > >>>>> wrote: > >>>>>> I believe this patch yields a semantic change in the SELinux exech= eap > >>>>>> permission check. That said, I think the change is for the better. > >>>>> > >>>>> Agreed. I'm also in favor of using a helper which is maintained by > >>>>> the VM folks over open coded logic in the SELinux code. > >>>> > >>>> Yes, only caveat is in theory it could trigger new execheap denials > >>>> under existing policies. > >>>> Trying to construct an example based on the > >>>> selinux-testsuite/tests/mmap/mprot_heap.c example but coming up empt= y > >>>> so far on something that both works and yields different results > >>>> before and after this patch. > >>> > >>> My gut feeling is that this will not be an issue, but I could very > >>> well be wrong. If it becomes a significant issue we can revert the > >>> SELinux portion of the patch. > >>> > >>> Of course, if you have any luck demonstrating this with reasonable > >>> code, that would be good input too. > >> > >> So, it turns out this does affect actual code. Thus far, we know about > >> gcl [1] and wine [2]. The gcl case is easy to reproduce (just install > >> gcl on Fedora and run gcl without arguments), so I was able to dig a > >> bit deeper. > >> > >> gcl has the following relevant memory mappings as captured by gdb: > >> Start Addr End Addr Size Offset Perms objfile > >> 0x413000 0xf75000 0xb62000 0x3fa000 rw-p > >> /usr/lib/gcl-2.6.14/unixport/saved_ansi_gcl > >> 0xf75000 0xf79000 0x4000 0x0 rwxp [heap] > >> > >> It tries to call mprotect(0x883000, 7282688, > >> PROT_READ|PROT_WRITE|PROT_EXEC), i.e. it tries to make the region > >> 0x883000 - 0xf75000 executable. Before this patch it was allowed, > >> whereas now it triggers an execheap SELinux denial. But this seems > >> wrong - the affected region is merely adjacent to the [heap] region, > >> it does not actually overlap with it. So even if we accept that the > >> correct semantics is to catch any region that overlaps with the heap > >> (before only subregions of the heap were subject to the execheap > >> check), this corner case doesn't seem to be handled correctly by the > >> new check (and the same bug seems to have been in fs/proc/task_mmu.c > >> before commit 11250fd12eb8 ("mm: factor out VMA stack and heap > >> checks")). > > Yes, the heap check exists for a long time, > > [start_brk brk] > case1: [vm_start,vm_end] > case2: [vm_start,vm_end] > case3: [vm_start,vm_end] > case4: [vm_start, vm_end] > > case5: [vm_start,vm_end] > case6: [vm_start,vm_end] > > old check: > vma->vm_start >=3D vma->vm_mm->start_brk && vma->vm_end <=3D vma->vm_mm->= brk > > Only include case1=EF=BC=8C vma range must be within heap > > new check: > vma->vm_start <=3D vma->vm_mm->brk && vma->vm_end >=3D vma->vm_mm->start_= brk > > Include case1~case6, but case5(vm_end=3Dstart_brk) and case6(vm_start=3Db= rk) > are the corner cases, gcl issue matchs the case5. > > >> > >> I didn't analyze the wine case ([2]), but it may be the same situation= . > >> > >> Unless I'm mistaken, those <=3D & >=3D in should in fact be just < & >= . > > I support this change. > vma->vm_start < vma->vm_mm->brk && vma->vm_end > vma->vm_mm->start_brk > > >> And the expression in vma_is_initial_stack() is also suspicious (but > >> I'm not going to make any assumption on what is the intended semantics > >> there...) > >> > >> [1] https://bugzilla.redhat.com/show_bug.cgi?id=3D2252391 > >> [2] https://bugzilla.redhat.com/show_bug.cgi?id=3D2247299 > > Could you quickly verify after the above change? Yes, changing the operators as suggested fixes the gcl case. BTW, the vma_is_*() helpers introduced by your patch also have wrong indentation - the "return" lines are indented by 7 spaces instead of a tab. If you are going to submit a fixing patch you might want to fix that, too. --=20 Ondrej Mosnacek Senior Software Engineer, Linux Security - SELinux kernel Red Hat, Inc.