Received-SPF: pass (google.com: domain of linux-kernel+bounces-179997-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:40f1:3f00::1 as permitted sender) client-ip=2604:1380:40f1:3f00::1;
Precedence: bulk
Mime-Version: 1.0
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset=UTF-8
Date: Wed, 15 May 2024 17:28:43 +0300
Message-Id: <D1AAFDCSFYH1.11RF8JUS2NEZS@kernel.org>
Cc: <mona.vij@intel.com>, <kailun.qin@intel.com>, <stable@vger.kernel.org>,
 =?utf-8?q?Marcelina_Ko=C5=9Bcielnicka?= <mwk@invisiblethingslab.com>
Subject: Re: [PATCH v2 1/2] x86/sgx: Resolve EAUG race where losing thread
 returns SIGBUS
From: "Jarkko Sakkinen" <jarkko@kernel.org>
To: "Dave Hansen" <dave.hansen@intel.com>, "Dmitrii Kuvaiskii"
 <dmitrii.kuvaiskii@intel.com>, <dave.hansen@linux.intel.com>,
 <kai.huang@intel.com>, <haitao.huang@linux.intel.com>,
 <reinette.chatre@intel.com>, <linux-sgx@vger.kernel.org>,
 <linux-kernel@vger.kernel.org>
References: <20240515131240.1304824-1-dmitrii.kuvaiskii@intel.com>
 <20240515131240.1304824-2-dmitrii.kuvaiskii@intel.com>
 <D1A9PC6LWL2S.38KB2X3EL9X79@kernel.org>
 <58e9d453-7909-4157-86fd-a2d5561e728e@intel.com>
In-Reply-To: <58e9d453-7909-4157-86fd-a2d5561e728e@intel.com>

On Wed May 15, 2024 at 5:15 PM EEST, Dave Hansen wrote:
> On 5/15/24 06:54, Jarkko Sakkinen wrote:
> > I'd cut out 90% of the description out and just make the argument of
> > the wrong error code, and done. The sequence is great for showing
> > how this could happen. The prose makes my head hurt tbh.
>
> The changelog is too long, but not fatally so.  I'd much rather have a
> super verbose description than something super sparse.
>
> Would something like this make more sense to folks?
>
> 	Imagine an mmap()'d file. Two threads touch the same address at
> 	the same time and fault. Both allocate a physical page and race
> 	to install a PTE for that page. Only one will win the race. The
> 	loser frees its page, but still continues handling the fault as
> 	a success and returns VM_FAULT_NOPAGE from the fault handler.
>
> 	The same race can happen with SGX. But there's a bug: the loser
> 	in the SGX steers into a failure path. The loser EREMOVE's the
> 	winner's EPC page, then returns SIGBUS, likely killing the app.
>
> 	Fix the SGX loser's behavior. Change the return code to
> 	VM_FAULT_NOPAGE to avoid SIGBUS and call sgx_free_epc_page()
> 	which avoids EREMOVE'ing the winner's page and only frees the
> 	page that the loser allocated.

Yes!

I did read the whole thing. My comment was only related to the
chain of maintainers who also have to deal with this patch
eventually.

BR, Jarkko