netdev - Re: [PATCH net-next 3/4] rocker: do not make neighbour entry changes when preparing transactions

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:	Tue, 19 May 2015 17:32:15 +0900
From:	Toshiaki Makita <makita.toshiaki@....ntt.co.jp>
To:	Simon Horman <simon.horman@...ronome.com>,
	Jiri Pirko <jiri@...nulli.us>,
	Scott Feldman <sfeldma@...il.com>,
	David Miller <davem@...emloft.net>
CC:	netdev@...r.kernel.org
Subject: Re: [PATCH net-next 3/4] rocker: do not make neighbour entry changes
 when preparing transactions

On 2015/05/19 15:24, Simon Horman wrote:
> rocker_port_ipv4_nh() and in turn rocker_port_ipv4_neigh() may be
> be called with trans == SWITCHDEV_TRANS_PREPARE and then
> trans == SWITCHDEV_TRANS_COMMIT from switchdev_port_obj_set() via
> fib_table_insert().
> 
> Although I have been unable to find a scenario where a bug manifests
> I do see some incorrect behaviour when adding a fib entry with a gateway.
> 
> The first time that rocker_port_ipv4_nh() is called, with
> trans == SWITCHDEV_TRANS_PREPARE, _rocker_neigh_add()
> adds a new entry to the neigh table.
> 
> And the second time  rocker_port_ipv4_nh() is called, with
> trans == SWITCHDEV_TRANS_COMMIT, that entry is found and
> _rocker_neigh_update() is called.
> 
> Assuming the transaction runs to completion this appears to be harmless.
> I suspect it would be not correct if the transaction was aborted.

Hi Simon,

I encountered oops by this bug without abort.
Adding a route by ip command and pinging to the gateway triggers oops.

The scenario I guess is below:
In the PREPARE phase, a new entry is allocated and added to the hash
table. In the COMMIT phase, the allocated entry is the same object as
that in the hash table. As "adding" is not true in the COMMIT phase, the
entry is freed in rocker_port_ipv4_nh() while it is still in the hash
table. This causes use-after-free and maybe corrupts pointers in the entry.

This seems to fix that problem.

Thanks,
Toshiaki Makita

--
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