rtems_bsdnet_fdToSocket()

[Till Straumann]

Joel Sherrill wrote:
> Till Straumann wrote:
>> Chris Johns wrote:
>>  
>>> Till Straumann wrote:
>>>    
>>>> IMHO rtems_bsdnet_fdToSocket() doesn't
>>>> verify thoroughly enough that the file
>>>> descriptor passed in is really a socket.
>>>>
>>>> If you e.g., try getsockname() on a fd
>>>> that was opened on a termios device then
>>>> a crash may result.
>>>>
>>>> I propose to let rtems_bsdnet_fdToSocket()
>>>> test not only that iop->data1 is NULL but
>>>> to add an additional test.
>>>>
>>>> Either
>>>> 1) add a new flag LIBIO_FLAGS_ISSOCK
>>>>    which is set by rtems_bsdnet_makeFdForSocket()
>>>>    and tested by rtems_bsdnet_fdToSocket()
>>>> 1a) instead of one flag we could use a few
>>>>    more bits and create a LIBIO_FLAGS_IOP_TYPE
>>>>    bitmask (in case the need for other special
>>>>    cases besides sockets arises in the future).
>>>>    The test would then look like
>>>>
>>>>    if ( LIBIO_FLAGS_IOP_TYPE(iop->flags) != 
>>>> LIBIO_FLAGS_IOP_TYPE_SOCK ) {
>>>>        errno = ENOTSOCK;
>>>>        return NULL;
>>>>    }
>>>>
>>>>    Sidenote: IMO, if data1 == NULL then errno should
>>>>              also be ENOTSOCK instead of EBADF.
>>>>
>>>>       
>>> The LIBIO flags do not describe the fd rather they deal with what 
>>> you can do. I do not like the type of fd being added to the flags.
>>>
>>> NetBSD has a type field in the fp referenced by the fd:
>>>     
>> Problemo being that libio has no such thing. It has
>> absolutely no knowledge about what is attached to
>> 'data', 'data1', 'file-info'.
>>
>> Where do you see an equivalent to fp_type in the iop
>> or how would you propose that, given a iop, the
>> code can figure out what kind of object
>> is attached to e.g., 'data1' ?
>>
>> Do you like testing the 'handlers' better?
>>   
> Looking at libcsupport/src/libio_socket.c, that's the only
> thing you can currently rely on.
>
> Might not be a bad generic check.  Worth a libio macro
> like rtems_libio_handlers_match?
Maybe, in the future.

However, I'd like to move ahead and fix rtems_bsdnet_fdToSocket()
for now. A macro can easily be added later.

I see three methods:

a) export 'rtems_bsdnet_socket_handlers' and let fdToSocket compare
    iop->handlers == &rtems_bsdnet_socket_handlers.

    This is a bit ugly since the matching 'makeFdForSocket()' does
    not use the global variable but is passed a pointer to the handler
    table explicitly.


b) change fdToSocket so that it also takes a handler argument.
    However, this means an API change and unfortunately, there
    is no public declaration of this routine so there is always the
    possibility that somewhere the old API is used.

In case we go for 'b' I'd suggest to change the name of 
'rtems_bsdnet_fdToSocket'

In any case - since I really don't like API changes I'm inclined to
bite the 'a' bullet.

T.
>
> --joel
>> T.
>>  
>>> int
>>> getsock(struct filedesc *fdp, int fdes, struct file **fpp)
>>> {
>>>         struct file     *fp;
>>>
>>>         if ((fp = fd_getfile(fdp, fdes)) == NULL)
>>>                 return (EBADF);
>>>
>>>         FILE_USE(fp);
>>>
>>>         if (fp->f_type != DTYPE_SOCKET) {
>>>                 FILE_UNUSE(fp, NULL);
>>>                 return (ENOTSOCK);
>>>         }
>>>         *fpp = fp;
>>>         return (0);
>>> }
>>>
>>> I would prefer this.
>>>
>>> Regards
>>> Chris
>>>
>>>    
>>>> 2) alternatively, rtems_bsdnet_fdToSocket() could
>>>>    test if iop->handlers == socket_handlers.
>>>>
>>>>    socket_handlers would then have to become a globally
>>>>    visible symbol (right now a static variable).
>>>>
>>>>    Unfortunately, this would introduce some asymmetry
>>>>    (fdToSocket() refers to global symbol, makeFdForSocket
>>>>    gets pointer to handlers passed in).
>>>>
>>>> If nobody objects or has a better idea then I would
>>>> implement 1a), create a PR and commit to 4.9 and HEAD.
>>>>
>>>> RFC
>>>> -Till
>>>> _______________________________________________
>>>> rtems-users mailing list
>>>> rtems-users at rtems.com
>>>> http://rtems.rtems.org/mailman/listinfo/rtems-users
>>>>       
>>
>> _______________________________________________
>> rtems-users mailing list
>> rtems-users at rtems.com
>> http://rtems.rtems.org/mailman/listinfo/rtems-users
>>   
>
>