Re: [PATCH 3/6] xen/arm: ffa: Tighten notification parameter validation

[Jens Wiklander <jens.wiklander@xxxxxxxxxx>]

Hi Bertand,

On Fri, Apr 17, 2026 at 3:41 PM Bertrand Marquis
<bertrand.marquis@xxxxxxx> wrote:
>
> The notification handlers still validate overlapping subsets of their
> inputs. BIND, UNBIND, and SET each decode caller and destination IDs
> locally, GET still accepts a non-zero receiver vCPU ID and reserved flag
> bits, and SET still accepts non-zero NS-virtual flags. BIND also treats
> unsupported non-zero flag encodings as a supported-feature failure
> instead of as malformed input.
>
> Add ffa_notif_parse_params() and use it to centralize the common
> caller/destination and non-zero bitmap checks for BIND, UNBIND, and SET.
> Also reject malformed GET and SET requests locally before touching
> cached state or forwarding anything to the SPMC. Keep BIND limited to
> global notifications and reject unsupported non-zero flag encodings with
> INVALID_PARAMETERS.
>
> - add a shared parameter parser for notification caller/destination
>   validation
> - wire BIND and UNBIND through the shared parser and reject unsupported
>   bind flag encodings with INVALID_PARAMETERS
> - reject non-zero receiver vCPU and reserved flag bits in
>   FFA_NOTIFICATION_GET
> - reject non-zero flags in the NS-virtual FFA_NOTIFICATION_SET path
>
> Functional impact: malformed notification requests are rejected
> consistently earlier in the mediator.
>
> Signed-off-by: Bertrand Marquis <bertrand.marquis@xxxxxxx>
> ---
>  xen/arch/arm/tee/ffa_notif.c | 61 +++++++++++++++++++++++++++++-------
>  1 file changed, 50 insertions(+), 11 deletions(-)
>
> diff --git a/xen/arch/arm/tee/ffa_notif.c b/xen/arch/arm/tee/ffa_notif.c
> index d15119409a25..491db3b04df5 100644
> --- a/xen/arch/arm/tee/ffa_notif.c
> +++ b/xen/arch/arm/tee/ffa_notif.c
> @@ -42,21 +42,40 @@ static void inject_notif_pending(struct domain *d)
>                 d);
>  }
>
> +static int32_t ffa_notif_parse_params(uint16_t dom_id, uint16_t caller_id,
> +                                      uint16_t dest_id, uint32_t bitmap_lo,
> +                                      uint32_t bitmap_hi)

Nit: I would have picked ffa_notif_validate_params() or
ffa_notif_check_params(), but that might be more a matter of taste.
Anyway, looks good:
Reviewed-by: Jens Wiklander <jens.wiklander@xxxxxxxxxx>

Cheers,
Jens

