netdev - Re: null pointer dereference caused by a188a54d1162 ("macvlan: simplify the structure port")

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:	Wed, 13 Aug 2014 16:00:39 -0700
From:	Cong Wang <cwang@...pensource.com>
To:	"Keller, Jacob E" <jacob.e.keller@...el.com>
Cc:	"netdev@...r.kernel.org" <netdev@...r.kernel.org>,
	"dingtianhong@...wei.com" <dingtianhong@...wei.com>
Subject: Re: null pointer dereference caused by a188a54d1162 ("macvlan:
 simplify the structure port")

On Wed, Aug 13, 2014 at 2:57 PM, Keller, Jacob E
<jacob.e.keller@...el.com> wrote:
> Hello,
>
> I found a null pointer dereference caused by instantiating multiple
> macvlans on a single parent port device. Below is a copy of the NULL
> pointer dereference stack dump.
>
[...]
>
> This was done on the head of net tree with the offending patch reverted,
> but I also added in a trace_printk to show when the macvlan port was
> empty via the list_head vlans parameter. As you can see above, the
> macvlan_uninit is called multiple, but only after the dellink has been
> called. We call list_del on the vlan item in macvlan_dellink, but then
> we later call macvlan_uninit for each item. Because we deleted the item
> from the list in macvlan_dellink, the list is already emptied completely
> by the time we call macvlan_uninit. In the bugged case, we would have
> ended up calling macvlan_port_destroy for each macvlan we created. (Four
> in my scenario above.)
>
> I think a possible fix besides simply reverting the patch might be to
> move the list_del into the macvlan_uninit. This is the same place we
> originally decremented the count, so thus makes more sense than doing
> the list_del in the macvlan_dellink function.
>
> Hopefully the above provided information is enough to determine a good
> solution to resolve the issue. My opinion is that either the above
> list_del fix or simply reverting this patch is enough.

Or just checking against NULL before calling macvlan_port_destroy().
But that will make the first macvlan_uninit() free the port instead of the
last one. I don't know, I'd vote to just revert that commit, it brings nothing
but a bug.
--
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