diff mbox series

[5/8] unix-stream-server: add st_dev and st_mode to socket stolen checks

Message ID 89100959528c6a3c16622cf86e6920273f5ed2d3.1614889047.git.gitgitgadget@gmail.com (mailing list archive)
State New, archived
Headers show
Series Simple IPC Cleanups | expand

Commit Message

Jeff Hostetler March 4, 2021, 8:17 p.m. UTC
From: Jeff Hostetler <jeffhost@microsoft.com>

When checking to see if our unix domain socket was stolen, also check
whether the st.st_dev and st.st_mode fields changed (in addition to
the st.st_ino field).

The inode by itself is not unique; it is only unique on a given
device.

Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
---
 unix-stream-server.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

René Scharfe March 6, 2021, 11:42 a.m. UTC | #1
Am 04.03.21 um 21:17 schrieb Jeff Hostetler via GitGitGadget:
> From: Jeff Hostetler <jeffhost@microsoft.com>
>
> When checking to see if our unix domain socket was stolen, also check
> whether the st.st_dev and st.st_mode fields changed (in addition to
> the st.st_ino field).
>
> The inode by itself is not unique; it is only unique on a given
> device.
>
> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
> ---
>  unix-stream-server.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/unix-stream-server.c b/unix-stream-server.c
> index f00298ca7ec3..366ece69306b 100644
> --- a/unix-stream-server.c
> +++ b/unix-stream-server.c
> @@ -120,8 +120,11 @@ int unix_stream_server__was_stolen(
>
>  	if (st_now.st_ino != server_socket->st_socket.st_ino)
>  		return 1;
> +	if (st_now.st_dev != server_socket->st_socket.st_dev)
> +		return 1;
>
> -	/* We might also consider the ctime on some platforms. */

Why remove that comment?  (This change is not mentioned in the commit
message.)

> +	if (!S_ISSOCK(st_now.st_mode))
> +		return 1;
>
>  	return 0;
>  }
>
Jeff Hostetler March 8, 2021, 2:14 p.m. UTC | #2
On 3/6/21 6:42 AM, René Scharfe wrote:
> Am 04.03.21 um 21:17 schrieb Jeff Hostetler via GitGitGadget:
>> From: Jeff Hostetler <jeffhost@microsoft.com>
>>
>> When checking to see if our unix domain socket was stolen, also check
>> whether the st.st_dev and st.st_mode fields changed (in addition to
>> the st.st_ino field).
>>
>> The inode by itself is not unique; it is only unique on a given
>> device.
>>
>> Signed-off-by: Jeff Hostetler <jeffhost@microsoft.com>
>> ---
>>   unix-stream-server.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/unix-stream-server.c b/unix-stream-server.c
>> index f00298ca7ec3..366ece69306b 100644
>> --- a/unix-stream-server.c
>> +++ b/unix-stream-server.c
>> @@ -120,8 +120,11 @@ int unix_stream_server__was_stolen(
>>
>>   	if (st_now.st_ino != server_socket->st_socket.st_ino)
>>   		return 1;
>> +	if (st_now.st_dev != server_socket->st_socket.st_dev)
>> +		return 1;
>>
>> -	/* We might also consider the ctime on some platforms. */
> 
> Why remove that comment?  (This change is not mentioned in the commit
> message.)

I added it as a TODO to myself thinking that it might give us
additional assurances on some platforms while I was originally
getting things working.  In hindsight (and now that we have the
lockfile helping us), I didn't think it was actually needed, so
I removed it.

I didn't think it warranted a mention in the commit message.

> 
>> +	if (!S_ISSOCK(st_now.st_mode))
>> +		return 1;
>>
>>   	return 0;
>>   }
>>
>
diff mbox series

Patch

diff --git a/unix-stream-server.c b/unix-stream-server.c
index f00298ca7ec3..366ece69306b 100644
--- a/unix-stream-server.c
+++ b/unix-stream-server.c
@@ -120,8 +120,11 @@  int unix_stream_server__was_stolen(
 
 	if (st_now.st_ino != server_socket->st_socket.st_ino)
 		return 1;
+	if (st_now.st_dev != server_socket->st_socket.st_dev)
+		return 1;
 
-	/* We might also consider the ctime on some platforms. */
+	if (!S_ISSOCK(st_now.st_mode))
+		return 1;
 
 	return 0;
 }