linux-kernel - RE: [Patch v9 03/12] net: mana: Handle vport sharing between devices

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: <PH7PR21MB32633AC8730AFB4E6C247BABCE2E9@PH7PR21MB3263.namprd21.prod.outlook.com>
Date:   Mon, 24 Oct 2022 18:45:00 +0000
From:   Long Li <longli@...rosoft.com>
To:     Yunsheng Lin <linyunsheng@...wei.com>,
        KY Srinivasan <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        Wei Liu <wei.liu@...nel.org>, Dexuan Cui <decui@...rosoft.com>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Jason Gunthorpe <jgg@...pe.ca>,
        Leon Romanovsky <leon@...nel.org>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        "shiraz.saleem@...el.com" <shiraz.saleem@...el.com>,
        Ajay Sharma <sharmaajay@...rosoft.com>
CC:     "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>
Subject: RE: [Patch v9 03/12] net: mana: Handle vport sharing between devices

> > +int mana_cfg_vport(struct mana_port_context *apc, u32
> protection_dom_id,
> > +		   u32 doorbell_pg_id)
> >  {
> >  	struct mana_config_vport_resp resp = {};
> >  	struct mana_config_vport_req req = {};
> >  	int err;
> >
> > +	/* This function is used to program the Ethernet port in the hardware
> > +	 * table. It can be called from the Ethernet driver or the RDMA driver.
> > +	 *
> > +	 * For Ethernet usage, the hardware supports only one active user on
> a
> > +	 * physical port. The driver checks on the port usage before
> programming
> > +	 * the hardware when creating the RAW QP (RDMA driver) or
> exposing the
> > +	 * device to kernel NET layer (Ethernet driver).
> > +	 *
> > +	 * Because the RDMA driver doesn't know in advance which QP type
> the
> > +	 * user will create, it exposes the device with all its ports. The user
> > +	 * may not be able to create RAW QP on a port if this port is already
> > +	 * in used by the Ethernet driver from the kernel.
> > +	 *
> > +	 * This physical port limitation only applies to the RAW QP. For RC QP,
> > +	 * the hardware doesn't have this limitation. The user can create RC
> > +	 * QPs on a physical port up to the hardware limits independent of
> the
> > +	 * Ethernet usage on the same port.
> > +	 */
> > +	mutex_lock(&apc->vport_mutex);
> > +	if (apc->vport_use_count > 0) {
> > +		mutex_unlock(&apc->vport_mutex);
> > +		return -EBUSY;
> > +	}
> > +	apc->vport_use_count++;
> > +	mutex_unlock(&apc->vport_mutex);
> > +
> >  	mana_gd_init_req_hdr(&req.hdr, MANA_CONFIG_VPORT_TX,
> >  			     sizeof(req), sizeof(resp));
> >  	req.vport = apc->port_handle;
> > @@ -679,9 +714,16 @@ static int mana_cfg_vport(struct
> > mana_port_context *apc, u32 protection_dom_id,
> >
> >  	apc->tx_shortform_allowed = resp.short_form_allowed;
> >  	apc->tx_vp_offset = resp.tx_vport_offset;
> > +
> > +	netdev_info(apc->ndev, "Configured vPort %llu PD %u DB %u\n",
> > +		    apc->port_handle, protection_dom_id, doorbell_pg_id);
> >  out:
> > +	if (err)
> > +		mana_uncfg_vport(apc);
> 
> There seems to be a similar race between error handling here and the "apc-
> >vport_use_count > 0" checking above as pointed out in v7.

Thanks for looking into this.

This is different to the locking bug in mana_ib_cfg_vport(). The vport sharing
between Ethernet and RDMA is exclusive, not shared. If another driver tries
to take the vport while it is being configured, it will fail immediately. It is by
design to prevent possible deadlock.