inet_drv.c

[Klacke]

We have found another bug in the inet_drv.c file.
This one is actually pretty bad.
The problem is that if 

1. the driver is in packet http
2. the driver http state wants to read the request line (GET ...)
3. It get eiter "\n" or "\r\n" from a broken client that
   starts of with a crnl before sending the request line

The http_message() function returns 0 ... and the
file descriptor leaks away. The caller sits in inet_prim:recv0
for ever. The timer get canceled in the driver.

Im not sure that my fix is the correct one, it does seem 
to remedy the problem though. I am not entirely certain
of what happens in the other (return 0) cases that apparently
can happen.
(driver_output_term retuns 0 in various cases, but I think
 we want to continue to a) poll the fd and (b) have the
 timer going even in those cases ???

Anyway here's the patch: (boring $Id diff that you need to live with .. sorry)

An alternative would be to return -1 in http_message() but that
would break a lot of other stuff. There are many many broken clients
out there and the code is written so that it is supposed to handle
this case (but doesn't)



I also attach a program by Luke which manifestas the bug.

Cheers



/klacke






tita:common> cvs diff -r1.32 inet_drv.c 
Index: inet_drv.c
===================================================================
RCS file: /home/share/erlang/cvsroot/otp/erts/emulator/drivers/common/inet_drv.c
,v
retrieving revision 1.32
retrieving revision 1.33
diff -c -b -r1.32 -r1.33
*** inet_drv.c  25 Feb 2003 13:36:21 -0000      1.32
--- inet_drv.c  4 Mar 2003 10:11:27 -0000       1.33
***************
*** 13,19 ****
   * Portions created by Ericsson are Copyright 1999, Ericsson Utvecklings
   * AB. All Rights Reserved.''
   * 
!  *     $Id: inet_drv.c,v 1.32 2003/02/25 13:36:21 magnus Exp $
   */
  
  #ifdef HAVE_CONFIG_H
--- 13,19 ----
   * Portions created by Ericsson are Copyright 1999, Ericsson Utvecklings
   * AB. All Rights Reserved.''
   * 
!  *     $Id: inet_drv.c,v 1.33 2003/03/04 10:11:27 klacke Exp $
   */
  
  #ifdef HAVE_CONFIG_H
***************
*** 2573,2579 ****
      else
          code = tcp_message(INETP(desc), buf, len);
  
!     if (code < 0)
        return code;
      if (desc->inet.active == INET_ONCE)
        desc->inet.active = INET_PASSIVE;
--- 2573,2579 ----
      else
          code = tcp_message(INETP(desc), buf, len);
  
!     if (code <= 0)
        return code;
      if (desc->inet.active == INET_ONCE)
        desc->inet.active = INET_PASSIVE;
***************
*** 2611,2617 ****
        return inet_async_binary_data(INETP(desc), 0, bin, offs, len);
      else
        code = tcp_binary_message(desc, bin, offs, len);
!     if (code < 0)
        return code;
      if (desc->inet.active == INET_ONCE)
        desc->inet.active = INET_PASSIVE;
--- 2611,2617 ----
        return inet_async_binary_data(INETP(desc), 0, bin, offs, len);
      else
        code = tcp_binary_message(desc, bin, offs, len);
!     if (code <= 0)
        return code;
      if (desc->inet.active == INET_ONCE)
        desc->inet.active = INET_PASSIVE;
***************
*** 5558,5564 ****
                desc->i_remain = 0;
        }
  
!       if (code < 0)
            return code;
  
        count++;
--- 5558,5564 ----
                desc->i_remain = 0;
        }
  
!       if (code <= 0)
            return code;
  
        count++;




-- 
Claes Wikstrom                        -- Caps lock is nowhere and
Alteon WebSystems                     -- everything is under control          
http://www.bluetail.com/~klacke      
cellphone: +46 70 2097763
-------------- next part --------------
%%%----------------------------------------------------------------------
%%% File    : inetleak.erl
%%% Author  : Luke Gorrie <luke@REDACTED>
%%% Purpose : Test case for inet_driver leaking file descriptors
%%% Created :  3 Mar 2003 by Luke Gorrie <luke@REDACTED>
%%%----------------------------------------------------------------------

%% go/1 starts a server, and test_client/1 connects to that server and
%% hangs it with a malformed HTTP request.

-module(inetleak).
-author('luke@REDACTED').

-compile(export_all).
%%-export([Function/Arity, ...]).

go(Port) ->
    spawn_link(fun() ->
		       {ok, L} = gen_tcp:listen(Port, [{active, false},
						       binary,
						       {reuseaddr, true},
						       {packet, http}]),
		       accept_loop(L)
	       end).

accept_loop(L) ->
    case gen_tcp:accept(L) of
	{ok, S} ->
	    worker(S);
	Err ->
	    exit({accept, Err})
    end.

worker(S) ->
    io:format("~p trying a read with timeout..~n", [self()]),
    case gen_tcp:recv(S, 0, 30000) of
	{ok, Data} ->
	    io:format("~p got ~p~n", [self(), Data]),
	    worker(S);
	{error, Rsn} ->
	    io:format("~p error: ~p~n", [self(), Rsn])
    end.


test_client(Port) ->
    {ok,S} = gen_tcp:connect("localhost", Port, [{active,false}, binary]),
    ok = gen_tcp:send(S, "\r\n"),
    ok = gen_tcp:close(S).