netdev - RE: [PATCH for-rc 1/2] IB/hfi1: Restore kfree in dummy

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>] [day] [month] [year] [list]

Date:   Thu, 25 Jun 2020 18:11:36 +0000
From:   "Marciniszyn, Mike" <mike.marciniszyn@...el.com>
To:     Jason Gunthorpe <jgg@...pe.ca>
CC:     "dledford@...hat.com" <dledford@...hat.com>,
        "linux-rdma@...r.kernel.org" <linux-rdma@...r.kernel.org>,
        "Wan, Kaike" <kaike.wan@...el.com>,
        "Dan Carpenter (dan.carpenter@...cle.com)" <dan.carpenter@...cle.com>,
        "Dalessandro, Dennis" <dennis.dalessandro@...el.com>,
        "David S. Miller" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>
Subject: RE: [PATCH for-rc 1/2] IB/hfi1: Restore kfree in dummy_netdev cleanup

> 
> Gross - make sure your rework allocates netdevs using alloc_netdev.
> 

I don't think that is the correct solution.

All other users of the dummy net device embed it in other structures:

init_dummy_netdev(&mal->dummy_dev);
init_dummy_netdev(&eth->dummy_dev);
init_dummy_netdev(&ar->napi_dev);
init_dummy_netdev(&irq_grp->napi_ndev);
init_dummy_netdev(&wil->napi_ndev);
init_dummy_netdev(&trans_pcie->napi_dev);
init_dummy_netdev(&dev->napi_dev);
init_dummy_netdev(&bus->mux_dev);

There is NO explicit free of the dummy since its lifetime is controlled by its parent struct.

The fault with the current AIP use of the dummy netdev is that it confuses an is-a with a has-a relationship.

This code highlights the confusion by extending the allocation to include the real rx data:

	const int netdev_size = sizeof(*dd->dummy_netdev) +
		sizeof(struct hfi1_netdev_priv);
	<snip>
	dd->dummy_netdev = kcalloc_node(1, netdev_size, GFP_KERNEL, dd->node);

and this kludgy code to find the rx data given a net_device:

static inline
struct hfi1_netdev_priv *hfi1_netdev_priv(struct net_device *dev)
{
        return (struct hfi1_netdev_priv *)&dev[1];
}

The solution is to embed the dummy net_device in a renamed hfi1_netdev_rx struct like other use cases.

The lifetime of that struct IS controlled properly and a kfree() of the renamed struct would include the embedded dummy.

Mike