netdev - Re: [net PATCH v2 1/1] drivers: net : cpsw: pass proper device name while requesting irq

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]

Message-Id: <20131217.143919.986923631175327534.davem@davemloft.net>
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