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] [day] [month] [year] [list]
Date:	Tue, 17 Dec 2013 14:39:19 -0500 (EST)
From:	David Miller <davem@...emloft.net>
To:	mugunthanvnm@...com
Cc:	netdev@...r.kernel.org, linux-omap@...r.kernel.org,
	george.cherian@...com
Subject: Re: [net PATCH v2 1/1] drivers: net : cpsw: pass proper device
 name while requesting irq

From: Mugunthan V N <mugunthanvnm@...com>
Date: Fri, 13 Dec 2013 14:35:27 +0530

> During checking the interrupts with "cat /proc/interrupts", it is showing
> device name as (null), this change was done with commit id aa1a15e2d where
> request_irq is changed to devm_request_irq also changing the irq name from
> platform device name to net device name, but the net device is not
> registered at this point with the network frame work, so devm_request_irq
> is called with device name as NULL, by which it is showed as "(null)" in
> "cat /proc/interrupts". So this patch moved the devm_request_irq after
> the net device register so that the device name shows as eth*.

This change is buggy.

If the request irq fails, you have to unregister the network device,
branching to clean_ale_ret is insufficient.

And this shows the more fundamental problem with your change, you
cannot register the network device before you request the IRQs
if your ->open() method assumes that the IRQs are registered already.

Which this driver does.

The very moment you call request_irq(), the interface can be brought
up and then ->open() method invoked.

Therefore, register_netdevice() absolutely must be the very last
action during the probe function.

Why don't you just use the platform device name as the interrupt
name?  The other alternative is to only register the IRQs in the
->open() routine and free the IRQ in the ->close() method.

I can't apply this, sorry.
--
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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