> +{
> +    if ( caller_id != dom_id || dest_id == dom_id || !dest_id )
> +        return FFA_RET_INVALID_PARAMETERS;
> +
> +    if ( !bitmap_lo && !bitmap_hi )
> +        return FFA_RET_INVALID_PARAMETERS;
> +
> +    return FFA_RET_OK;
> +}
> +
>  int32_t ffa_handle_notification_bind(struct cpu_user_regs *regs)
>  {
>      struct domain *d = current->domain;
> +    struct ffa_ctx *ctx = d->arch.tee;
> +    int32_t ret;
>      uint32_t src_dst = get_user_reg(regs, 1);
>      uint32_t flags = get_user_reg(regs, 2);
>      uint32_t bitmap_lo = get_user_reg(regs, 3);
>      uint32_t bitmap_hi = get_user_reg(regs, 4);
> +    uint16_t caller_id = src_dst & GENMASK(15, 0);
> +    uint16_t dest_id = src_dst >> 16;
>
> -    if ( (src_dst & GENMASK(15, 0)) != ffa_get_vm_id(d) )
> +    if ( flags )    /* Only global notifications are supported */
>          return FFA_RET_INVALID_PARAMETERS;
>
> -    if ( flags )    /* Only global notifications are supported */
> -        return FFA_RET_DENIED;
> +    ret = ffa_notif_parse_params(ctx->ffa_id, caller_id, dest_id, bitmap_lo,
> +                                 bitmap_hi);
> +    if ( ret )
> +        return ret;
>
> -    if ( FFA_ID_IS_SECURE(src_dst >> 16) && fw_notif_enabled )
> +    if ( FFA_ID_IS_SECURE(dest_id) && fw_notif_enabled )
>          return ffa_simple_call(FFA_NOTIFICATION_BIND, src_dst, flags,
>                                 bitmap_lo, bitmap_hi);
>
> @@ -66,16 +85,22 @@ int32_t ffa_handle_notification_bind(struct cpu_user_regs 
> *regs)
>  int32_t ffa_handle_notification_unbind(struct cpu_user_regs *regs)
>  {
>      struct domain *d = current->domain;
> +    struct ffa_ctx *ctx = d->arch.tee;
> +    int32_t ret;
>      uint32_t src_dst = get_user_reg(regs, 1);
>      uint32_t bitmap_lo = get_user_reg(regs, 3);
>      uint32_t bitmap_hi = get_user_reg(regs, 4);
> +    uint16_t caller_id = src_dst & GENMASK(15, 0);
> +    uint16_t dest_id = src_dst >> 16;
>
> -    if ( (src_dst & GENMASK(15, 0)) != ffa_get_vm_id(d) )
> -        return FFA_RET_INVALID_PARAMETERS;
> +    ret = ffa_notif_parse_params(ctx->ffa_id, caller_id, dest_id, bitmap_lo,
> +                                 bitmap_hi);
> +    if ( ret )
> +        return ret;
>
> -    if ( FFA_ID_IS_SECURE(src_dst >> 16) && fw_notif_enabled )
> -        return  ffa_simple_call(FFA_NOTIFICATION_UNBIND, src_dst, 0, 
> bitmap_lo,
> -                                bitmap_hi);
> +    if ( FFA_ID_IS_SECURE(dest_id) && fw_notif_enabled )
> +        return ffa_simple_call(FFA_NOTIFICATION_UNBIND, src_dst, 0, 
> bitmap_lo,
> +                               bitmap_hi);
>
>      return FFA_RET_NOT_SUPPORTED;
>  }
> @@ -142,6 +167,12 @@ void ffa_handle_notification_get(struct cpu_user_regs 
> *regs)
>          return;
>      }
>
> +    if ( recv >> 16 || (flags & GENMASK(31, 4)) )
> +    {
> +        ffa_set_regs_error(regs, FFA_RET_INVALID_PARAMETERS);
> +        return;
> +    }
> +
>      if ( fw_notif_enabled && (flags & ( FFA_NOTIF_FLAG_BITMAP_SP |
>                                          FFA_NOTIF_FLAG_BITMAP_SPM )) )
>      {
> @@ -204,11 +235,19 @@ int32_t ffa_handle_notification_set(struct 
> cpu_user_regs *regs)
>      uint32_t flags = get_user_reg(regs, 2);
>      uint32_t bitmap_lo = get_user_reg(regs, 3);
>      uint32_t bitmap_hi = get_user_reg(regs, 4);
> +    uint16_t caller_id = src_dst >> 16;
> +    uint16_t dest_id = src_dst & GENMASK(15, 0);
> +    int32_t ret;
> +
> +    ret = ffa_notif_parse_params(ffa_get_vm_id(d), caller_id, dest_id,
> +                                 bitmap_lo, bitmap_hi);
> +    if ( ret )
> +        return ret;
>
> -    if ( (src_dst >> 16) != ffa_get_vm_id(d) )
> +    if ( flags )
>          return FFA_RET_INVALID_PARAMETERS;
>
> -    if ( FFA_ID_IS_SECURE(src_dst & GENMASK(15, 0)) && fw_notif_enabled )
> +    if ( FFA_ID_IS_SECURE(dest_id) && fw_notif_enabled )
>          return ffa_simple_call(FFA_NOTIFICATION_SET, src_dst, flags, 
> bitmap_lo,
>                                 bitmap_hi);
>
> --
> 2.53.0
>