Received-SPF: pass (google.com: domain of linux-kernel+bounces-35785-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.199.223 as permitted sender) client-ip=147.75.199.223;
Date: Tue, 23 Jan 2024 12:33:20 -0500
From: "Liam R. Howlett" <Liam.Howlett@Oracle.com>
To: Theo de Raadt <deraadt@openbsd.org>
Cc: Jeff Xu <jeffxu@chromium.org>, akpm@linux-foundation.org,
        keescook@chromium.org, jannh@google.com, sroettger@google.com,
        willy@infradead.org, gregkh@linuxfoundation.org,
        torvalds@linux-foundation.org, usama.anjum@collabora.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 v7 0/4] Introduce mseal()
Message-ID: <20240123173320.2xl3wygzbxnrei2c@revolver>
Mail-Followup-To: "Liam R. Howlett" <Liam.Howlett@Oracle.com>,
	Theo de Raadt <deraadt@openbsd.org>, Jeff Xu <jeffxu@chromium.org>,
	akpm@linux-foundation.org, keescook@chromium.org, jannh@google.com,
	sroettger@google.com, willy@infradead.org,
	gregkh@linuxfoundation.org, torvalds@linux-foundation.org,
	usama.anjum@collabora.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: <20240122152905.2220849-1-jeffxu@chromium.org>
 <726.1705938579@cvs.openbsd.org>
 <CABi2SkXrnUZsWvpqS61mHw-SqDBOodqpcfjdoTTyeeYG9tRJGA@mail.gmail.com>
 <86181.1705962897@cvs.openbsd.org>
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <86181.1705962897@cvs.openbsd.org>
User-Agent: NeoMutt/20220429
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: 
	=?utf-8?B?VGVYaXNNZVJwSldGQ3hoL0dlR1VJanFrRlEyR1Yra2F0NU11bUE2M2cwYjB6?=
 =?utf-8?B?RXNrejNFVWpvN1kvSUNHVHpTNkc5eDNEZ0JBU3VWKzZ2Zm5yeHdyOTRhNFdN?=
 =?utf-8?B?QkVqa0dIQlh3MWZvN01lemMwTFB2VUU0bTlJYkVWN1lyVUhZcE5NNmNNV1hm?=
 =?utf-8?B?alNCOFNxV205ZCtjM3NxWUVpalA3d0hSaXlQZ3hMRlgyWGIzeTV1ZUhsQjUw?=
 =?utf-8?B?djZIMFZJWFV1d1VJYVI1MVI4RFhFengrYWE0c2pKb1RBdHZnekx5Qno5U052?=
 =?utf-8?B?dk5LWXVnSEV1aW5HRDdDVmVuVTN0M083MXJuTFFhOXRMNmdXUDAzc1lsck5j?=
 =?utf-8?B?S1hmOTdEaEhmWnFNblBDL1hCMDk3SXg2UU9FOFcyeFFsa2NoOFhINWJLTGVG?=
 =?utf-8?B?MW1WRlNXQWRzS05QUVZyb0I1TWovZEpFQjNEZUxDbGVmRm84S2FrK05iaHBI?=
 =?utf-8?B?NmwzUmxNOSs0NXVDUlZhRHVQQUpMa3hNRThKWllubU1CUmQ3Qkg1bFFtaDZm?=
 =?utf-8?B?M0JGSjdiNldsZlRSWlRqaXBMMGNkUmpsYTEzbW85V1RIQVN3YWMwZjVFVlFU?=
 =?utf-8?B?alR6dThSMUUwTXVJQzB5VW5oUFcrbWRGaWtiQlVQS0o1eWVKYXRMc1BXZTFI?=
 =?utf-8?B?RkhyaWprNnM5QzBUOC95Y3A1ZnRsbUh2QnEwT1VCeVJnQUptMHV1eERvTmRD?=
 =?utf-8?B?MzAreGtrU3lueG92U0s1eCtMTEwwZTdQaEIyVTlXdWNaZXBHc1U3Y3E0cU5q?=
 =?utf-8?B?ZXlhbmZ6ZllEem5Lc0pTRFhaQUZmYmlucER5V0xZemhtN1hsK2UyWmFNU3Js?=
 =?utf-8?B?bkw4M3g4QTNjeGF2ZjlacW96YVhnUDkydUQ5dmFhSmpPZXNFUFFDSzhJMG96?=
 =?utf-8?B?QWp4a2ZIRXFiSnZUM1UrUFVBOXd6b1VNQVVWOHRXTE96MHpEK1lIYS91Skhn?=
 =?utf-8?B?R05ickRPa3o2dzlYa3prUXgrc0NzSWd2RmMxVTBqakVYYUJjSDJJR0Rudkxl?=
 =?utf-8?B?UWRsck03cDJuRzZsam9WMk9QLy9sdWpuRTlvcm1WOGVmaGFkOE5ITXgzRnpt?=
 =?utf-8?B?ME5yL3V6WFJINlhRZ056MFhxRWlqME9NeWdCR1VpdkdlU3c0UHUrdm5ja2Fj?=
 =?utf-8?B?RThFbFJodXk3WFBMZktrSGdvcVVUTStCRE5mYVhmQmtvb0IvZ0s2TVV0c3Yr?=
 =?utf-8?B?cmlydmlvcSswaFVQMnpRUWp0V0lFMStXTm1FUXNSdHBHM24za2FqSjVaOGUx?=
 =?utf-8?B?VElOVHJzVGYxSWE5N0NNeDRjell1Y3M1Z1piTi80eERXRG9CVmRLWGR5a0li?=
 =?utf-8?B?cFJQcno2bDRyNGx2R0JRSlN1WjU2MDVLR2dXZXdMZUt0Mno3c1Z3ck5JQkli?=
 =?utf-8?B?YTd0bE02RnhaWFdvYXdCbFZha3krSUVLT1pGZ0Y1OVpwZy9wOE1iNzBVZGlG?=
 =?utf-8?B?NktvMEVESElrMDFCR24rZ1dwYWhuU2g5NUFpb3V0cWhjRUtOeExRR0gvQ0VU?=
 =?utf-8?B?MkRUQ3gweE1hNGZmOTI5bG5FS1hMQVp0SlRWczJDb2dMNnZBdEhBZVQ0UzB6?=
 =?utf-8?B?NjNLck5Yd0JuVHFBS0syNTdDVmdhQ0J6K3dxSGpDMVM1QlhWck5YUFAvdzFX?=
 =?utf-8?B?RkJ0Q0Y1REZmMmtnT0VUWDJ6cGhGUkhBSlNPTThrNG5mcUtwYmlXYUxyd1c3?=
 =?utf-8?B?aUJ2dUcvT2wrQzhEYlJnOFFGRnBwSWdOREpXd2NpdVV4UTl1REdueXBNY01t?=
 =?utf-8?B?VlNYS09KTUFKeUlGRklNK1NYQ0tJVHJNeTFJa2F6RDc2aC9GMDJIRDM0d1Iz?=
 =?utf-8?B?dW9BYWtLRHhDZUVDMTlQeHNLdjJHckVWNVJ6NnYrVkVvWDZxK1VIejIzM2Zx?=
 =?utf-8?B?cXhkZWhqbHZPeEdBY3psNVhVZCtFcnIzbm1OcXRGRmxqZ001NmVocGdzSUhw?=
 =?utf-8?B?V3FianBMUE5lbmpuYmlQenEram5PeXF1RDdzYUl6SWtnTUl1djNLMk5RNGs0?=
 =?utf-8?B?QitXeUhySjZSTUZNTnU3eWhBdWFXdUVkWTVWODU0TlhYaFR5Y2dqajBFcC9o?=
 =?utf-8?B?ZXI1TTBlTmdVRk5JOWRZTXZRU0Y5aHlTd2s0dHhEaHZEZFhTdHRaVzd5d2sr?=
 =?utf-8?Q?yZIs9eSoemNuP8I0i8T9LHVyo?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: 
	PvLpFS9djU3mH44+9KQCKdCv1A8c3HqtRy4VG/D2e6TIGEqPchjE0WlfQ35rfffqXEnTIR2Mxe/zp+Wgn4LBz+bu2JbcJjUomHEc2GzcHkkgSn3KJvVKmUjEQtw7ffsInmXrb2FewOm6Ugejg5np4Oai+mAguHW2uFkCuEsz9SS33n83om6I1VHWqbcT9yNQH6uFsonRYdU9KhBJ4Q8ASbCVzcEoQoKDu1rNstcoC8N/8vbkUy9Q/F0K382nn52ZJyjFyWTmoZ1V6VSdOd6pKptjGVxAMleRQSD7/U4n6MpxVN8akTxhcdQSQ0SqHKeoYbv5yHtd2lY5nO1aRVTFmOdt8YyFMCRul8M7PoETzWVWazVaNxHdrrzQ8ezsZJRJTp7qRbluzzEcLa4DNfxpTM71aV4HrZSsks+Nd3JD35fovF7dPFF4ZJTFy+4EQr4bucfT6VJoQ5K4jplRSBYcuQLbuoXAWdbVhkd8fpvaBn+aNnhoKOprf9MYz2wt/2iL5zv7aUyfT1bKVppW6LNq+WzXIvzbgpwe8f0llZ60vQIaK332Fj0MCVmMmSRRPToQJrNFfj9ScEe2x4E9TKC6zK9aYwfjEwQWHNPt1T9E2+Y=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 1e10e8cd-2830-4c90-b8ee-08dc1c3965fb
