Subject: Re: kernel option for "socket: Protocol not supported"
To: None <tech-kern@NetBSD.org, tech-security@netbsd.org>
From: Ignatios Souvatzis <is@netbsd.org>
List: tech-kern
Date: 02/22/2006 19:05:30
On Wed, Feb 22, 2006 at 12:16:28PM +0000, Iain Hibbert wrote:
> On Wed, 22 Feb 2006, Ignatios Souvatzis wrote:
> 
> > On Tue, Feb 21, 2006 at 10:20:53PM +0100, Ignatios Souvatzis wrote:
> > > On Tue, Feb 21, 2006 at 01:11:52PM -0800, jonathan@dsg.stanford.edu wrote:
> > > Yes, and that error message is bogus as you noted: [...]
> >
> > I've looked at the code - IMHO there is a clean one-line patch:
> 
> I've not looked at the code (ok just a sneak peek), but it strikes me that
> there could be other errors returned here (ENOMEM for instance), so I
> propose a different one liner, which is more accurate if you just consider
> that specific result to be not an error at this time.
> 
> -         if (sock < 0)
> +         if (sock < 0 && errno != EPROTONOSUPPORT)
> 
> > RCS file: /cvsroot/src/crypto/dist/ssh/sshconnect.c,v
> > retrieving revision 1.31
> > diff -u -r1.31 sshconnect.c
> > --- sshconnect.c        23 Apr 2005 16:53:29 -0000      1.31
> > +++ sshconnect.c        22 Feb 2006 10:29:20 -0000
> > @@ -188,7 +188,7 @@
> >         }
> >         sock = socket(ai->ai_family, ai->ai_socktype, ai->ai_protocol);
> >         if (sock < 0)
> > -               error("socket: %.100s", strerror(errno));
> > +               debug("socket: %.100s", strerror(errno));
> >
> >         /* Bind the socket to an alternative local IP address */
> >         if (options.bind_address == NULL)
> >
> > Note that the last setting of errno is printed after the for loop in the
> > function ssh_connect(), anyway - see line 395, so if _no_ address has a
> > supported address family, the user would be notified.
> 
> I wish I hadnt taken that sneak peek now. Maybe it can't happen for other
> reasons, but as error() is not an exit function, close(-1) and bind(-1)
> both seem to be possible when (options.bind_address != NULL) and the
> socket() failed
> 
> better to do:
> 
> 	if (sock < 0) {
> 		if (errno != EPROTONOSUPPORT)
> 			error(...);
> 
> 		return -1;
> 	}
> 
> Also, if you did change the error() to debug() then you should really
> change the comment on line 391 "Any error is already output" to something
> more appropriate. I would suggest "Not necessarily an error"

In your example you forgot the else debug(...), but I guess otherwise
that's fine.

	-is