[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH] x86/boot: Restrict directmap permissions for .text/.rodata
- To: Jan Beulich <jbeulich@xxxxxxxx>
- From: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
- Date: Tue, 28 Mar 2023 11:27:55 +0100
- Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=citrix.com; dmarc=pass action=none header.from=citrix.com; dkim=pass header.d=citrix.com; arc=none
- Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=R+1/s8iZ851MSYZGrNlrsauuVrtCXjUKHj5dmy4jKTQ=; b=cZ1MdMpUNPF17twHU5pNr7yrQlaPlM04dzdJ/F3CZw/e0LPiHAKPPf1iBfxECJUuzuiVPmoE079lOU+nBVCN2KM5SaW63qEYdExKTAQgv8ffSck+vjkIwbpAqhT0ifpd8rblT0luK799DJlXv0Lu4Ozg1jWIG/aJ5fANeRolHq2lcnSaKuEgQCd5wJZroqBc/LNaqwAzsfeDTjMrDJ/gX6K9BHW5qsWjgyhlEP06Epfv5QbMSsO9+HeLWkvC2/eE65wdSFSCjeNlTLGg3vY1geE/jnKo80wUC/CMApw3+SDXmLr4L3YiSXnR65cKvLunbMKOimDwC1FZ7vY03L/16g==
- Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=aiqQ1hZFB+vj5QUWVhq8OpNyBrJkoFjUpIXlSCZV3rbMAPi8LFh3fxOTQrJ7gU3EXKhNapAegDnwAsaYQZWuXV9/cv7V6hMreLTsF3BI7aZW13vDQKNagBluuwmddAVLQoJjuIpiX9JwQ4sh86NFCUwqFL+m6pcsADVSp59gfZfuem4pF4mi/3ugbdf9py8IwL6zwl44ocpHMxfDzx44BzeA3XlhEe3n0fYhjo2P6hSNcU1be87ax1StL28By4a7R1ZLSPamzIa2CBPvUHH0qNobAEhd2PvCR0gHRzQyiUARTl3yMPwjCH5pkMOsNYqdclX+gPeG1D/KyOyO5onqrw==
- Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=citrix.com;
- Cc: Roger Pau Monné <roger.pau@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
- Delivery-date: Tue, 28 Mar 2023 10:28:28 +0000
- Ironport-data: A9a23:3cdW9qltMKKo7oivZ30JAnLo5gysJ0RdPkR7XQ2eYbSJt1+Wr1Gzt xIbCj2BOqveZWX0Ktxzatyy8k9X78WGy9QwSQtuq38yHiMWpZLJC+rCIxarNUt+DCFhoGFPt JxCN4aafKjYaleG+39B55C49SEUOZmgH+a6U6icfHgqH2eIcQ954Tp7gek1n4V0ttawBgKJq LvartbWfVSowFaYCEpNg064gE4p7aSaVA8w5ARkPqgQ5g+GzRH5MbpETU2PByqgKmVrNrbSq 9brlNmR4m7f9hExPdKp+p6TnpoiG+O60aCm0xK6aoD66vRwjnVaPpUTbZLwXXx/mTSR9+2d/ f0W3XCGpaXFCYWX8AgVe0Ew/yiTpsSq8pefSZS0mZT7I0Er7xIAahihZa07FdRwxwp5PY1B3 dYBCAAvYFO4vrro75CpdrYrq+MZKeC+aevzulk4pd3YJdAPZMmbBoD1v5pf1jp2gd1SF/HDY cZfcSBocBnLfxxIPBEQFY46m+CrwHL4dlW0qnrM/fZxvzeVkVE3ieC2WDbWUoXiqcF9t0CUv G/ZuU/+BQkXLoe3wjuZ6HO8wOTImEsXXapLTODnr6Ay3gf7Kmo7EDkfEnrhhuGAu0+kUPgPc 2Ys1HsulP1nnKCsZpynN/Gim1aGtBMBX9tbE8Uh9RqAjKHT5m6xGWwsXjNHLts8u6ceVTEsk 1OEgd7tLThuq6GOD2KQ8K+OqjG/MjRTKnUNDRLoViMA6tjn5YQs1BTGS487FLbv14WuXzbt3 zqNsS4ywa0JitIG3Lm6+laBhC+wop/OTUg+4QC/sn+Z0z6VrbWNP+SAgWU3J94ZRGpFZjFtZ EQ5pvU=
- Ironport-hdrordr: A9a23:ZMvwQ6xeXqTteJkzlVrJKrPw671zdoMgy1knxilNoRw8SL3/qy nOppQmPHrP4wr5N0tApTntAtjkfZq+z+8N3WByB8bbYOCOggLBQ+9fBOPZskbd8kbFh4pgPM lbAs9DIey1IGJWyeDdy2CDf+rIxuPszImYwd3z9TNGayZES49d1C9FKiC9VndbeWB9dPkEPa vZ6cpDqyChangMB/7XOlAOQ/LfodnGj7LKCCR2ZSIa1A==
- List-id: Xen developer discussion <xen-devel.lists.xenproject.org>
On 27/03/2023 4:43 pm, Jan Beulich wrote:
> On 24.03.2023 23:08, Andrew Cooper wrote:
>> While we've been diligent to ensure that the main text/data/rodata mappings
>> have suitable restrictions, their aliases via the directmap were left fully
>> read/write. Worse, we even had pieces of code making use of this as a
>> feature.
>>
>> Restrict the permissions for .text/rodata, as we have no legitimate need for
>> writeability of these areas via the directmap alias. Note that the
>> compile-time allocated pagetables do get written through their directmap
>> alias, so need to remain writeable.
>>
>> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx>
Thanks.
>
>> Notes:
>> * The stubs are still have RX via one alias, RW via another, and these need
>> to stay. We should harden this using PKS (available on SPR and later) to
>> block incidental writes.
>> * Backing memory for livepatch text/rodata needs similar treatment.
> Right, but there it's somewhat more involved because upon removal the
> attributes also need restoring.
Yeah, I'm going to defer it to a gitlab ticket for now. And the PKS
addition too.
>> * For backporting, this patch depends on c/s e7f147bf4ac7 ("x86/crash: Drop
>> manual hooking of exception_table[]") and c/s e7db635f4428 ("x86/pv-shim:
>> Don't modify the hypercall table"). No compile error will occur from
>> getting these dependencies wrong.
> I suppose the latter isn't strictly a prereq, as the modification was done
> from an __init function (i.e. before this new code runs).
This code here runs long before pv_shim_setup_dom(). This dependency
was found experimentally via testing IIRC.
> Iirc we didn't backport prior similar hardening work? So I'm not sure we'd
> want/need to do so in this case.
That patch wasn't backported. I was originally planning to, as part of
the CET-IBT work for Intel-retbleed, but in the end didn't.
We went for the "ENDBR on every function" approach on older trees
because it was better than nothing, and far less invasive than
backporting cf_check everywhere.
~Andrew
|