netdev - Re: [PATCH ipsec-next v2 2/6] xfrm: replay: get rid of duplicated notification code

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]

Message-ID: <20200625070741.GA2939559@bistromath.localdomain>
Date:   Thu, 25 Jun 2020 09:07:41 +0200
From:   Sabrina Dubroca <sd@...asysnail.net>
To:     Florian Westphal <fw@...len.de>
Cc:     steffen.klassert@...unet.com, netdev@...r.kernel.org
Subject: Re: [PATCH ipsec-next v2 2/6] xfrm: replay: get rid of duplicated
 notification code

Hi Florian,

2020-06-24, 10:08:00 +0200, Florian Westphal wrote:
> After previous patch, we can consolidate some code:
> 
> xfrm_replay_notify, xfrm_replay_notify_bmp and _esn all contain the
> same code at the end.
> 
> Remove it from xfrm_replay_notify_bmp/esn and reuse the one
> in xfrm_replay_notify.
> 
> Signed-off-by: Florian Westphal <fw@...len.de>
> ---
>  net/xfrm/xfrm_replay.c | 22 ++++------------------
>  1 file changed, 4 insertions(+), 18 deletions(-)
> 
> diff --git a/net/xfrm/xfrm_replay.c b/net/xfrm/xfrm_replay.c
> index e42a7afb8ee5..fac2f3af4c1a 100644
> --- a/net/xfrm/xfrm_replay.c
> +++ b/net/xfrm/xfrm_replay.c
> @@ -56,10 +56,10 @@ void xfrm_replay_notify(struct xfrm_state *x, int event)
>  		break;
>  	case XFRM_REPLAY_MODE_BMP:
>  		xfrm_replay_notify_bmp(x, event);
> -		return;
> +		goto notify;
>  	case XFRM_REPLAY_MODE_ESN:
>  		xfrm_replay_notify_esn(x, event);
> -		return;
> +		goto notify;

These two functions have some early returns that skip the
notification, but now the notification will be sent in all cases:

	static void xfrm_replay_notify_bmp(struct xfrm_state *x, int event)
	{
		<snip>

		switch (event) {
		case XFRM_REPLAY_UPDATE:
			if (...) {
				if (x->xflags & XFRM_TIME_DEFER)
					event = XFRM_REPLAY_TIMEOUT;
				else
					return;
			}

			break;


And this also changes the value that ends up in c.data.aevent. That
change will be lost after this patch.

>  	}
>  
>  	switch (event) {
> @@ -86,6 +86,8 @@ void xfrm_replay_notify(struct xfrm_state *x, int event)
>  	}
>  
>  	memcpy(&x->preplay, &x->replay, sizeof(struct xfrm_replay_state));
> +
> +notify:
>  	c.event = XFRM_MSG_NEWAE;
>  	c.data.aevent = event;
>  	km_state_notify(x, &c);

-- 
Sabrina