From: Toshiaki Makita <makita.toshiaki@lab•ntt.co.jp>
To: Simon Horman <simon.horman@netronome•com>,
Jiri Pirko <jiri@resnulli•us>, Scott Feldman <sfeldma@gmail•com>,
David Miller <davem@davemloft•net>
Cc: netdev@vger•kernel.org
Subject: Re: [PATCH net-next 3/4] rocker: do not make neighbour entry changes when preparing transactions
Date: Tue, 19 May 2015 17:32:15 +0900 [thread overview]
Message-ID: <555AF50F.1090402@lab.ntt.co.jp> (raw)
In-Reply-To: <1432016657-24231-4-git-send-email-simon.horman@netronome.com>
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
next prev parent reply other threads:[~2015-05-19 8:32 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-19 6:24 [PATCH net-next 0/4] rocker: transaction fixes Simon Horman
2015-05-19 6:24 ` [PATCH net-next 1/4] rocker: do not delete fdb entries in rocker_port_fdb_flush() when preparing transactions Simon Horman
2015-05-19 15:57 ` Scott Feldman
2015-05-19 6:24 ` [PATCH net-next 2/4] rocker: do not modify fdb table in rocker_port_fdb() " Simon Horman
2015-05-19 16:01 ` Scott Feldman
2015-05-19 6:24 ` [PATCH net-next 3/4] rocker: do not make neighbour entry changes " Simon Horman
2015-05-19 8:32 ` Toshiaki Makita [this message]
2015-05-20 1:53 ` Simon Horman
2015-05-19 16:11 ` Scott Feldman
2015-05-19 22:33 ` Simon Horman
2015-05-19 6:24 ` [PATCH net-next 4/4] rocker: make rocker_port_internal_vlan_id_{get,put}() non-transactional Simon Horman
2015-05-19 16:04 ` Scott Feldman
2015-05-19 16:13 ` [PATCH net-next 0/4] rocker: transaction fixes Scott Feldman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=555AF50F.1090402@lab.ntt.co.jp \
--to=makita.toshiaki@lab$(echo .)ntt.co.jp \
--cc=davem@davemloft$(echo .)net \
--cc=jiri@resnulli$(echo .)us \
--cc=netdev@vger$(echo .)kernel.org \
--cc=sfeldma@gmail$(echo .)com \
--cc=simon.horman@netronome$(echo .)com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox