From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wr1-f66.google.com (mail-wr1-f66.google.com [209.85.221.66]) by mx.groups.io with SMTP id smtpd.web11.34144.1585218937750095520 for ; Thu, 26 Mar 2020 03:35:38 -0700 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=TDbO9nUr; spf=pass (domain: linaro.org, ip: 209.85.221.66, mailfrom: ard.biesheuvel@linaro.org) Received: by mail-wr1-f66.google.com with SMTP id a25so7135376wrd.0 for ; Thu, 26 Mar 2020 03:35:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=8cv4dgM9qGvMXB312H7/eTZoEXt8AQcouopJ9bIWxoc=; b=TDbO9nUrxULgNoceIXaDy3/rxPoqYVtBFtiDtXkd4rcB26xhgMTM2qk/DJDIPYX+RA Pw02IFgFX3rYTfzUEHovXqH1Inztwl0DNT6tfWjT6KCsKHp+8yzVxBG55uBeHartosP4 Ic0QO1in6xiFrCVG1AEAvtlCyZ5ki+PC3eG2K4RtAUh6gHcQeU786s1PF5oh8WaklKxm W/+DV3Lu5aBP/MDniG4EdjyItAMzHWL0HFeAHfa0Csz49LywqVM05BRTOQh6O2K4qfU7 0UXt8IHfsVsLs1eqckx03GlrDV0uUpKe0K0OI7/z4SN9L8fcG+S//lSeOZjab5XUT+X1 /zXw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=8cv4dgM9qGvMXB312H7/eTZoEXt8AQcouopJ9bIWxoc=; b=uOUaLlHj0LCM4SICjnkAV/Qp75xvbwytV4gW+4zBh63KEc+bGphUiqKlalC+Jtf7aA XQTJO9XBe6m6vvkfY16Amg8thPtFilqj1YcCIkzTxclYoSpSBVQXdzq2Ifgy091ethnu 8mOgTGKpY/zhcXui/PfC5cGhPCyic5Jt8H9ANlBtd1aV1waeKC1+d+NgnFfPyhdQ6B34 iPyTwZCMu75sPuUQgSA16b6tRXEE0HO4ras0YBtuDSfLHjhUyIcG4D0+B72S8Odi0Aus YZaxC6VWlC6/NuSC2UKakpeKBCMZGFwQSf1A02M2hUMwa5HuZkjZfGICfH0MVq2bE7HI tmnw== X-Gm-Message-State: ANhLgQ0aPuSfNvrRXMo0L/6hzWYLaEl8ZWygRkA4PeOJW8kgqJduer9w M+bZoo+3auw+4o7jqTGtt1w/sqPK9nMAYVvYOd6ZXg== X-Google-Smtp-Source: ADFU+vuKOmnAiLCimVSK5onSWA88Kj2pVIgLA0jaqvnHe1gKl4ETGhoKGb62psmum2H9w3uEi+3sn1WSfXzck+NDAPA= X-Received: by 2002:a5d:4fcf:: with SMTP id h15mr8515449wrw.262.1585218936205; Thu, 26 Mar 2020 03:35:36 -0700 (PDT) MIME-Version: 1.0 References: <20200325152940.1492-1-ard.biesheuvel@linaro.org> <4e92b528-921a-6159-55b8-bb0d6d7dea7d@redhat.com> In-Reply-To: <4e92b528-921a-6159-55b8-bb0d6d7dea7d@redhat.com> From: "Ard Biesheuvel" Date: Thu, 26 Mar 2020 11:35:25 +0100 Message-ID: Subject: Re: [edk2-devel] [PATCH v3 0/3] ArmPkg/ArmMmuLib AARCH64: correctness fix To: Laszlo Ersek Cc: edk2-devel-groups-io , Leif Lindholm , Ashish Singhal Content-Type: text/plain; charset="UTF-8" On Wed, 25 Mar 2020 at 20:49, Laszlo Ersek wrote: > > On 03/25/20 16:29, Ard Biesheuvel wrote: > > The new ArmMmuLib code is easier to reason about, so that is what I did: > > currently, when we create mappings that cover existing table entries, we > > may end up overwriting those with block entries without taking the mapping > > attributes of the original table entries into account. So let's fix this. > > > > I honestly don't know whether the original code was better at dealing with > > this: I do remember some changes from Heyi that may have been related, but > > the old code is not easy to follow. In any case, I didn't manage to hit this > > case in practice, given that we typically start out with large mappings, and > > break them down later (to set permissions), rather than the other way around. > > > > Patch #1 adds some helpers to hide the insane way the type bits change > > meaning when you change to level 3. > > > > Patch #2 ensures that we only replace (and free) table entries with block > > entries if it is guaranteed that doing so will not lose any attribute > > information. > > > > Changes since v2: > > - add patch to limit recursion to levels < 3 in FreePageTablesRecursive() > > > > Changes since v1: > > - zero newly allocated pages before splitting a block entry into a table > > entry, to avoid garbage in that page being misidentified as entry type > > attributes - this should fix the crash observed by Laszlo > > > > Cc: Laszlo Ersek > > Cc: Leif Lindholm > > Cc: Ashish Singhal > > > > Ard Biesheuvel (3): > > ArmPkg/ArmMmuLib AARCH64: limit recursion when freeing page tables > > ArmPkg/ArmMmuLib AARCH64: use helpers to determine table entry types > > ArmPkg/ArmMmuLib AARCH64: preserve attributes when replacing a table > > entry > > > > .../Library/ArmMmuLib/AArch64/ArmMmuLibCore.c | 83 +++++++++++++++---- > > 1 file changed, 68 insertions(+), 15 deletions(-) > > > > Tested-by: Laszlo Ersek > Pushed to master Thanks all