Thank you Darren for your comments.

The team will respond to your comments within the next few days.

Thanks,
Anders

Darren Reed wrote:
> Just to get things started...
>
> soconfig.c line 262
> Type: T
> Priority: 2
> Comment: explicit strings for path names should not be used directly
> in code, rather there should be #defines used (e.g. DEVDIR)
>
> audit_event.c lines 3331, 3332
> Type: T
> Priority: 1
> Comment: if volo is introducing a new api through which new socket
> types can be introduced, then code such as that at lines 3331 and
> 3332 is going to cause us problems because it incorrectly assumes
> that the maximum size of a sockaddr structure is sockaddr_in6. The
> actual maximum size is that of sockaddr_storage. Feedback from the
> project is required to understand what sorts of sockets they plan
> to allow to be loaded via sockfs and if they've given any consideration
> to what this means for code that currently assumes that sockaddr_in6
> is the biggest structure required (false.) I'll raise this issue
> just once.
>
>
> audit_event.c lines 3372-3374
> Type: T
> Priority: 1
> Comment: This project is adding in the mechanism to support loadable
> socket modules (sockmod) but it would appear that parts of the kernel
> are yet to be fashioned in a way that removes specific knowledge of
> particular socket types from it. For example, here we see a switch
> statement built around the known behaviour of specific types of
> address families. Given that new address families can purportedly
> be introduced by a new sockmod, code that makes explicit checks
> like what is being done here needs to be rethought. For example,
> what if I want to add a sockmod that provides a socket interface
> to NetBUI? Or maybe IPX? (Netware) Can I do it or are there still
> significant obstacles in the way such as the code here? I'll raise
> this issue just once.
>
> audit_event.c lines 3384-3389
> Type: T
> Priority: 1
> Comment: The potential return of an error is being ignored here in
> two ways: the use of (void) to ignore what socket_getsockname() returns
> and later when add_sock_token is set to 1. If we assume that the
> relevant socket implementations will return an error if they do not
> support the operation, then can't we solve the aforementioned problem
> and this one by always attempting to do socket_getsockname() and
> only setting add_sock_token() if both calls succeed? I'll raise this
> issue just once.
>
> audit_event.c
> Type: T
> Priority: 3
> Comment: Further reading of this file suggests that the existing
> interfaces in this project do not go far enough. As one comment
> in the pre-existing code suggests, what about SOCK_RDM and
> SOCK_SEQPAKCET? (I'm sure there are others too.) As a rough guide,
> follow-on work for volo could consist of developing new interfaces
> that remove the need for any specific references to anything IP
> related, from this file.
>
> smb_net.c
> Type: T
> Priority: 1
> Comment: Why are there "#ifdef VOLO_FIX"'s in this code?
>
>
> _______________________________________________
> networking-discuss mailing list
> [email protected]
>   

_______________________________________________
networking-discuss mailing list
[email protected]

Reply via email to