[4/7] linux: Fix setsockopt fallback

Message ID 20210705183027.3804093-5-adhemerval.zanella@linaro.org
State New
Headers show
Series
  • Linux 64-bit time_t socket fixes
Related show

Commit Message

naohirot--- via Libc-alpha July 5, 2021, 6:30 p.m.
The final 2 arguments for SO_TIMESTAMP/SO_TIMESTAMPNS are being set
wrongly.

Checked on x86_64-linux-gnu and i686-linux-gnu.
---
 sysdeps/unix/sysv/linux/setsockopt.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.30.2

Comments

naohirot--- via Libc-alpha July 5, 2021, 7:07 p.m. | #1
* Adhemerval Zanella via Libc-alpha:

> The final 2 arguments for SO_TIMESTAMP/SO_TIMESTAMPNS are being set

> wrongly.

>

> Checked on x86_64-linux-gnu and i686-linux-gnu.

> ---

>  sysdeps/unix/sysv/linux/setsockopt.c | 2 +-

>  1 file changed, 1 insertion(+), 1 deletion(-)

>

> diff --git a/sysdeps/unix/sysv/linux/setsockopt.c b/sysdeps/unix/sysv/linux/setsockopt.c

> index a4780a9d33..fae7305c9d 100644

> --- a/sysdeps/unix/sysv/linux/setsockopt.c

> +++ b/sysdeps/unix/sysv/linux/setsockopt.c

> @@ -78,7 +78,7 @@ setsockopt32 (int fd, int level, int optname, const void *optval,

>  	  optname = COMPAT_SO_TIMESTAMP_OLD;

>  	if (optname == COMPAT_SO_TIMESTAMPNS_NEW)

>  	  optname = COMPAT_SO_TIMESTAMPNS_OLD;

> -	r = setsockopt_syscall (fd, level, optname, NULL, 0);

> +	r = setsockopt_syscall (fd, level, optname, optval, len);

>        }

>        break;

>      }


Maybe add a comment that the recvmsg code does conversation, so the old
socket option is also acceptable?

The patch itself looks okay to me.

Thanks,
Florian
naohirot--- via Libc-alpha July 6, 2021, 1:07 p.m. | #2
On 05/07/2021 16:07, Florian Weimer wrote:
> * Adhemerval Zanella via Libc-alpha:

> 

>> The final 2 arguments for SO_TIMESTAMP/SO_TIMESTAMPNS are being set

>> wrongly.

>>

>> Checked on x86_64-linux-gnu and i686-linux-gnu.

>> ---

>>  sysdeps/unix/sysv/linux/setsockopt.c | 2 +-

>>  1 file changed, 1 insertion(+), 1 deletion(-)

>>

>> diff --git a/sysdeps/unix/sysv/linux/setsockopt.c b/sysdeps/unix/sysv/linux/setsockopt.c

>> index a4780a9d33..fae7305c9d 100644

>> --- a/sysdeps/unix/sysv/linux/setsockopt.c

>> +++ b/sysdeps/unix/sysv/linux/setsockopt.c

>> @@ -78,7 +78,7 @@ setsockopt32 (int fd, int level, int optname, const void *optval,

>>  	  optname = COMPAT_SO_TIMESTAMP_OLD;

>>  	if (optname == COMPAT_SO_TIMESTAMPNS_NEW)

>>  	  optname = COMPAT_SO_TIMESTAMPNS_OLD;

>> -	r = setsockopt_syscall (fd, level, optname, NULL, 0);

>> +	r = setsockopt_syscall (fd, level, optname, optval, len);

>>        }

>>        break;

>>      }

> 

> Maybe add a comment that the recvmsg code does conversation, so the old

> socket option is also acceptable?


I added the following comment:

  /* The expected type for the option is an 'int' for both
     types of timestamp formats, so there is no need to convert it.  */

> 

> The patch itself looks okay to me.
naohirot--- via Libc-alpha July 6, 2021, 2:08 p.m. | #3
* Adhemerval Zanella:

> On 05/07/2021 16:07, Florian Weimer wrote:

>> * Adhemerval Zanella via Libc-alpha:

>> 

>>> The final 2 arguments for SO_TIMESTAMP/SO_TIMESTAMPNS are being set

>>> wrongly.

>>>

>>> Checked on x86_64-linux-gnu and i686-linux-gnu.

>>> ---

>>>  sysdeps/unix/sysv/linux/setsockopt.c | 2 +-

>>>  1 file changed, 1 insertion(+), 1 deletion(-)

>>>

>>> diff --git a/sysdeps/unix/sysv/linux/setsockopt.c b/sysdeps/unix/sysv/linux/setsockopt.c

>>> index a4780a9d33..fae7305c9d 100644

>>> --- a/sysdeps/unix/sysv/linux/setsockopt.c

>>> +++ b/sysdeps/unix/sysv/linux/setsockopt.c

>>> @@ -78,7 +78,7 @@ setsockopt32 (int fd, int level, int optname, const void *optval,

>>>  	  optname = COMPAT_SO_TIMESTAMP_OLD;

>>>  	if (optname == COMPAT_SO_TIMESTAMPNS_NEW)

>>>  	  optname = COMPAT_SO_TIMESTAMPNS_OLD;

>>> -	r = setsockopt_syscall (fd, level, optname, NULL, 0);

>>> +	r = setsockopt_syscall (fd, level, optname, optval, len);

>>>        }

>>>        break;

>>>      }

>> 

>> Maybe add a comment that the recvmsg code does conversation, so the old

>> socket option is also acceptable?

>

> I added the following comment:

>

>   /* The expected type for the option is an 'int' for both

>      types of timestamp formats, so there is no need to convert it.  */


And it's possible to request the old timestamp because of the
conversation in recvmsg/recvmmsg from the old to the new timestamp.

Thanks,
Florian

Patch

diff --git a/sysdeps/unix/sysv/linux/setsockopt.c b/sysdeps/unix/sysv/linux/setsockopt.c
index a4780a9d33..fae7305c9d 100644
--- a/sysdeps/unix/sysv/linux/setsockopt.c
+++ b/sysdeps/unix/sysv/linux/setsockopt.c
@@ -78,7 +78,7 @@  setsockopt32 (int fd, int level, int optname, const void *optval,
 	  optname = COMPAT_SO_TIMESTAMP_OLD;
 	if (optname == COMPAT_SO_TIMESTAMPNS_NEW)
 	  optname = COMPAT_SO_TIMESTAMPNS_OLD;
-	r = setsockopt_syscall (fd, level, optname, NULL, 0);
+	r = setsockopt_syscall (fd, level, optname, optval, len);
       }
       break;
     }