内核net_device设备框架的一个缺陷

前几天在看Linux内核源码时，发现一个net_device设备框架的一个问题，以至于upstream的内核源码中，至少有12个设备驱动和虚拟设备存在内存泄漏的风险。

struct net_device_ops是net_device结构的操作函数，用于定义该设备自定义的操作函数，其定义有将近200行。—— 好可怕的struct定义，而struct net_device结构本身，其定义更长，260+行。（net_device应该是内核里面最大的struct了，被诟病很久了，期待refactor）。

下面仅列出net_device_ops的一部分：

内核代码已经是很geek的代码了，但大家可以看看，内核中的“命名”起的非常好，从成员变量的名称上，就可以看出该函数的用途。所以，变量命名不是越短越好，而是尽量让读者看得明白，易于维护。当然在满足这个条件下，名字短点更好。

今天的重点是其中的两个成员函数：ndo_init和ndo_uninit。其中前者用于初始化net_dev，包括申请资源，如内存等。而ndo_uninit，自然是做初始化的反向动作，释放资源等等。

如果一切都是这样工作的，那世界该有多美好啊~~~可惜，在前文中提到的net_device结构中，还有这样一个成员变量void (*destructor)(struct net_device *dev)。这个函数指针用于net_device unregister时，做自定义“析构”（借用C++的概念:)）动作，也用于资源清理。

Bang，bug出现了。以team driver为例——这个driver的用途，请自行百度，简单说就是增强版的bond，其ndo_init函数如下：

申请了一块PerCPU内存给team->pcpu_stats，而其ndo_uninit函数如下：

其并没有释放dev->pcpu_stats指向的内存。这块内存是由team driver的destructor函数释放的。

这样看，好像也没有什么问题。无非是一个函数用于申请资源，而由两个函数释放资源。

这时，我们来查看一个关键函数register_netdevice，这个函数用于注册net_device设备。

int register_netdevice(struct net_device *dev)

{

...... ......

/* Init, if this function is available */

if (dev->netdev_ops->ndo_init) {

ret = dev->netdev_ops->ndo_init(dev);

if (ret) {

if (ret > 0)

ret = -EIO;

goto out;

}

}

/* 中间处理出错，则跳转到err_uninit */

out:

return ret;

err_uninit:

if (dev->netdev_ops->ndo_uninit)

dev->netdev_ops->ndo_uninit(dev);

goto out;

}

上面不能直接上图片，手机上看代码体验就很差了。简单说明一下，register_netdevice会调用ndo_init来初始化net_dev设备，如果在后面的处理中出错，则进入错误处理，调用no_uninit反向初始化net_dev设备。

这时，就出现一个内存泄漏的隐患：只要是在ndo_init函数中申请了资源，且这个资源不是在ndo_uninit中释放，而是在destructor中释放。那么在register_netdevice中调用ndo_init后出错，就会产生内存或者资源泄漏。register_netdevice失败后，根据框架的设计，一般都是直接调用free_netdev释放net_device结构。所以，ndo_init申请的资源就彻底丢失了。

这个bug的根本原因，是net_device框架的一个设计缺陷造成的。按照软件设计原则，是不应该一处申请内存，两处分别释放。内核源码已经是高水平程序员的杰作了，并且时时刻刻都有大量的人员在review和阅读源码。在这样的条件下，违反设计原则的代码，都会产生bug。可想而知，对于大部分团队的开发水平和质量控制下，违反设计原则的代码，会导致多少Bug！

所以，这告诉我们。书不是白读的，前人总结的经验一般都是有血淋淋的教训。软件开发的一些原则，一定要坚持遵守！

PS：发现这个缺陷后，我搜索了内核源码，判断upstream中有12处含有内存泄漏风险的bug。但是我不敢轻易改动net_device框架（牵涉太多了），于是只能针对这12个driver进行workaround的修改。在Review的过程中，netdev的maintainer David Miller决定亲自修改这个bug，目前已经在upstream中了。

争取每周都能写一篇，这次的来晚了一天。