Received-SPF: pass (google.com: domain of linux-kernel+bounces-179438-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Wed, 15 May 2024 00:53:47 -0400
From: "Liam R. Howlett" <Liam.Howlett@oracle.com>
To: Theo de Raadt <deraadt@openbsd.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>,
        Matthew Wilcox <willy@infradead.org>, Jonathan Corbet <corbet@lwn.net>,
        Andrew Morton <akpm@linux-foundation.org>, jeffxu@chromium.org,
        keescook@chromium.org, jannh@google.com, sroettger@google.com,
        gregkh@linuxfoundation.org, usama.anjum@collabora.com,
        surenb@google.com, merimus@google.com, rdunlap@infradead.org,
        jeffxu@google.com, jorgelo@chromium.org, groeck@chromium.org,
        linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
        linux-mm@kvack.org, pedro.falcato@gmail.com, dave.hansen@intel.com,
        linux-hardening@vger.kernel.org
Subject: Re: [PATCH v10 0/5] Introduce mseal
Message-ID: <6wtyezhn7spm4ehpo7jyi6xy3ywznkztjpzlafodpwj6x4wldx@rjia2otxbcxu>
Mail-Followup-To: "Liam R. Howlett" <Liam.Howlett@oracle.com>, 
	Theo de Raadt <deraadt@openbsd.org>, Linus Torvalds <torvalds@linux-foundation.org>, 
	Matthew Wilcox <willy@infradead.org>, Jonathan Corbet <corbet@lwn.net>, 
	Andrew Morton <akpm@linux-foundation.org>, jeffxu@chromium.org, keescook@chromium.org, jannh@google.com, 
	sroettger@google.com, gregkh@linuxfoundation.org, usama.anjum@collabora.com, 
	surenb@google.com, merimus@google.com, rdunlap@infradead.org, jeffxu@google.com, 
	jorgelo@chromium.org, groeck@chromium.org, linux-kernel@vger.kernel.org, 
	linux-kselftest@vger.kernel.org, linux-mm@kvack.org, pedro.falcato@gmail.com, 
	dave.hansen@intel.com, linux-hardening@vger.kernel.org
References: <20240514104646.e6af4292f19b834777ec1e32@linux-foundation.org>
 <871q646rea.fsf@meer.lwn.net>
 <ZkPXcT_JuQeZCAv0@casper.infradead.org>
 <56001.1715726927@cvs.openbsd.org>
 <CAHk-=wgsGCKvN0Db6ZRZqJwXQrmhZyWB6RmABaOp4DiZbXgNew@mail.gmail.com>
 <16982.1715734632@cvs.openbsd.org>
 <CAHk-=wi04ZCm3vTtkcVnAUdiOpX3a0hBZ-aQWONwCubOJQEdXw@mail.gmail.com>
 <84192.1715737666@cvs.openbsd.org>
 <CAHk-=wj=Yb1r_g=geQe8YDGv-TA-UQBQe6-OVhA8FOGjWKnQmA@mail.gmail.com>
 <75628.1715740958@cvs.openbsd.org>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <75628.1715740958@cvs.openbsd.org>
User-Agent: NeoMutt/20231103
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?us-ascii?Q?1jlLKpl+GW1uu/1C43LBgYPTRAN/5CCUPbvDftEItC+xRW5vt/SbQdviLQG+?=
 =?us-ascii?Q?9zbSLkbjKML10rsimUHPQm0bf4brFejkhnSp0QLWeY8IQ3735gJN5tJzJiyZ?=
 =?us-ascii?Q?0/bSQ2XUhpMyCGdsBtOXIh1bQML/ME3Sxr+wz8DSz6y4ShcEOITFiqwdDqPW?=
 =?us-ascii?Q?mhBPRjSgQad1KVnmDZm1IT9FYxeJ2avtYKQyed2QS6tUSCjNwHWlHOXsRCh9?=
 =?us-ascii?Q?EwQ2rZbA5PnOc5warj0YVLEXXfdT0MKoRe3ZafoHfBq4ZzeltNi0RpC9eyfg?=
 =?us-ascii?Q?Iq9wHozaSYQWizvarjPVS4ifawMC9W8GZhK+dCKcqa9d44Vp3mXdr0gOSDZq?=
 =?us-ascii?Q?QkJ2MNrG9Uswuec4pXCF8yWHjZ/tdfQxrzzDgUNto6K0eYJtL6oXor+DB66P?=
 =?us-ascii?Q?dPtZIur7QVLYFKymoHkC15Aj08GUcFEoLezKlAQSmpYWgA5lgAyZUb73GC6Z?=
 =?us-ascii?Q?2hZ5/LaAJao79lB0HgQOuQT1iGCACAxcUGM+6JNIouFUBAL76Cty7bp5u22G?=
 =?us-ascii?Q?4zRAy7ZZ9wAVDAu15HKkfJQa2BsZUwSdXh6z6xaysd3Nkx5QQUf7Dn+fGAtl?=
 =?us-ascii?Q?XvH9uqnrsW4lQdm2LqDUGoCmXa4Sl7VfRjzlUw+6RWUaIrgmChA/ld3XOEGF?=
 =?us-ascii?Q?GRyfxrqsqG0YNbRYHU9sitAuHRyC8/VIpMjFJOfGhmNmxakVtkKSXOfJhp8x?=
 =?us-ascii?Q?zWh4A+KNdvEM5hdgOeYMVeq9EkbSJCQGnso/4XeSMqggP5CgPSgmiuZObgtj?=
 =?us-ascii?Q?JCU56aX59/pT8pIwFs7iS2bq9PaCLvJBYdrrQsC8ngtuR0+A0vfPoLAT4eDy?=
 =?us-ascii?Q?2s490paZWxL2lObX/cYYSO1HdS4I0bj0tMJ9QtB1Uz4O1yE3s+1i8c4RFZF1?=
 =?us-ascii?Q?xdO1oHpontfhLeuvhexfG5t55s5hP0BfhgYKTp1MYTIa94rOw6/CblMvrYOL?=
 =?us-ascii?Q?9Nl1T8RAj+CXWp+3k/E772bPr07Gp+uVRlLWKyBS1GzXq606rfwBz4CVUHm1?=
 =?us-ascii?Q?l9cSaGtrazPbBLMxayhtN398EyKZ3IziZnwAv20sLKUcmOZdiOGr+TN39neX?=
 =?us-ascii?Q?Z0eRVaomw76JNUCenmq9E8dVjJSSpv0NFgX9IsarLW7X+oBIkIJodKhtIRUc?=
 =?us-ascii?Q?4ti/mWobE+bES/0A1MDw5cVlv0syU6oKHT1kCz7VKB2H6YDBxr2Qp+5mWnWy?=
 =?us-ascii?Q?1Qg6d0NrEHlaQ3lYCwjisIZhfOKh2S3VJW/czPLaoXp3MQh6TjUGiqzEwrfl?=
 =?us-ascii?Q?p3Saa5XEak/bdpbYMPwbEpO4dK25XrccwZdvg1Caxci9nTVX7SgCVmNMvXMb?=
 =?us-ascii?Q?ySh/acZGzeY8YZ6hIdieM0B2r4d8TMqDKZPkVdAIySPGF7e4WvR0F2UJGmRk?=
 =?us-ascii?Q?ndaHLzqfhqaONfjB8f9QRUIDDYn7sZod+dRFJQR2KBlJPU5Fu4PEdA/2253+?=
 =?us-ascii?Q?djr1YiQzF9iT5MtfasKRACe4gWW5uNimgDXhPxh3BN8b+jTjrhVRfwlqurNP?=
 =?us-ascii?Q?DiT8GMqOaAJj6sxiDv/V82X9+mE268WnFhGwwIjOVNxxvSXV32dQRHUlX1K1?=
 =?us-ascii?Q?5f84I3dKX0u9qQnmq8x8/BLm555+RPHAToh7/Dfi?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	/rKO52+sMASAPUuUPzXMtfZ57suMbqxFCSF3HKfTRJ9hjk45gAbclXGcmOZC+S0MaM5heh8atOf3hfOxas0qDFtw3y9qRoN2lfD+6MDGtOJl4WS4W28JbfkHGZy/eA4w8VoV9v7FN4QnyFpB+C/Xh4PjJ++oxcddaEVkSMuZV4F18v+zVE410JszlGWwbtNTI2f/R4XfesSX2t2XWVHJsUw/QXk7oDf6kVs76fSgWyT4Map4k3VqrADNbkzSquYyj+8cTWcVT+LbHPkwZ/TEwMqueiauPznIUOdz0p2jkPnVoicMDgAzvAmZSfj2Jd/UM0zsFinZyNxGkZw+tTLLRC2VI439Gt5hrZC/9a0uY+dnCtCR+xsbZhmeabgz+6jK9jQV11PscOOHQ52QY5EXcwgn8KAl80vUqG+T93beuaJrl9/m1Cv/Tz8vABZu2AChrG5HVsJVI/po0RirtibA4D5IN2NRVKGArTBJqFwgo7fmRcv7hxdkEZgbsc/JNCWbDJD3UdfLR3rRCi7hUzPlDOBH2RfPA2GJdj6lf6Z2id7GURnhaqCh/uBXlYYs84xZCPP7+fGVFota5dS7pM7GDMK56nMuZHX03wxLJuq1vqo=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 9417018a-e42d-418a-136e-08dc749b0301
X-MS-Exchange-CrossTenant-AuthSource: DS0PR10MB7933.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 15 May 2024 04:53:50.0348
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: tsjRDvCY4nMnaGE4lvd/Jns/LxhBgS5+Gj90o1gbku77VpLLHM3BJ93EjgIcHd3hTOdNYvohiYrAU0mS0z5EPw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL3PR10MB6259
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.293,Aquarius:18.0.1039,Hydra:6.0.650,FMLib:17.11.176.26
 definitions=2024-05-15_01,2024-05-14_01,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 suspectscore=0 mlxlogscore=826
 mlxscore=0 spamscore=0 bulkscore=0 phishscore=0 malwarescore=0
 adultscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2405010000 definitions=main-2405150033
X-Proofpoint-ORIG-GUID: 5-uaNppjfRwVAA-AvDfzre-02S0sFS99
X-Proofpoint-GUID: 5-uaNppjfRwVAA-AvDfzre-02S0sFS99

* Theo de Raadt <deraadt@openbsd.org> [240514 22:42]:
> Linus Torvalds <torvalds@linux-foundation.org> wrote:
> 
> > On Tue, 14 May 2024 at 18:47, Theo de Raadt <deraadt@openbsd.org> wrote:
> > >
> > > Linus Torvalds <torvalds@linux-foundation.org> wrote:
> > >
> > > Regarding mprotect(), POSIX also says:
> > >
> > >     An implementation may permit accesses other than those specified by
> > >     prot; however, no implementation shall permit a write to succeed where
> > >     PROT_WRITE has not been set or shall permit any access where PROT_NONE
> > >     alone has been set.
> > 
> > Why do you quote entirely irrelevant issues?
> > 
> > If the mprotect didn't succeed, then clearly the above is irrelevant.
> 
> Imagine the following region:
> 
> 
>     <--------------------------------------------- len
>     [region PROT_READ] [region PROT_READ + sealed] 
> addr ^
> 
> then perform
>     mprotect(addr, len, PROT_WRITE | PROT_READ);
> 
> This will return -1, with EPERM, when it encounters the sealed region.
> 
> I believe in Linux, since it has not checked for errors as a first
> phase, this changes the first region of memory to PROT_READ |
> PROT_WRITE.  Liam, is that correct?

I really don't want to fight about this - I just want to have reliable
code that is maintainable.  I think the correctness argument is always
going to be unclear because we're all going to interpret the
documentation from our point of view - which is probably how we got here
in the first place.  My opinion of the matter of correctness is,
obviously, the least important.

My problem right now is that we're changing it so that we are not
consistent in when we should check.  I'm not sure how doing both fits
into either model, but it increases the next change going to the 'wrong'
side of the argument (whatever side that happens to be from your view).

If there isn't a technical reason to keep the check before, then we
should treat mseal the same as all other checks.

If we are going to have an up-front check, then it makes sense to keep
the checks that we can (reasonably) do at the same time together.
Linus, you said up front checks is a good thing to aim for.

That said, I don't think the example above will allow the madvise to
succeed at all.  mseal checks the entire region up front while most
other checks occur during the loop across vmas.

Thanks,
Liam