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 DFB4FC4167B for ; Thu, 7 Dec 2023 04:50:57 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 55C7A6B0080; Wed, 6 Dec 2023 23:50:57 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 4E5976B0085; Wed, 6 Dec 2023 23:50:57 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 385B26B0087; Wed, 6 Dec 2023 23:50:57 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0010.hostedemail.com [216.40.44.10]) by kanga.kvack.org (Postfix) with ESMTP id 2578E6B0080 for ; Wed, 6 Dec 2023 23:50:57 -0500 (EST) Received: from smtpin08.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id EFC37A0194 for ; Thu, 7 Dec 2023 04:50:56 +0000 (UTC) X-FDA: 81538797312.08.4C7D7F3 Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by imf27.hostedemail.com (Postfix) with ESMTP id BA50E40013 for ; Thu, 7 Dec 2023 04:50:52 +0000 (UTC) Authentication-Results: imf27.hostedemail.com; dkim=none; spf=pass (imf27.hostedemail.com: domain of wangkefeng.wang@huawei.com designates 45.249.212.187 as permitted sender) smtp.mailfrom=wangkefeng.wang@huawei.com; dmarc=pass (policy=quarantine) header.from=huawei.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1701924653; 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; bh=PMMT7L+YJT7W79hXuQqSELecQkoOHrdWfZreChBqg0k=; b=sYCpdggjiSU7eue5505npWImcfZ4j6aQavhHv6SWpzRcWznM7nW3WXGBC9uN/KExea5kbH VCfXc8QGiPqDojNYkDvfPETulyW1O4Rg6hu7aMXZEvczT+MXgPqvK49PfLysWAP+sZrU0p TplkoUP5AX/CqtBao4G1WYSQpqqBe8o= ARC-Authentication-Results: i=1; imf27.hostedemail.com; dkim=none; spf=pass (imf27.hostedemail.com: domain of wangkefeng.wang@huawei.com designates 45.249.212.187 as permitted sender) smtp.mailfrom=wangkefeng.wang@huawei.com; dmarc=pass (policy=quarantine) header.from=huawei.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1701924653; a=rsa-sha256; cv=none; b=WtnUYSp2VqyDhY0WdmQ+J96a4d8ey1HDjodZaMfTNFrLL4rpdgDmXII0ce60RzBai/ti9d QjPiEVWDx25vBqXQZx+6XvWNXaCFELFlLZ+P/zIPgkR68YI6Eti7NrSV9KoC64Auy8PkZ3 A2DrnPDxkdanz4RoHcyYBoPOKKpbkVQ= Received: from dggpemm100001.china.huawei.com (unknown [172.30.72.53]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4Sm1zN2BmbzYspL; Thu, 7 Dec 2023 12:50:08 +0800 (CST) Received: from [10.174.177.243] (10.174.177.243) by dggpemm100001.china.huawei.com (7.185.36.93) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2507.35; Thu, 7 Dec 2023 12:50:47 +0800 Message-ID: <79f6131f-7d69-408d-917a-f0ca6fccc5b2@huawei.com> Date: Thu, 7 Dec 2023 12:50:47 +0800 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird From: Kefeng Wang Subject: Re: [PATCH v3 3/4] selinux: use vma_is_initial_stack() and vma_is_initial_heap() To: Paul Moore , Ondrej Mosnacek CC: Stephen Smalley , SElinux list , "open list:MEMORY MANAGEMENT" , David Hildenbrand , , Andrew Morton References: <20230728050043.59880-1-wangkefeng.wang@huawei.com> <20230728050043.59880-4-wangkefeng.wang@huawei.com> Content-Language: en-US In-Reply-To: Content-Type: text/plain; charset="UTF-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.174.177.243] X-ClientProxiedBy: dggems706-chm.china.huawei.com (10.3.19.183) To dggpemm100001.china.huawei.com (7.185.36.93) X-CFilter-Loop: Reflected X-Rspamd-Queue-Id: BA50E40013 X-Rspam-User: X-Stat-Signature: eppwkm4ro78ra7nwkr66wgkycqqbyqkt X-Rspamd-Server: rspam01 X-HE-Tag: 1701924652-976800 X-HE-Meta: U2FsdGVkX1/ZwmKi5qwYqsH3HywetwRcC2XFji4nPbpselBo4CG77iCpxztevtyurd0aLK8KJJjDe4gArn1vP+N6raiBu83U1GZGAUCsX28MaiqUL14psAGlIoBb2FwH/93UCaC2otVc+XERv0UTw7LOK2Ul71MKqAobVP78fgQGO030iS2EeP3lMcLcID1SEdn/lcJKmt1gHWnApbD6g1GtgZYoPHqSUXDGoBXb20dZk0EBK9RhsGBoGoXDQMQiUjSghQ2T5tVhMHsIacqsm+54PpK0wPHtCAdnJb6psclAVeCAa0iOdYc1G9nRg/6/A5YMESH2ReaR6Ov/KvA/Di1nWm2ni4awjw5i20pHqxiiSCD4YmLh/9Re0a8jjJKO3na1WIQpmdr+PaDykgxwczaNAAwy/DpIMqalfrQC7crQcQEuWGOCzrUM2gbkagv59+0yh+bu41J0+ieZMH8PckXHS2Gwx7n/8a8gGijTEGYlD5dWQVmo2dro3/Tj3MBCCtHkSAw59V7JKHsZdOAuNZLc9q2GocrofSsRRV77I3iWmLsooac9MszpKZgX83OOfYLcZCUSfWzaU4pM9iohyaG8FDRDNodWU+TOK03xdSWvhewZfe+JqPwIXy2lsgGGJlQThLX9tPS88kMXFi9MZ4sHIDN3Vv3wXkSufRBr8KCvVsmx4BP9e99yyxLDBXZGhHII62dzQkyvjS4ygOklEhPXp2Vgp68r8RY11huqOINP52fdEVhTQI+x+m4LPfy9rDKIKLtfakr5hnfV2CP01l7PTgvtMcaUXvk/wsa2oSg5wwGkRL4PugJhip6tZEXDkDj+9INBY5UABK/PfGWYsgDz95SZMU1Q1CQ2u71hZPobj45L00yVjL7xUPqP8wayhWMO7KTDJ+2a74Xb0fWYVewAlXuaKnV37gd3RE/hMZo99vhn5HADx7FHCywhzdcsbBZKo1cXihp9+I86d52 eRsvKF0l I4ERG+IKLKyjyAq9M9RlXYNtrqDcXVlyGoWr4ZJMgySRfYlc9BDCj4zm/KJYI7TZIyXOG9nVTybvbllWdppP03nX0CCbOiGjBEhaXV0LSXITOnJxcnVkjTMImSPbBqZdBi2Geflfe7DqKjRrXjyLYkKJcYhbdp/f+WJeJiFqNdIpF2zKg2BJ48kY1xftHbC70o8yJKfcH5nMljrt7SjzXQ4RSIfJPVvRJPvm0kenNw90B3myryAXjqHD+w+jS6S3ppPcRKU/r15dWZvuHxIOS6KmB02fCEfQrekgvmotUr/2LmD0HnGMOf04UCAgFT5b51kqP3gZiTk/ZiRUjf5z0W8+GfkLaRVtIDrm6g0pJfhOKN66BgwnJFw3rW+KQoBvcoVePo6Zrhp2VEubCjN67jC/G+Mc+b5nb5RpiABLjkXGSR8g+5aG1DDUdUwUMf3i3kWJs23LgBZqatwOMJnDQ/oGzwuYKbQaOFVEY7Hqz0AxVDhMNWhm9aoT0tA== 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 2023/12/7 4:47, Paul Moore wrote: > On Wed, Dec 6, 2023 at 9:22 AM 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 AM Paul Moore wrote: >>> On Mon, Jul 31, 2023 at 4:02 PM Stephen Smalley >>> wrote: >>>> On Mon, Jul 31, 2023 at 12:19 PM Paul Moore wrote: >>>>> >>>>> On Mon, Jul 31, 2023 at 10:26 AM Stephen Smalley >>>>> wrote: >>>>>> I believe this patch yields a semantic change in the SELinux execheap >>>>>> 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 empty >>>> 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 >= vma->vm_mm->start_brk && vma->vm_end <= vma->vm_mm->brk Only include case1， vma range must be within heap new check: vma->vm_start <= vma->vm_mm->brk && vma->vm_end >= vma->vm_mm->start_brk Include case1~case6, but case5(vm_end=start_brk) and case6(vm_start=brk) 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 <= & >= 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=2252391 >> [2] https://bugzilla.redhat.com/show_bug.cgi?id=2247299 Could you quickly verify after the above change? > > Thanks Ondrej. > > I'm hoping the mm folks will comment on this as it looks like this is > an issue with the helper functions, but just in case I'm going to prep > a revert for just the SELinux changes. If we don't hear anything in > the next couple of days I'll send the revert up to Linus with the idea > that we can eventually shift back to the helpers when this is sorted. >