mirror of
				https://github.com/torvalds/linux.git
				synced 2025-11-04 02:30:34 +02:00 
			
		
		
		
	bpf, sockmap: Don't let sock_map_{close,destroy,unhash} call itself
sock_map proto callbacks should never call themselves by design. Protect against bugs like [1] and break out of the recursive loop to avoid a stack overflow in favor of a resource leak. [1] https://lore.kernel.org/all/00000000000073b14905ef2e7401@google.com/ Suggested-by: Eric Dumazet <edumazet@google.com> Signed-off-by: Jakub Sitnicki <jakub@cloudflare.com> Acked-by: John Fastabend <john.fastabend@gmail.com> Link: https://lore.kernel.org/r/20230113-sockmap-fix-v2-1-1e0ee7ac2f90@cloudflare.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
This commit is contained in:
		
							parent
							
								
									74bc3a5acc
								
							
						
					
					
						commit
						5b4a79ba65
					
				
					 1 changed files with 34 additions and 27 deletions
				
			
		| 
						 | 
					@ -1569,15 +1569,16 @@ void sock_map_unhash(struct sock *sk)
 | 
				
			||||||
	psock = sk_psock(sk);
 | 
						psock = sk_psock(sk);
 | 
				
			||||||
	if (unlikely(!psock)) {
 | 
						if (unlikely(!psock)) {
 | 
				
			||||||
		rcu_read_unlock();
 | 
							rcu_read_unlock();
 | 
				
			||||||
		if (sk->sk_prot->unhash)
 | 
							saved_unhash = READ_ONCE(sk->sk_prot)->unhash;
 | 
				
			||||||
			sk->sk_prot->unhash(sk);
 | 
						} else {
 | 
				
			||||||
		return;
 | 
							saved_unhash = psock->saved_unhash;
 | 
				
			||||||
 | 
							sock_map_remove_links(sk, psock);
 | 
				
			||||||
 | 
							rcu_read_unlock();
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
						if (WARN_ON_ONCE(saved_unhash == sock_map_unhash))
 | 
				
			||||||
	saved_unhash = psock->saved_unhash;
 | 
							return;
 | 
				
			||||||
	sock_map_remove_links(sk, psock);
 | 
						if (saved_unhash)
 | 
				
			||||||
	rcu_read_unlock();
 | 
							saved_unhash(sk);
 | 
				
			||||||
	saved_unhash(sk);
 | 
					 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
EXPORT_SYMBOL_GPL(sock_map_unhash);
 | 
					EXPORT_SYMBOL_GPL(sock_map_unhash);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					@ -1590,17 +1591,18 @@ void sock_map_destroy(struct sock *sk)
 | 
				
			||||||
	psock = sk_psock_get(sk);
 | 
						psock = sk_psock_get(sk);
 | 
				
			||||||
	if (unlikely(!psock)) {
 | 
						if (unlikely(!psock)) {
 | 
				
			||||||
		rcu_read_unlock();
 | 
							rcu_read_unlock();
 | 
				
			||||||
		if (sk->sk_prot->destroy)
 | 
							saved_destroy = READ_ONCE(sk->sk_prot)->destroy;
 | 
				
			||||||
			sk->sk_prot->destroy(sk);
 | 
						} else {
 | 
				
			||||||
		return;
 | 
							saved_destroy = psock->saved_destroy;
 | 
				
			||||||
 | 
							sock_map_remove_links(sk, psock);
 | 
				
			||||||
 | 
							rcu_read_unlock();
 | 
				
			||||||
 | 
							sk_psock_stop(psock);
 | 
				
			||||||
 | 
							sk_psock_put(sk, psock);
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
						if (WARN_ON_ONCE(saved_destroy == sock_map_destroy))
 | 
				
			||||||
	saved_destroy = psock->saved_destroy;
 | 
							return;
 | 
				
			||||||
	sock_map_remove_links(sk, psock);
 | 
						if (saved_destroy)
 | 
				
			||||||
	rcu_read_unlock();
 | 
							saved_destroy(sk);
 | 
				
			||||||
	sk_psock_stop(psock);
 | 
					 | 
				
			||||||
	sk_psock_put(sk, psock);
 | 
					 | 
				
			||||||
	saved_destroy(sk);
 | 
					 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
EXPORT_SYMBOL_GPL(sock_map_destroy);
 | 
					EXPORT_SYMBOL_GPL(sock_map_destroy);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
| 
						 | 
					@ -1615,16 +1617,21 @@ void sock_map_close(struct sock *sk, long timeout)
 | 
				
			||||||
	if (unlikely(!psock)) {
 | 
						if (unlikely(!psock)) {
 | 
				
			||||||
		rcu_read_unlock();
 | 
							rcu_read_unlock();
 | 
				
			||||||
		release_sock(sk);
 | 
							release_sock(sk);
 | 
				
			||||||
		return sk->sk_prot->close(sk, timeout);
 | 
							saved_close = READ_ONCE(sk->sk_prot)->close;
 | 
				
			||||||
 | 
						} else {
 | 
				
			||||||
 | 
							saved_close = psock->saved_close;
 | 
				
			||||||
 | 
							sock_map_remove_links(sk, psock);
 | 
				
			||||||
 | 
							rcu_read_unlock();
 | 
				
			||||||
 | 
							sk_psock_stop(psock);
 | 
				
			||||||
 | 
							release_sock(sk);
 | 
				
			||||||
 | 
							cancel_work_sync(&psock->work);
 | 
				
			||||||
 | 
							sk_psock_put(sk, psock);
 | 
				
			||||||
	}
 | 
						}
 | 
				
			||||||
 | 
						/* Make sure we do not recurse. This is a bug.
 | 
				
			||||||
	saved_close = psock->saved_close;
 | 
						 * Leak the socket instead of crashing on a stack overflow.
 | 
				
			||||||
	sock_map_remove_links(sk, psock);
 | 
						 */
 | 
				
			||||||
	rcu_read_unlock();
 | 
						if (WARN_ON_ONCE(saved_close == sock_map_close))
 | 
				
			||||||
	sk_psock_stop(psock);
 | 
							return;
 | 
				
			||||||
	release_sock(sk);
 | 
					 | 
				
			||||||
	cancel_work_sync(&psock->work);
 | 
					 | 
				
			||||||
	sk_psock_put(sk, psock);
 | 
					 | 
				
			||||||
	saved_close(sk, timeout);
 | 
						saved_close(sk, timeout);
 | 
				
			||||||
}
 | 
					}
 | 
				
			||||||
EXPORT_SYMBOL_GPL(sock_map_close);
 | 
					EXPORT_SYMBOL_GPL(sock_map_close);
 | 
				
			||||||
| 
						 | 
					
 | 
				
			||||||
		Loading…
	
		Reference in a new issue