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 5B222C7EE26 for ; Tue, 23 May 2023 09:12:46 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id CDDCF900003; Tue, 23 May 2023 05:12:45 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id C8EB7900002; Tue, 23 May 2023 05:12:45 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id B7D54900003; Tue, 23 May 2023 05:12:45 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0016.hostedemail.com [216.40.44.16]) by kanga.kvack.org (Postfix) with ESMTP id A80DB900002 for ; Tue, 23 May 2023 05:12:45 -0400 (EDT) Received: from smtpin06.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay08.hostedemail.com (Postfix) with ESMTP id 7A7661406EF for ; Tue, 23 May 2023 09:12:45 +0000 (UTC) X-FDA: 80820954690.06.447B83A Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by imf25.hostedemail.com (Postfix) with ESMTP id A4D16A0013 for ; Tue, 23 May 2023 09:12:42 +0000 (UTC) Authentication-Results: imf25.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=XiSlec67; spf=pass (imf25.hostedemail.com: domain of david@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=david@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1684833163; 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=jnRPg7QY2jmRwV2BnlPo6Mcti7P+VISSM7w+qVsNvmc=; b=1ybzl8OzS/31+NtWB6DarCQoWrEtFVETkRY6alDg/S4pDKi0btBCCOh/wcrshN//K9WAP3 SPhLWo9GuKy4+MKK69u7fVGOpS8Tf9v204CZgmp3PgV6EaBUFVF84X/2eEqHvQKrth9oL3 ptunlgWbEZfJtkHm2RFP6clRZ7JWxn4= ARC-Authentication-Results: i=1; imf25.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=XiSlec67; spf=pass (imf25.hostedemail.com: domain of david@redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=david@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1684833163; a=rsa-sha256; cv=none; b=qhS0PGit7Q9XsHfngzzMTqldX+6nzMQyQmvoWXBCNInEljF/MH7WnQFO2nGus2jtqNo+LA pcVKf5kG5QhnLlN7AYnTpU3zr6F80WQJ61wZMeeU0W8z4Ndqkjguq456B2MiqeJWA3Dg5B dOJIJ66Neku2EfuMTWsA3P/bBTd7/tQ= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1684833161; 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=jnRPg7QY2jmRwV2BnlPo6Mcti7P+VISSM7w+qVsNvmc=; b=XiSlec67OSl+5B9kolvuBOOqdsm/ZSS+vETAWV3eTlyPL6nIzOJOmUWvJHwetX6UuuaSWK EP045Em6YB2iFYFSxJle0B0E17EZIRdN4liNlQemlBYwf37AI4dYfbu6+QtI7V+tk+L63a aqw68KQTLAVUQoLXTWvbyr7yjwpsvIg= Received: from mail-wr1-f71.google.com (mail-wr1-f71.google.com [209.85.221.71]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-413-nBC59O7oPQCquYL1L9hMNA-1; Tue, 23 May 2023 05:12:40 -0400 X-MC-Unique: nBC59O7oPQCquYL1L9hMNA-1 Received: by mail-wr1-f71.google.com with SMTP id ffacd0b85a97d-30a8ba2debdso865920f8f.3 for ; Tue, 23 May 2023 02:12:40 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1684833159; x=1687425159; h=content-transfer-encoding:in-reply-to:subject:organization:from :references:cc:to:content-language:user-agent:mime-version:date :message-id:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=jnRPg7QY2jmRwV2BnlPo6Mcti7P+VISSM7w+qVsNvmc=; b=LmlbVlajWKJa3gAKjZb+X+WO3nbansgDbpTcBsfCWpECFxD7CM3Ow4rO/+AWLL4Msc VDw6m4icanoi0AC/D7f+PFPdsH3OHZiicl5N2+cDJsQ7kunqFuNRc0sfwKU03OZeu4LC clzLzkbaKYqrQ3YZ49OOo5O4ecDJHBB4d9lT8aqxDsGKHA5DzYXFXgAh747wdE2EY8ul R6rf8fVH+91fJ/6hZ6v3jlAMxIU+sIptD5xBlnjxGPeRPmBo9MVYl3mL1E32CySA4uny 1tdcXVeDMmoW9X0FdyTDbuU4RsSa0VEt6JtgIfI697oSfWeYt/VFbotnvzflP36/F1hu vk4g== X-Gm-Message-State: AC+VfDwwzjFWV6mPGjCxuv40g+bAC9JKo9CJoLKfn4WZNQ+Q910szmcQ zVf09m3ViCqOiGy4X29hGFpgIrw6OBdi2Qmljr/bbGPIJAJUckRE2UYqbDqWStDEIbVGlVII4ZP MFitroyB16UE= X-Received: by 2002:a5d:4e8c:0:b0:309:4642:8773 with SMTP id e12-20020a5d4e8c000000b0030946428773mr10054996wru.55.1684833159527; Tue, 23 May 2023 02:12:39 -0700 (PDT) X-Google-Smtp-Source: ACHHUZ5DElfwciEr1wIsTm3SdAMxxR1WaQ5j0YnhXCadJrC6nCvLteEd4TgO+w2gRb7AK1f623mTXg== X-Received: by 2002:a5d:4e8c:0:b0:309:4642:8773 with SMTP id e12-20020a5d4e8c000000b0030946428773mr10054968wru.55.1684833159103; Tue, 23 May 2023 02:12:39 -0700 (PDT) Received: from ?IPV6:2003:cb:c74c:b400:5c8b:a0b2:f57e:e1cd? (p200300cbc74cb4005c8ba0b2f57ee1cd.dip0.t-ipconnect.de. [2003:cb:c74c:b400:5c8b:a0b2:f57e:e1cd]) by smtp.gmail.com with ESMTPSA id k1-20020adfd841000000b002f28de9f73bsm10365717wrl.55.2023.05.23.02.12.37 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 23 May 2023 02:12:38 -0700 (PDT) Message-ID: Date: Tue, 23 May 2023 11:12:37 +0200 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.10.0 To: Alexey Izbyshev Cc: Florent Revest , linux-kernel@vger.kernel.org, linux-mm@kvack.org, akpm@linux-foundation.org, catalin.marinas@arm.com, anshuman.khandual@arm.com, joey.gouly@arm.com, mhocko@suse.com, keescook@chromium.org, peterx@redhat.com, broonie@kernel.org, szabolcs.nagy@arm.com, kpsingh@kernel.org, gthelen@google.com, toiwoton@gmail.com References: <20230517150321.2890206-1-revest@chromium.org> <20230517150321.2890206-4-revest@chromium.org> <884d131bbc28ebfa0b729176e6415269@ispras.ru> <3c2e210b75bd56909322e8a3e5086d91@ispras.ru> From: David Hildenbrand Organization: Red Hat Subject: Re: [PATCH v2 3/5] mm: Make PR_MDWE_REFUSE_EXEC_GAIN an unsigned long In-Reply-To: <3c2e210b75bd56909322e8a3e5086d91@ispras.ru> X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Language: en-US Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit X-Rspamd-Queue-Id: A4D16A0013 X-Rspam-User: X-Stat-Signature: swasmb4ykhusmi68tch1iptw1i73fwj3 X-Rspamd-Server: rspam01 X-HE-Tag: 1684833162-901222 X-HE-Meta: U2FsdGVkX19xYThIVqealqblkzlHOFBmjDR3/x4fvmtJEzDjPhtlj1dDf4WKOo7IZx+llxA+Kgmvtm3QHXNTrT/vGNkHBBMdEiTTEYItOCdDGhI9YLr9gu4c0svmqDLUglcEaM0g48sdKNS64FRzutWeKZNSWBjzSmRTT4PquPrxKCcTScV0bJdFT/GEYYHwzJn1rPlCoMGYUD/8qLZMEVeICpCNk1ZM2SFiUWUyKzFM631klKavQGjq6NebhhRc7zGT1PBRpRS9/92Hfe4MsnuAWMMBL+vxl6QBt4JM0asN0Wecqna08GCMM3F42ZXfxXu04lm1buzXc2B82EEjhIBr+FzzfcHgLUnyfxFiZa8idi5v2J8tnwUfedm/pWaF2Auv8oP1JCZSKpCtLHdpqWlJb/noSv75yO2q0CfI/hyb4zZDaCulBQvL6Orbik1FOwdXo26riLcAIxzKf081XPQvnUHZx24x1Za1yTXDxxDYZuD7MjmwSgeJe0TqzDU0mI5rBt17t8Dxopt7nP1/rjvpdRx52Fbi/o+5nsDDFySDs4uHfMzWeLIUnOPknJw5IVft3YXytUEHZfToj6k1A0Hpw/goKY7pJ+0V/kOsjq9wgTtOZ3zRrBT639rHcMgEBhjt4kijhj8vErAgTtkTUfNeSstIIrqAbOG64VoPT5svPTVF6VGw2M6SKzHukxAaIDXxYJGwk3g/KqOfVKd4PgAiSPxMhwbJcxs5YbCc8jxicKgOztz5jaN67J+evW2LlHVMGjiKcK5g08BfNb5wa0knaP6CnHx0B/ZTRC+aRnHBtfe+ETKCxLCYm3Mcre+KhC+mYRcCUKh6iJIBj3fARvieFQg8lbzbp0j91UanNceUqsc1mBBo5vRI8yj7eDaGVFiPmT89jmXzwQ79v3WYl+zEsC8/hPeJvZnDulW11IO6BiVMrZkp77aKY1CPpjGKNWIDKDhDTouPhJUk4R/ 5ytN3zwG sMs8b1F7SwWIHUxvJdOjoybI37CC9j0Dk+Ldn7ujoWAKo9qtDR0jlIeh9U059wrzk7wdXYvVD3P6ebwra9tXlUe2SFAD3iPzmEnV9GrOWMd1e5V/mJpbgKCFvPOj7Pj+r+AmIH/03t+0K8jM7ljJOg3f+fDCpLL0WhXyrnh9nG5pdpa+qVSAtlpN9sTSuRsH/0yYykTeJK3FfzlKlRqEj+DFftmCGeIlluxH6YENkPR03SumsZJtU3Su5TX6ZxNFI1aYOITLWGM8HA570TU0UxzKptYFP+NYITkCVP8376356tAiq/Au1EFGecouJaPjz2v3Cd11nWNNpOtDqkwUa/2EgQKdunVP7LYnFpnR+yPJeKSLn1iuM0Dn7veaYhhPKxfHUB++lNK+5NcGtnE64VU4ApOhHZ2P9O4JD4MzQ6nZpHlQW73rpH+6KHsQTj0i+qqWd+++FtQKFFkasrwFPciIC98sKmMUwIDlDLhdi2HvXK8wPeCAkYkPZSrnY6enAAs+lAf0xlh17fWKsZxpQESVTZ4iub1ekpWMGDRwbDfHNSnQ= 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 22.05.23 20:58, Alexey Izbyshev wrote: > On 2023-05-22 19:22, David Hildenbrand wrote: >> On 22.05.23 12:35, Alexey Izbyshev wrote: >>> On 2023-05-22 11:55, David Hildenbrand wrote: >>>> On 17.05.23 17:03, Florent Revest wrote: >>>>> Alexey pointed out that defining a prctl flag as an int is a footgun >>>>> because, under some circumstances, when used as a flag to prctl, it >>>>> can >>>>> be casted to long with garbage upper bits which would result in >>>>> unexpected behaviors. >>>>> >>>>> This patch changes the constant to a UL to eliminate these >>>>> possibilities. >>>>> >>>>> Signed-off-by: Florent Revest >>>>> Suggested-by: Alexey Izbyshev >>>>> --- >>>>> include/uapi/linux/prctl.h | 2 +- >>>>> tools/include/uapi/linux/prctl.h | 2 +- >>>>> 2 files changed, 2 insertions(+), 2 deletions(-) >>>>> >>>>> diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h >>>>> index f23d9a16507f..6e9af6cbc950 100644 >>>>> --- a/include/uapi/linux/prctl.h >>>>> +++ b/include/uapi/linux/prctl.h >>>>> @@ -283,7 +283,7 @@ struct prctl_mm_map { >>>>> /* Memory deny write / execute */ >>>>> #define PR_SET_MDWE 65 >>>>> -# define PR_MDWE_REFUSE_EXEC_GAIN 1 >>>>> +# define PR_MDWE_REFUSE_EXEC_GAIN (1UL << 0) >>>>> #define PR_GET_MDWE 66 >>>>> diff --git a/tools/include/uapi/linux/prctl.h >>>>> b/tools/include/uapi/linux/prctl.h >>>>> index 759b3f53e53f..6e6563e97fef 100644 >>>>> --- a/tools/include/uapi/linux/prctl.h >>>>> +++ b/tools/include/uapi/linux/prctl.h >>>>> @@ -283,7 +283,7 @@ struct prctl_mm_map { >>>>> /* Memory deny write / execute */ >>>>> #define PR_SET_MDWE 65 >>>>> -# define PR_MDWE_REFUSE_EXEC_GAIN 1 >>>>> +# define PR_MDWE_REFUSE_EXEC_GAIN (1UL << 0) >>>>> #define PR_GET_MDWE 66 >>>>> >>>> >>>> Both are changing existing uapi, so you'll already have existing user >>>> space using the old values, that your kernel code has to deal with >>>> no? >>> >>> I'm the one who suggested this change, so I feel the need to clarify. >>> >>> For any existing 64-bit user space code using the kernel and the uapi >>> headers before this patch and doing the wrong prctl(PR_SET_MDWE, >>> PR_MDWE_REFUSE_EXEC_GAIN) call instead of the correct >>> prctl(PR_SET_MDWE, >>> (unsigned long)PR_MDWE_REFUSE_EXEC_GAIN), there are two possibilities >>> when prctl() implementation extracts the second argument via >>> va_arg(op, >>> unsigned long): >>> >>> * It gets lucky, and the upper 32 bits of the argument are zero. The >>> call does what is expected by the user. >>> >>> * The upper 32 bits are non-zero junk. The flags argument is rejected >>> by >>> the kernel, and the call fails with EINVAL (unexpectedly for the >>> user). >>> >>> This change is intended to affect only the second case, and only after >>> the program is recompiled with the new uapi headers. The currently >>> wrong, but naturally-looking prctl(PR_SET_MDWE, >>> PR_MDWE_REFUSE_EXEC_GAIN) call becomes correct. >>> >>> The kernel ABI is unaffected by this change, since it has been defined >>> in terms of unsigned long from the start. >> >> The thing I'm concerned about is the following: old user space (that >> would fail) on new kernel where we define some upper 32bit to actually >> have a meaning (where it would succeed with wrong semantics). >> >> IOW, can we ever really "use" these upper 32bit, or should we instead >> only consume the lower 32bit in the kernel and effectively ignore the >> upper 32bit? >> > I see, thanks. But I think this question is mostly independent from this > patch. The patch removes a footgun, but it doesn't change the flags > check in the kernel, and nothing stops the user from doing > > int flags = PR_MDWE_REFUSE_EXEC_GAIN; > prctl(PR_SET_MDWE, flags); > > So we have to decide whether to ignore the upper 32 bits or not even if > this patch is not applied (actually *had to* when PR_SET_MDWE op was > being added). Well, an alternative to this patch would be to say "well, for this prctl we ignore any upper 32bit. Then, this change would not be needed. Yes, I also don't like that :) Bu unrelated, I looked at some other random prctl. #define SUID_DUMP_USER 1 And in kernel/sys.c: case PR_SET_DUMPABLE: if (arg2 != SUID_DUMP_DISABLE && arg2 != SUID_DUMP_USER) ... Wouldn't that also suffer from the same issue, or how is this different? Also, how is passing "0"s to e.g., PR_GET_THP_DISABLE reliable? We need arg2 -> arg5 to be 0. But wouldn't the following also just pass a 0 "int" ? prctl(PR_GET_THP_DISABLE, 0, 0, 0, 0) I'm easily confused by such (va_args) things, so sorry for the dummy questions. -- Thanks, David / dhildenb