[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



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.