linux-kernel - Re: [PATCH rdma-rc] RDMA/cma: Clear all multicast request fields

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20220117161621.GC84788@nvidia.com>
Date:   Mon, 17 Jan 2022 12:16:21 -0400
From:   Jason Gunthorpe <jgg@...dia.com>
To:     Leon Romanovsky <leon@...nel.org>
Cc:     linux-kernel@...r.kernel.org, linux-rdma@...r.kernel.org,
        syzbot+8fcbb77276d43cc8b693@...kaller.appspotmail.com
Subject: Re: [PATCH rdma-rc] RDMA/cma: Clear all multicast request fields

On Mon, Jan 10, 2022 at 08:05:40PM +0200, Leon Romanovsky wrote:

> > We should probably check the PS even earlier to prevent the IB side
> > from having the same issue.
> 
> What do you think about this?

IB is a bit different, it has a bunch of PS's that are UD compatible..

Probably what we really want here is to check/restrict the CM ID to
SIDR mode, which does have the qkey and is the only mode that makes
sense to be mixed with multicast, and then forget about port space
entirely.

It may be that port space indirectly restricts the CM ID to SIDR mode,
but the language here should be 'is in sidr mode', not some confusing
open coded port space check.

I'm also not sure of the lifecycle of the qkey, qkeys only exist in
SIDR mode so obviously anything that sets/gets a qkey should be
restriced to SIDR CM IDs..

> diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c
> index 835ac54d4a24..0a1f008ca929 100644
> +++ b/drivers/infiniband/core/cma.c
> @@ -4669,12 +4669,8 @@ static int cma_join_ib_multicast(struct rdma_id_private *id_priv,
>         if (ret)
>                 return ret;
> 
> -       ret = cma_set_qkey(id_priv, 0);
> -       if (ret)
> -               return ret;
> -
>         cma_set_mgid(id_priv, (struct sockaddr *) &mc->addr, &rec.mgid);
> -       rec.qkey = cpu_to_be32(id_priv->qkey);
> +       rec.qkey = cpu_to_be32(RDMA_UDP_QKEY);

And I'm not sure this makes sense? The UD qkey should still be
negotiated right?

Jason