On 12/2/2017 2:33 PM, Yossi Kuperman wrote:
On 1 Dec 2017, at 9:09, Steffen Klassert <steffen.klass...@secunet.com> wrote:
On Tue, Nov 28, 2017 at 07:55:41PM +0200, av...@mellanox.com wrote:
From: Aviv Heller <av...@mellanox.com>
Adding the state to the offload device prior to replay init in
xfrm_state_construct() will result in NULL dereference if a matching
ESP packet is received in between.
In order to inhibit driver offload logic from processing the state's
packets prior to the xfrm_state object being completely initialized and
added to the SADBs, a new activate() operation was added to inform the
driver the aforementioned conditions have been met.
We discussed this already some time ago, and I still think that
we should fix this by setting XFRM_STATE_VALID only after the
state is fully initialized.
An upcoming patch will refactor the if statement (encap_type < 0) in
xfrm_input, in order to support crypto offload with GRO disabled. Currently it
doesn’t work. This entails yet another check for the validity of the state.
Resulting in total of 3 copies: 1) for normal traffic, 2) GRO and 3) crypto
offload.
Anyway, IMO it is not right that we (the driver) allow an incoming packet to be
delivered while the SA is not yet ready. Rather than checking for an invalid
input I prefer to make sure that such a case won’t happen in the first place.
To complete the picture, there is another patch to the driver which simply drop
incoming packets that underwent successful decryption and haven’t been
activated yet. Active state merely means that the SA is present in the driver’s
hash table.
We can make a separate patch to set the state to valid once it is fully
initialized, it make sense on its own.
What do you think?
If the SA isn't ready, just don't tell the driver about it. Please
don't add yet another state for the driver to track. This should be as
simple as possible, and shouldn't be any more complex than the model
already used by ndo_vlan_rx_add_vid and ndo_vlan_rx_kill_vid.
sln
