Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <6dbbc3da-78c9-8101-d52a-0be47da9d67e@intel.com>
Date:   Thu, 18 May 2023 14:43:25 -0700
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101
 Thunderbird/102.10.0
Subject: Re: [PATCH 2/6] PKEY: Add arch_check_pkey_enforce_api()
Content-Language: en-US
To:     jeffxu@chromium.org, luto@kernel.org, jorgelo@chromium.org,
        keescook@chromium.org, groeck@chromium.org, jannh@google.com,
        sroettger@google.com
Cc:     akpm@linux-foundation.org, jeffxu@google.com,
        linux-kernel@vger.kernel.org, linux-kselftest@vger.kernel.org,
        linux-mm@kvack.org, linux-hardening@vger.kernel.org
References: <20230515130553.2311248-1-jeffxu@chromium.org>
 <20230515130553.2311248-3-jeffxu@chromium.org>
From:   Dave Hansen <dave.hansen@intel.com>
In-Reply-To: <20230515130553.2311248-3-jeffxu@chromium.org>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 5/15/23 06:05, jeffxu@chromium.org wrote:
> +static inline int __arch_check_vma_pkey_for_write(struct vm_area_struct *vma)
> +{
> +	int pkey = vma_pkey(vma);
> +
> +	if (mm_pkey_enforce_api(vma->vm_mm, pkey)) {
> +		if (!__pkru_allows_write(read_pkru(), pkey))
> +			return -EACCES;
> +	}
> +
> +	return 0;
> +}

Please think very carefully about what I'm about to say:

What connects vma->vm_mm to read_pkru() here?

Now think about what happens when we have kthread_use_mm() or a ptrace()
doing get_task_mm() and working on another process's mm or VMA.

Look at arch_vma_access_permitted() and notice how it avoids read_pkru()
for 'foreign' aka. 'remote' accesses:

> static inline bool arch_vma_access_permitted(struct vm_area_struct *vma,
>                 bool write, bool execute, bool foreign)
> {
...
>         if (foreign || vma_is_foreign(vma))
>                 return true;
>         return // check read_pkru()
> }

In other words, it lets all remote accesses right through.  That's
because there is *NOTHING* that fundamentally and tightly connects the
PKRU value in this context to the VMA or the context that initiated this
operation.

If your security model depends on PKRU protection, this 'remote'
disconnection is problematic.  The PKRU enforcement inside the kernel is
best-effort.  That usually doesn't map into the security space very well.

Do you have a solid handle on all call paths that will reach
__arch_check_vma_pkey_for_write() and can you ensure they are all
non-remote?