diff mbox series

SUNRPC: mutexed access blacklist_read state variable.

Message ID 20220726201243.103800-1-attila.kovacs@cfa.harvard.edu (mailing list archive)
State New, archived
Headers show
Series SUNRPC: mutexed access blacklist_read state variable. | expand

Commit Message

Attila Kovacs July 26, 2022, 8:12 p.m. UTC
From: Attila Kovacs <attipaci@gmail.com>

bindresvport()_sa(), in bidresvport.c checks blacklist_read w/o mutex
before calling load_blacklist() which changes blacklist_read() also
unmutexed.

Clearly, the point is to read the blacklist only once on the first call,
but because the checking whether the blacklist is loaded is not mutexed,
more than one thread may race to load the blacklist concurrently, which
of course can jumble the list because of the race condition.

The fix simply moves the checking within the mutexed aread of the code
to eliminate the race condition.

---
 src/bindresvport.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Steve Dickson July 28, 2022, 1:17 p.m. UTC | #1
On 7/26/22 4:12 PM, Attila Kovacs wrote:
> From: Attila Kovacs <attipaci@gmail.com>
> 
> bindresvport()_sa(), in bidresvport.c checks blacklist_read w/o mutex
> before calling load_blacklist() which changes blacklist_read() also
> unmutexed.
> 
> Clearly, the point is to read the blacklist only once on the first call,
> but because the checking whether the blacklist is loaded is not mutexed,
> more than one thread may race to load the blacklist concurrently, which
> of course can jumble the list because of the race condition.
> 
> The fix simply moves the checking within the mutexed aread of the code
> to eliminate the race condition.
Committed (tag: libtirpc-1-3-3-rc4)

steved.
> 
> ---
>   src/bindresvport.c | 3 ++-
>   1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/src/bindresvport.c b/src/bindresvport.c
> index ef9b345..5c0ddcf 100644
> --- a/src/bindresvport.c
> +++ b/src/bindresvport.c
> @@ -164,10 +164,11 @@ bindresvport_sa(sd, sa)
>   	int endport = ENDPORT;
>   	int i;
>   
> +	mutex_lock(&port_lock);
> +
>   	if (!blacklist_read)
>   		load_blacklist();
>   
> -	mutex_lock(&port_lock);
>   	nports = ENDPORT - startport + 1;
>   
>           if (sa == NULL) {
diff mbox series

Patch

diff --git a/src/bindresvport.c b/src/bindresvport.c
index ef9b345..5c0ddcf 100644
--- a/src/bindresvport.c
+++ b/src/bindresvport.c
@@ -164,10 +164,11 @@  bindresvport_sa(sd, sa)
 	int endport = ENDPORT;
 	int i;
 
+	mutex_lock(&port_lock);
+
 	if (!blacklist_read)
 		load_blacklist();
 
-	mutex_lock(&port_lock);
 	nports = ENDPORT - startport + 1;
 
         if (sa == NULL) {