linux-kernel - Re: [PATCH 4.19 003/146] rxrpc: Abstract out the calculation of whether theres Tx space

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: <20200416170515.GA29803@duo.ucw.cz>
Date:   Thu, 16 Apr 2020 19:05:15 +0200
From:   Pavel Machek <pavel@...x.de>
To:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:     linux-kernel@...r.kernel.org, stable@...r.kernel.org,
        David Howells <dhowells@...hat.com>,
        Sasha Levin <sashal@...nel.org>
Subject: Re: [PATCH 4.19 003/146] rxrpc: Abstract out the calculation of
 whether theres Tx space

Hi!

> Abstract out the calculation of there being sufficient Tx buffer space.
> This is reproduced several times in the rxrpc sendmsg code.

I don't think this is suitable for stable. It does not fix anything.

> +/*
> + * Return true if there's sufficient Tx queue space.
> + */
> +static bool rxrpc_check_tx_space(struct rxrpc_call *call, rxrpc_seq_t *_tx_win)
> +{
> +	unsigned int win_size =
> +		min_t(unsigned int, call->tx_winsize,
> +		      call->cong_cwnd + call->cong_extra);
> +	rxrpc_seq_t tx_win = READ_ONCE(call->tx_hard_ack);
> +
> +	if (_tx_win)
> +		*_tx_win = tx_win;

Plus, this is very very strange. Most callers pass NULL here, so we
do READ_ONCE(call->tx_hard_ack) and it can't be optimized out, and
then we drop the result.

> @@ -72,9 +85,7 @@ static int rxrpc_wait_for_tx_window_nonintr(struct rxrpc_sock *rx,
>  		set_current_state(TASK_UNINTERRUPTIBLE);
>  
>  		tx_win = READ_ONCE(call->tx_hard_ack);
> -		if (call->tx_top - tx_win <
> -		    min_t(unsigned int, call->tx_winsize,
> -			  call->cong_cwnd + call->cong_extra))
> +		if (rxrpc_check_tx_space(call, &tx_win))
>  			return 0;
>  
>  		if (call->state >= RXRPC_CALL_COMPLETE)

And the remaining caller already has right value in tx_win, and it is
discarded and READ again.

This does not make sense.

Best regards,
									Pavel
-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Download attachment "signature.asc" of type "application/pgp-signature" (196 bytes)