Netdev Archive on lore.kernel.org
 help / color / Atom feed
From: Steffen Klassert <steffen.klassert@secunet.com>
To: Xiaodong Xu <stid.smth@gmail.com>
Cc: <herbert@gondor.apana.org.au>, <davem@davemloft.net>,
	<chenborfc@163.com>, <netdev@vger.kernel.org>
Subject: Re: [PATCH] xfrm: release device reference for invalid state
Date: Mon, 11 Nov 2019 07:17:22 +0100
Message-ID: <20191111061722.GO13225@gauss3.secunet.de> (raw)
In-Reply-To: <20191108082059.22515-1-stid.smth@gmail.com>

Please make sure to always Cc netdev@vger.kernel.org on networking
patches.

Aso, what is the difference between this patch and the one you sent
before? Please add version numbers to your patches and describe the
changes between the versions.

On Fri, Nov 08, 2019 at 12:20:59AM -0800, Xiaodong Xu wrote:
> An ESP packet could be decrypted in async mode if the input handler for
> this packet returns -EINPROGRESS in xfrm_input(). At this moment the device
> reference in skb is held. Later xfrm_input() will be invoked again to
> resume the processing.
> If the transform state is still valid it would continue to release the
> device reference and there won't be a problem; however if the transform
> state is not valid when async resumption happens, the packet will be
> dropped while the device reference is still being held.
> When the device is deleted for some reason and the reference to this
> device is not properly released, the kernel will keep logging like:
> 
> unregister_netdevice: waiting for ppp2 to become free. Usage count = 1
> 
> The issue is observed when running IPsec traffic over a PPPoE device based
> on a bridge interface. By terminating the PPPoE connection on the server
> end for multiple times, the PPPoE device on the client side will eventually
> get stuck on the above warning message.
> 
> This patch will check the async mode first and continue to release device
> reference in async resumption, before it is dropped due to invalid state.
> 
> Fixes: 4ce3dbe397d7b ("xfrm: Fix xfrm_input() to verify state is valid when (encap_type < 0)")
> Signed-off-by: Xiaodong Xu <stid.smth@gmail.com>
> Reported-by: Bo Chen <chenborfc@163.com>
> Tested-by: Bo Chen <chenborfc@163.com>
> ---
>  net/xfrm/xfrm_input.c | 30 +++++++++++++++++++++---------
>  1 file changed, 21 insertions(+), 9 deletions(-)
> 
> diff --git a/net/xfrm/xfrm_input.c b/net/xfrm/xfrm_input.c
> index 9b599ed66d97..80c5af7cfec7 100644
> --- a/net/xfrm/xfrm_input.c
> +++ b/net/xfrm/xfrm_input.c
> @@ -474,6 +474,13 @@ int xfrm_input(struct sk_buff *skb, int nexthdr, __be32 spi, int encap_type)
>  	if (encap_type < 0) {
>  		x = xfrm_input_state(skb);
>  
> +		/* An encap_type of -1 indicates async resumption. */
> +		if (encap_type == -1) {
> +			async = 1;
> +			seq = XFRM_SKB_CB(skb)->seq.input.low;
> +			goto resume;
> +		}
> +
>  		if (unlikely(x->km.state != XFRM_STATE_VALID)) {
>  			if (x->km.state == XFRM_STATE_ACQ)
>  				XFRM_INC_STATS(net, LINUX_MIB_XFRMACQUIREERROR);

Why not just dropping the reference here if the state became invalid
after async resumption?


       reply index

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20191108082059.22515-1-stid.smth@gmail.com>
2019-11-11  6:17 ` Steffen Klassert [this message]
2019-11-11  6:38   ` Xiaodong Xu
2019-11-11 11:32     ` Steffen Klassert

Reply instructions:

You may reply publically 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=20191111061722.GO13225@gauss3.secunet.de \
    --to=steffen.klassert@secunet.com \
    --cc=chenborfc@163.com \
    --cc=davem@davemloft.net \
    --cc=herbert@gondor.apana.org.au \
    --cc=netdev@vger.kernel.org \
    --cc=stid.smth@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Netdev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/netdev/0 netdev/git/0.git
	git clone --mirror https://lore.kernel.org/netdev/1 netdev/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 netdev netdev/ https://lore.kernel.org/netdev \
		netdev@vger.kernel.org
	public-inbox-index netdev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.netdev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git