netdev - Re: [PATCH]e100 in linux-3.18.0: some potential bugs

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]

Date:	Sat, 20 Dec 2014 16:32:12 +0300
From:	Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
To:	Jia-Ju Bai <baijiaju1990@....com>, todd.fujinaka@...el.com
CC:	netdev@...r.kernel.org, Linux-nics@...tope.jf.intel.com,
	linux.nics@...el.com, e1000-devel@...ts.sourceforge.net
Subject: Re: [PATCH]e100 in linux-3.18.0: some potential bugs

On 12/20/2014 10:40 AM, Jia-Ju Bai wrote:

> I have actually tested e100 driver on the real hardware(Intel 82559 PCI
> Ethernet Controller), and find some bugs:
> The target file is drivers/net/ethernet/intel/e100.c, which is used to build
> e100.ko.

> (1) The function pci_pool_create is called by e100_probe when initializing
> the ethernet card driver. But when pci_pool_create is failed, which means
> that it returns NULL to nic->cbs_pool, the system crash will happen. Because
> pci_pool_alloc (in e100_alloc_cbs in e100_up in e100_open) need to use
> nic->cbs_pool to allocate the resource, but it is NULL. I suggest that a
> check can be added in the code to detect whether pci_pool_create returns
> NULL.
> (2) In the normal process, netif_napi_add is called in e100_probe, but
> netif_napi_del is not called in e100_remove. However, many other ethernet
> card drivers call them in pairs, even in the error handling paths, such as
> r8169 and igb.

    Fixing one issue per patch is the rule of thumb.

> Meanwhile, I also write the patch to fix the bugs. I have run the patch on
> the hardware, it can work normally and fix the above bugs.

    Again, your sign-off is required. See Documentation/SubmittingPatches.

> diff --git a/drivers/net/ethernet/intel/e100.c
> b/drivers/net/ethernet/intel/e100.c
> index 781065e..2631d3f 100644
> --- a/drivers/net/ethernet/intel/e100.c
> +++ b/drivers/net/ethernet/intel/e100.c
> @@ -2969,6 +2969,11 @@ static int e100_probe(struct pci_dev *pdev, const
> struct pci_device_id *ent)
>   			   nic->params.cbs.max * sizeof(struct cb),
>   			   sizeof(u32),
>   			   0);
> +	if(!(nic->cbs_pool))

    Space needed after *if*. Please run your patches thru scripts/checkpatch.pl.

[...]

WBR, Sergei

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html