X-MS-Exchange-CrossTenant-AuthSource: DS0PR10MB7933.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Jan 2024 17:33:23.0870
 (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: 0xY7Ybwxl43AA23TI6raMLzaOtzRfJsA8A80iYUDOly7aXCHBIItmVXze9xhpxefSsoS2Mz98fFAR+aDJmYj7w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR10MB6693
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.272,Aquarius:18.0.1011,Hydra:6.0.619,FMLib:17.11.176.26
 definitions=2024-01-23_10,2024-01-23_02,2023-05-22_02
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 mlxlogscore=893 malwarescore=0
 phishscore=0 bulkscore=0 spamscore=0 suspectscore=0 adultscore=0
 mlxscore=0 classifier=spam adjust=0 reason=mlx scancount=1
 engine=8.12.0-2311290000 definitions=main-2401230129
X-Proofpoint-GUID: 1VUUgDAysAFPhMuVfJdGtsz7VeWmdvW1
X-Proofpoint-ORIG-GUID: 1VUUgDAysAFPhMuVfJdGtsz7VeWmdvW1

* Theo de Raadt <deraadt@openbsd.org> [240122 17:35]:
> Jeff Xu <jeffxu@chromium.org> wrote:
>=20
> > On Mon, Jan 22, 2024 at 7:49=E2=80=AFAM Theo de Raadt <deraadt@openbsd.=
org> wrote:
> > >
> > > Regarding these pieces
> > >
> > > > The PROT_SEAL bit in prot field of mmap(). When present, it marks
> > > > the map sealed since creation.
> > >
> > > OpenBSD won't be doing this.  I had PROT_IMMUTABLE as a draft.  In my
> > > research I found basically zero circumstances when you userland does
> > > that.  The most common circumstance is you create a RW mapping, fill =
it,
> > > and then change to a more restrictve mapping, and lock it.
> > >
> > > There are a few regions in the addressspace that can be locked while =
RW.
> > > For instance, the stack.  But the kernel does that, not userland.  I
> > > found regions where the kernel wants to do this to the address space,
> > > but there is no need to export useless functionality to userland.
> > >
> > I have a feeling that most apps that need to use mmap() in their code
> > are likely using RW mappings. Adding sealing to mmap() could stop
> > those mappings from being executable. Of course, those apps would
> > need to change their code. We can't do it for them.
>=20
> I don't have a feeling about it.
>=20
> I spent a year engineering a complete system which exercises the maximum
> amount of memory you can lock.
>=20
> I saw nothing like what you are describing.  I had PROT_IMMUTABLE in my
> drafts, and saw it turning into a dangerous anti-pattern.
>=20
> > Also, I believe adding this to mmap() has no downsides, only
> > performance gain, as Pedro Falcato pointed out in [1].
> >=20
> > [1] https://lore.kernel.org/lkml/CAKbZUD2A+=3Dbp_sd+Q0Yif7NJqMu8p__eb4y=
guq0agEcmLH8SDQ@mail.gmail.com/
>=20
> Are you joking?  You don't have any code doing that today.  More feelings=
?

The 'no downside" is to combining two calls together; mmap() & mseal(),
at least that is how I read the linked discussion.

The common case (since there are no users today) of just calling
mmap()/munmap() will have the downside.

There will be a performance impact once you have can_modify_mm() doing
more than just returning true.  Certainly, the impact will be larger
in munmap where multiple VMAs may need to be checked (assuming that's
the plan?).

This will require a new and earlier walk of the vma tree while holding
the mmap_lock.  Since you are checking (potentially multiple) VMAs for
something, I don't think there is a way around holding the lock.

I'm not saying the cost will be large, but it will be a positive
non-zero number.

Thanks,
Liam