On Mon, 2018-03-19 at 19:05 +0100, Greg Kroah-Hartman wrote:
4.4-stable review patch. If anyone has any objections, please let me know.
From: Gao Feng fgao@ikuai8.com
[ Upstream commit c48367427a39ea0b85c7cf018fe4256627abfd9e ]
Because sysctl_tcp_adv_win_scale could be changed any time, so there is one race in tcp_win_from_space. For example, 1.sysctl_tcp_adv_win_scale<=0 (sysctl_tcp_adv_win_scale is negative now) 2.space>>(-sysctl_tcp_adv_win_scale) (sysctl_tcp_adv_win_scale is postive now)
As a result, tcp_win_from_space returns 0. It is unexpected.
Certainly if the compiler put the sysctl_tcp_adv_win_scale into one register firstly, then use the register directly, it would be ok. But we could not depend on the compiler behavior.
This is true, but the compiler can also decide that this local variable is just an alias for the global variable and still read it twice. It is necessary to use READ_ONCE() to prevent that.
Ben.
Signed-off-by: Gao Feng fgao@ikuai8.com Signed-off-by: David S. Miller davem@davemloft.net Signed-off-by: Sasha Levin alexander.levin@microsoft.com Signed-off-by: Greg Kroah-Hartman gregkh@linuxfoundation.org
include/net/tcp.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-)
--- a/include/net/tcp.h +++ b/include/net/tcp.h @@ -1199,9 +1199,11 @@ void tcp_select_initial_window(int __spa static inline int tcp_win_from_space(int space) {
- return sysctl_tcp_adv_win_scale<=0 ?
(space>>(-sysctl_tcp_adv_win_scale)) :
space - (space>>sysctl_tcp_adv_win_scale);
- int tcp_adv_win_scale = sysctl_tcp_adv_win_scale;
- return tcp_adv_win_scale <= 0 ?
(space>>(-tcp_adv_win_scale)) :
space - (space>>tcp_adv_win_scale);
} /* Note: caller must be prepared to deal with negative returns */