diff mbox series

net/tcp_sigpool: Use kref_get_unless_zero()

Message ID 20231222-tcp-ao-kref_get_unless_zero-v1-1-551c2edd0136@arista.com (mailing list archive)
State Accepted
Commit b901a4e276943f61e11ddb597a0abc1e7dfadf0f
Delegated to: Netdev Maintainers
Headers show
Series net/tcp_sigpool: Use kref_get_unless_zero() | expand

Checks

Context Check Description
netdev/series_format warning Single patches do not need cover letters; Target tree name not specified in the subject
netdev/tree_selection success Guessed tree name to be net-next
netdev/ynl success Generated files up to date; no warnings/errors; no diff in generated;
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 1116 this patch: 1116
netdev/cc_maintainers fail 1 blamed authors not CCed: Steen.Hegelund@microchip.com; 1 maintainers not CCed: Steen.Hegelund@microchip.com
netdev/build_clang success Errors and warnings before: 1141 this patch: 1141
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success Fixes tag looks correct
netdev/build_allmodconfig_warn success Errors and warnings before: 1143 this patch: 1143
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 11 lines checked
netdev/build_clang_rust success No Rust files in patch. Skipping build
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

Dmitry Safonov Dec. 22, 2023, 1:13 a.m. UTC 
The freeing and re-allocation of algorithm are protected by cpool_mutex,
so it doesn't fix an actual use-after-free, but avoids a deserved
refcount_warn_saturate() warning.

A trivial fix for the racy behavior.

Fixes: 8c73b26315aa ("net/tcp: Prepare tcp_md5sig_pool for TCP-AO")
Suggested-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Dmitry Safonov <dima@arista.com>
---
 net/ipv4/tcp_sigpool.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)


---
base-commit: 1a44b0073b9235521280e19d963b6dfef7888f18
change-id: 20231222-tcp-ao-kref_get_unless_zero-fe7105781ba4

Best regards,

Comments

Bagas Sanjaya Dec. 22, 2023, 4:25 a.m. UTC | #1 
On Fri, Dec 22, 2023 at 01:13:59AM +0000, Dmitry Safonov wrote:
> The freeing and re-allocation of algorithm are protected by cpool_mutex,
> so it doesn't fix an actual use-after-free, but avoids a deserved
> refcount_warn_saturate() warning.
> 
> A trivial fix for the racy behavior.
> 
> Fixes: 8c73b26315aa ("net/tcp: Prepare tcp_md5sig_pool for TCP-AO")
> Suggested-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Dmitry Safonov <dima@arista.com>
> ---
>  net/ipv4/tcp_sigpool.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/net/ipv4/tcp_sigpool.c b/net/ipv4/tcp_sigpool.c
> index 55b310a722c7..8512cb09ebc0 100644
> --- a/net/ipv4/tcp_sigpool.c
> +++ b/net/ipv4/tcp_sigpool.c
> @@ -162,9 +162,8 @@ int tcp_sigpool_alloc_ahash(const char *alg, size_t scratch_size)
>  		if (strcmp(cpool[i].alg, alg))
>  			continue;
>  
> -		if (kref_read(&cpool[i].kref) > 0)
> -			kref_get(&cpool[i].kref);
> -		else
> +		/* pairs with tcp_sigpool_release() */
> +		if (!kref_get_unless_zero(&cpool[i].kref))
>  			kref_init(&cpool[i].kref);
>  		ret = i;
>  		goto out;
> 
> ---
> base-commit: 1a44b0073b9235521280e19d963b6dfef7888f18
> change-id: 20231222-tcp-ao-kref_get_unless_zero-fe7105781ba4
> 

No observable regressions when booting the kernel with this patch applied.

Tested-by: Bagas Sanjaya <bagasdotme@gmail.com>
Eric Dumazet Dec. 22, 2023, 5:08 p.m. UTC | #2 
On Fri, Dec 22, 2023 at 2:14 AM Dmitry Safonov <dima@arista.com> wrote:
>
> The freeing and re-allocation of algorithm are protected by cpool_mutex,
> so it doesn't fix an actual use-after-free, but avoids a deserved
> refcount_warn_saturate() warning.
>
> A trivial fix for the racy behavior.
>
> Fixes: 8c73b26315aa ("net/tcp: Prepare tcp_md5sig_pool for TCP-AO")
> Suggested-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Dmitry Safonov <dima@arista.com>

Reported-by: syzbot <syzkaller@googlegroups.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
patchwork-bot+netdevbpf@kernel.org Jan. 1, 2024, 2:50 p.m. UTC | #3 
Hello:

This patch was applied to netdev/net.git (main)
by David S. Miller <davem@davemloft.net>:

On Fri, 22 Dec 2023 01:13:59 +0000 you wrote:
> The freeing and re-allocation of algorithm are protected by cpool_mutex,
> so it doesn't fix an actual use-after-free, but avoids a deserved
> refcount_warn_saturate() warning.
> 
> A trivial fix for the racy behavior.
> 
> Fixes: 8c73b26315aa ("net/tcp: Prepare tcp_md5sig_pool for TCP-AO")
> Suggested-by: Eric Dumazet <edumazet@google.com>
> Signed-off-by: Dmitry Safonov <dima@arista.com>
> 
> [...]

Here is the summary with links:
  - net/tcp_sigpool: Use kref_get_unless_zero()
    https://git.kernel.org/netdev/net/c/b901a4e27694

You are awesome, thank you!
diff mbox series

Patch

diff --git a/net/ipv4/tcp_sigpool.c b/net/ipv4/tcp_sigpool.c
index 55b310a722c7..8512cb09ebc0 100644
--- a/net/ipv4/tcp_sigpool.c
+++ b/net/ipv4/tcp_sigpool.c
@@ -162,9 +162,8 @@  int tcp_sigpool_alloc_ahash(const char *alg, size_t scratch_size)
 		if (strcmp(cpool[i].alg, alg))
 			continue;
 
-		if (kref_read(&cpool[i].kref) > 0)
-			kref_get(&cpool[i].kref);
-		else
+		/* pairs with tcp_sigpool_release() */
+		if (!kref_get_unless_zero(&cpool[i].kref))
 			kref_init(&cpool[i].kref);
 		ret = i;
 		goto out;