| Message ID | 20240322124303.309423-2-sebastianene@google.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [1/2] KVM: arm64: Fix the identification range for the FF-A smcs | expand |
On Fri, Mar 22, 2024 at 12:43:03PM +0000, Sebastian Ene wrote: > The previous logic used a deny list to filter the FF-A calls. Because of > this, some of the calls escaped the check and they were forwarded by > default to Trustzone. (eg. FFA_MSG_SEND_DIRECT_REQ was denied but the 64 > bit version of the call was not). > Modify the logic to use an allowlist and allow only the calls specified in > the filter function to be proxied to TZ from the hypervisor. I had discussed this with Will back when the feature was upstreamed and he said there's a lot of off-label calls that necessitate a denylist implementation. Has anything changed to give us confidence that we can be restrictive, at least on the FF-A range?
On Fri, Mar 22, 2024 at 07:07:52PM -0700, Oliver Upton wrote: > On Fri, Mar 22, 2024 at 12:43:03PM +0000, Sebastian Ene wrote: > > The previous logic used a deny list to filter the FF-A calls. Because of > > this, some of the calls escaped the check and they were forwarded by > > default to Trustzone. (eg. FFA_MSG_SEND_DIRECT_REQ was denied but the 64 > > bit version of the call was not). > > Modify the logic to use an allowlist and allow only the calls specified in > > the filter function to be proxied to TZ from the hypervisor. Hi Oliver, > > I had discussed this with Will back when the feature was upstreamed and > he said there's a lot of off-label calls that necessitate a denylist > implementation. Has anything changed to give us confidence that we can > be restrictive, at least on the FF-A range? > I remember your proposal for having an allowlist instead. The current change makes sense if we have https://lore.kernel.org/kvmarm/20240322124303.309423-1-sebastianene@google.com/ which opens the window for more FF-A calls to be forwarded to TZ. Let me know if this clarifies, thanks Seb > -- > Thanks, > Oliver
On Mon, Mar 25, 2024 at 11:29:39AM +0000, Sebastian Ene wrote: > On Fri, Mar 22, 2024 at 07:07:52PM -0700, Oliver Upton wrote: > > On Fri, Mar 22, 2024 at 12:43:03PM +0000, Sebastian Ene wrote: > > > The previous logic used a deny list to filter the FF-A calls. Because of > > > this, some of the calls escaped the check and they were forwarded by > > > default to Trustzone. (eg. FFA_MSG_SEND_DIRECT_REQ was denied but the 64 > > > bit version of the call was not). > > > Modify the logic to use an allowlist and allow only the calls specified in > > > the filter function to be proxied to TZ from the hypervisor. > > Hi Oliver, > > > > > I had discussed this with Will back when the feature was upstreamed and > > he said there's a lot of off-label calls that necessitate a denylist > > implementation. Has anything changed to give us confidence that we can > > be restrictive, at least on the FF-A range? > > > > I remember your proposal for having an allowlist instead. The current change makes > sense if we have https://lore.kernel.org/kvmarm/20240322124303.309423-1-sebastianene@google.com/ > which opens the window for more FF-A calls to be forwarded to TZ. Got it. Last time I didn't catch the level of abuse we expect to endure from vendors, but now it seems we will not support non-conforming calls that appear in standardized SMC ranges. Adding mention of this to the changelog might be a good idea then.
On Tue, Mar 26, 2024 at 01:42:26AM -0700, Oliver Upton wrote: > On Mon, Mar 25, 2024 at 11:29:39AM +0000, Sebastian Ene wrote: > > On Fri, Mar 22, 2024 at 07:07:52PM -0700, Oliver Upton wrote: > > > On Fri, Mar 22, 2024 at 12:43:03PM +0000, Sebastian Ene wrote: > > > > The previous logic used a deny list to filter the FF-A calls. Because of > > > > this, some of the calls escaped the check and they were forwarded by > > > > default to Trustzone. (eg. FFA_MSG_SEND_DIRECT_REQ was denied but the 64 > > > > bit version of the call was not). > > > > Modify the logic to use an allowlist and allow only the calls specified in > > > > the filter function to be proxied to TZ from the hypervisor. > > > > Hi Oliver, > > > > > > > > I had discussed this with Will back when the feature was upstreamed and > > > he said there's a lot of off-label calls that necessitate a denylist > > > implementation. Has anything changed to give us confidence that we can > > > be restrictive, at least on the FF-A range? > > > > > > > I remember your proposal for having an allowlist instead. The current change makes > > sense if we have https://lore.kernel.org/kvmarm/20240322124303.309423-1-sebastianene@google.com/ > > which opens the window for more FF-A calls to be forwarded to TZ. > > Got it. Last time I didn't catch the level of abuse we expect to endure > from vendors, but now it seems we will not support non-conforming calls > that appear in standardized SMC ranges. > > Adding mention of this to the changelog might be a good idea then. > That's a good point. I didn't create a changelog for this but I should add one and specify this. > -- > Thanks, > Oliver Thanks, Sebastian
diff --git a/arch/arm64/kvm/hyp/nvhe/ffa.c b/arch/arm64/kvm/hyp/nvhe/ffa.c index 320f2eaa14a9..fc4f04209618 100644 --- a/arch/arm64/kvm/hyp/nvhe/ffa.c +++ b/arch/arm64/kvm/hyp/nvhe/ffa.c @@ -580,35 +580,35 @@ static void do_ffa_mem_reclaim(struct arm_smccc_res *res, ffa_to_smccc_res(res, ret); } -/* - * Is a given FFA function supported, either by forwarding on directly - * or by handling at EL2? - */ static bool ffa_call_supported(u64 func_id) { switch (func_id) { - /* Unsupported memory management calls */ - case FFA_FN64_MEM_RETRIEVE_REQ: - case FFA_MEM_RETRIEVE_RESP: - case FFA_MEM_RELINQUISH: - case FFA_MEM_OP_PAUSE: - case FFA_MEM_OP_RESUME: - case FFA_MEM_FRAG_RX: - case FFA_FN64_MEM_DONATE: - /* Indirect message passing via RX/TX buffers */ - case FFA_MSG_SEND: - case FFA_MSG_POLL: - case FFA_MSG_WAIT: - /* 32-bit variants of 64-bit calls */ + /* Discovery functions */ + case FFA_FEATURES: + case FFA_ID_GET: + case FFA_VERSION: + case FFA_PARTITION_INFO_GET: + + /* Memory management calls */ + case FFA_FN64_RXTX_MAP: + case FFA_RXTX_UNMAP: + case FFA_MEM_SHARE: + case FFA_FN64_MEM_SHARE: + case FFA_MEM_RECLAIM: + case FFA_MEM_LEND: + case FFA_FN64_MEM_LEND: + case FFA_MEM_FRAG_TX: + + /* State management */ + case FFA_RUN: + + /* Message passing */ case FFA_MSG_SEND_DIRECT_REQ: - case FFA_MSG_SEND_DIRECT_RESP: - case FFA_RXTX_MAP: - case FFA_MEM_DONATE: - case FFA_MEM_RETRIEVE_REQ: - return false; + case FFA_FN64_MSG_SEND_DIRECT_REQ: + return true; } - return true; + return false; } static bool do_ffa_features(struct arm_smccc_res *res,
The previous logic used a deny list to filter the FF-A calls. Because of this, some of the calls escaped the check and they were forwarded by default to Trustzone. (eg. FFA_MSG_SEND_DIRECT_REQ was denied but the 64 bit version of the call was not). Modify the logic to use an allowlist and allow only the calls specified in the filter function to be proxied to TZ from the hypervisor. Signed-off-by: Sebastian Ene <sebastianene@google.com> --- arch/arm64/kvm/hyp/nvhe/ffa.c | 46 +++++++++++++++++------------------ 1 file changed, 23 insertions(+), 23 deletions(-)