On Thu, Aug 22, 2024 at 06:19:38PM +0100, Catalin Marinas wrote:
On Thu, Aug 22, 2024 at 05:44:19PM +0100, Mark Brown wrote:
quoted
On Thu, Aug 22, 2024 at 05:12:30PM +0100, Catalin Marinas wrote:
quoted
On Thu, Aug 22, 2024 at 02:15:22AM +0100, Mark Brown wrote:
quoted
quoted
+static bool is_invalid_gcs_access(struct vm_area_struct *vma, u64 esr)
quoted
quoted
+ } else if (unlikely(vma->vm_flags & VM_SHADOW_STACK)) {
+ /* Only GCS operations can write to a GCS page */
+ return is_write_abort(esr);
+ }
quoted
Yes, that should check for a data abort. I think I'd formed the
impression that is_write_abort() included that check somehow. As you
say it's to avoid spinning trying to resolve a permission fault for a
write (non-GCS reads to a GCS page are valid), I do think we need the
is_write_abort() since non-GCS reads are valid so something like:
if (!esr_is_data_abort(esr))
return false;
return is_write_abort(esr);
We do need the write abort check but not unconditionally, only if to a
GCS page (you can have other genuine write aborts).
tThat was to replace the checks in the above case, not the function as a
whole.