netdev - Re: [PATCH net-next 07/15] rxrpc: Fix accidental cancellation of scheduled resend by ACK parser

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:   Fri, 23 Sep 2016 21:02:36 +0300
From:   Sergei Shtylyov <sergei.shtylyov@...entembedded.com>
To:     David Howells <dhowells@...hat.com>, netdev@...r.kernel.org
Cc:     linux-afs@...ts.infradead.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next 07/15] rxrpc: Fix accidental cancellation of
 scheduled resend by ACK parser

Hello.

On 09/23/2016 06:16 PM, David Howells wrote:

> When rxrpc_input_soft_acks() is parsing the soft-ACKs from an ACK packet,
> it updates the Tx packet annotations in the annotation buffer.  If a
> soft-ACK is an ACK, then we overwrite unack'd, nak'd or to-be-retransmitted
> states and that is fine; but if the soft-ACK is an NACK, we overwrite the
> to-be-retransmitted with a nak - which isn't.
>
> Instead, we need to let any scheduled retransmission stand if the packet
> was NAK'd.
>
> Note that we don't reissue a resend if the annotation is in the
> to-be-retransmitted state because someone else must've scheduled the
> resend already.
>
> Signed-off-by: David Howells <dhowells@...hat.com>
> ---
>
>  net/rxrpc/input.c |    2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/net/rxrpc/input.c b/net/rxrpc/input.c
> index 06027b6d9c19..d3d69ab1f0a1 100644
> --- a/net/rxrpc/input.c
> +++ b/net/rxrpc/input.c
> @@ -479,6 +479,8 @@ static void rxrpc_input_soft_acks(struct rxrpc_call *call, u8 *acks,
>  		case RXRPC_ACK_TYPE_NACK:
>  			if (anno_type == RXRPC_TX_ANNO_NAK)
>  				continue;
> +			if (anno_type == RXRPC_TX_ANNO_RETRANS)
> +				continue;

    Why not fold the above 2 *if*s together? Or use *else if* at least?

MBR, Sergei