Re: [PATCH for-next 3/7] IB/hfi1: Validate PKEY for incoming GSI MAD packets

[Leon Romanovsky <leon-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>]

[-- Attachment #1: Type: text/plain, Size: 2447 bytes --]

On Mon, Oct 23, 2017 at 06:06:00AM -0700, Dennis Dalessandro wrote:
> From: Sebastian Sanchez <sebastian.sanchez-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>
> These are the use-cases where the pkey needs to be tested to see
> if a packet needs to be dropped.
>
> a) Check if pkey is not FULL_MGMT_P_KEY or LIM_MGMT_P_KEY,
>    drop the packet as it's not part of the management partition.
>    Self-originated packets are an exception.
>
> b) If pkey index points to FULL_MGMT_P_KEY and LIM_MGMT_P_KEY is
>    in the table, the packet is coming from a management node,
>    and the receiving node is also a management node, so it is safe
>    for the packet to go through.
>
> c) If pkey index points to FULL_MGMT_P_KEY and LIM_MGMT_P_KEY is
>    NOT in the table, drop the packet as LIM_MGMT_P_KEY should
>    always be in the pkey table. It could be a misconfiguration.
>
> d) If pkey index points to LIM_MGMT_P_KEY and FULL_MGMT_P_KEY is
>    NOT in the table, it is safe for the packet to go through
>    since a non-management node is talking to another non-managment
>    node.
>
> e) If pkey index points to LIM_MGMT_P_KEY and FULL_MGMT_P_KEY is in
>    the table, drop the packet because a non-management node is
>    talking to a management node, and it could be an attack.
>
> For the implementation, these rules can be simplied to only checking
> for (a) and (e). There's no need to check for rule (b) as
> the packet doesn't need to be dropped. Rule (c) is not possible in
> the driver as LIM_MGMT_P_KEY is always in the pkey table.
>
> Reviewed-by: Michael J. Ruhl <michael.j.ruhl-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Sebastian Sanchez <sebastian.sanchez-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> Signed-off-by: Dennis Dalessandro <dennis.dalessandro-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
> ---
>  drivers/infiniband/hw/hfi1/mad.c |   86 +++++++++++++++++++++++++++++++++++++-
>  1 files changed, 84 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/hw/hfi1/mad.c b/drivers/infiniband/hw/hfi1/mad.c
> index 07b80fa..dfe6224 100644
> --- a/drivers/infiniband/hw/hfi1/mad.c
> +++ b/drivers/infiniband/hw/hfi1/mad.c
> @@ -98,6 +98,16 @@ static inline void clear_opa_smp_data(struct opa_smp *smp)
>  	memset(data, 0, size);
>  }
>
> +static inline u16 hfi1_lookup_pkey_value(struct hfi1_ibport *ibp, int pkey_idx)

Please, no "inline-function" in *.c files as it is written in CodingStyle.

Thanks

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]