Re: getsockopt(2)

[Robert Swindells <rjs%fdy2.co.uk@localhost>]

christos%astron.com@localhost (Christos Zoulas) wrote:
>In article <x7bmjabmv3.fsf%ren.fdy2.co.uk@localhost>,
>Robert Swindells  <rjs%fdy2.co.uk@localhost> wrote:
>>
>>I wrote:
>>>Does anyone know why we don't use the input 'optlen' parameter to the
>>>getsockopt(2) syscall, we do write back to it on return.
>>>
>>>In ip_output() there is this, which suggests that someone had come
>>>across this before.
>>>
>>>#if 0   /* defined(IPSEC) */
>>>                case IP_IPSEC_POLICY:
>>>                {
>>>                        struct mbuf *m = NULL;
>>>
>>>                        /* XXX this will return EINVAL as sopt is empty */
>>>                        error = ipsec4_get_policy(inp, sopt->sopt_data,
>>>                            sopt->sopt_size, &m);
>>>                        if (error == 0)
>>>                                error = sockopt_setmbuf(sopt, m);
>>>                        break;
>>>                }       
>>>#endif /*IPSEC*/  
>>>
>>>There are also lots of places in sctp_usrreq that want to use it.
>>>
>>>I can set it with the following patch (line numbers will be slightly
>>>out), but wondered if there was a reason for the current behaviour.
>>>
>>>Index: uipc_syscalls.c
>>>===================================================================
>>>RCS file: /cvsroot/src/sys/kern/uipc_syscalls.c,v
>>>retrieving revision 1.187
>>>diff -u -r1.187 uipc_syscalls.c
>>>--- uipc_syscalls.c	20 Jun 2017 20:34:49 -0000	1.187
>>>+++ uipc_syscalls.c	14 Oct 2017 21:33:09 -0000
>>>@@ -1235,7 +1240,7 @@
>>> 	if ((error = fd_getsock1(SCARG(uap, s), &so, &fp)) != 0)
>>> 		return (error);
>>> 
>>>-	sockopt_init(&sopt, SCARG(uap, level), SCARG(uap, name), 0);
>>>+	sockopt_init(&sopt, SCARG(uap, level), SCARG(uap, name), valsize);
>>> 
>>> 	if (fp->f_flag & FNOSIGPIPE)
>>> 		so->so_options |= SO_NOSIGPIPE;
>>
>>Has anyone had any other thoughts on this ?
>
>Yes, if you don't protect valsize against MCLBYTES like setsockopt does,
>it can be used to DoS kernel. Otherwise is can be done. Is 2K enough?

Yes, 2K would be enough. Have added the check in my tree.

>>I'm thinking of adding an extra socket option, maybe SO_PARAM, that you
>>would use with setsockopt(2) to set a selector to be used by a following
>>getsockopt(2) call.
>
>This would require some state keeping in the kernel and might not be
>reliable.

I know, it is the only alternative that I can think of to changing
setsockopt(2).

>>This wouldn't fix the IPSEC usage without changing userland though.
>>
>>The alternative is to make both setsockopt(2) and getsockopt(2)
>>bidirectional.
>
>So depending on the contents of the sockval we do something different?

FreeBSD does. The calls to copy in or out the data are scattered
throughout the network stack.

Linux seems to cheat and make use of the fact that their errors are
negative to return a positive integer word value from
setsockopt(). Their implementation of getsockopt() copies in and out.

I would still prefer to change both getsockopt(2) and setsockopt(2).

I would make getsockopt always copy in the supplied optval argument.

For setsockopt, the protocol code can update sopt_size to indicate the
amount of data to be copied back. I would add a line to the PRCO_SETOPT
handler in most protocols to set sopt_size to 0 so that nothing was
copied back.

The syscall definition for setsockopt() would need to be changed to
remove the 'const' from the optval argument.

>>I haven't checked in the userland implementation of sctp_opt_info(3)
>>yet. I took the one from FreeBSD but can modify it to work with a
>>different API.
>
>Ok.

This still requires an alternative API to use. I haven't found an
example of an OS that hides one in its version of sctp_opt_info().