Date: Mon, 12 Feb 2001 19:40:58 -0600 From: Jonathan Lemon <jlemon@flugsvamp.com> To: Archie Cobbs <archie@dellroad.org> Cc: Jonathan Lemon <jlemon@flugsvamp.com>, kris@obsecurity.org, net@freebsd.org Subject: Re: [itojun@iijlab.net: accept(2) behavior with tcp RST right after handshake] Message-ID: <20010212194058.A1479@prism.flugsvamp.com> In-Reply-To: <200102130051.QAA72498@curve.dellroad.org> References: <20010212122855.A92213@prism.flugsvamp.com> <200102130051.QAA72498@curve.dellroad.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Mon, Feb 12, 2001 at 04:51:48PM -0800, Archie Cobbs wrote:
> Jonathan Lemon writes:
> > > > Did you guys agree on a commit-worthy fix yet?
> > >
> > > I wasn't party to the issue that generated this thread in the first
> > > place, but.. I think the concensus is that if accept(2) returns
> > > an error then this will break some applications, so instead it
> > > should return a socket which will itself return an error on the
> > > first operation. Somebody correct me if I'm wrong.
> >
> > No, as this is the current behavior. The change will be for accept
> > to return an error, on the basis that 1) most apps already do the
> > wrong thing now anyway, and 2) it brings us closer to a 'standard',
> > e.g.: what other systems are doing as well.
>
> I don't understand then.
>
> What is the problem with the current behavior? Is this just an
> optimization or a real bug fix? I'd say it's not worth changing
> if it's just an optimization, because too many things will break.
> Several apps have already been pointed out.
>
> And what do you mean by ``most apps already do the wrong thing now''?
Consider the following bit of "canonical" code, of which some variant
is probably found in almost all network programs:
struct sockaddr_in sin;
len = sizeof(sin);
fd = accept(s, (struct sockaddr *)&sin, &len);
if (fd == -1)
err(1, "accept");
printf("peer address: %s\n", inet_ntoa(sin.sin_addr));
The bug with this code is that it blindly uses ``sin'' after accept
returns, since the usage of the returned sockaddr must first be qualified
by checking the returned value of len. Current behavior is for accept
to return a valid fd, and set len to 0.
To my knowledge, most applications (certainly those I inspected) do not
bother to check 'len' before using 'sin', and thus do the wrong thing;
this is the bug that BIND ran into. (just to be clear, this is a bug
in the application code, not the kernel; although you could make the
case that it is a documentation bug as well).
It seems that other systems (and maybe the SuS?) expect accept to return
ECONNABORTED in this case. Both BIND and Apache expect this, at least.
It seems to me that the odds of an application being able to correctly
handle an error return from accept() are far greater than the odds that
the code correctly checks 'len' upon return from accept. This, combined
with the standard, seems to be rationale enough to make the change.
--
Jonathan
To Unsubscribe: send mail to majordomo@FreeBSD.org
with "unsubscribe freebsd-net" in the body of the message
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20010212194058.A1479>