| Author |
Message |
|
| klacke at bluetail.com |
 Posted: Tue Mar 04, 2003 10:24 am |
 |
|
|
Guest
|
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 "
" or "
" 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
Post generated using Mail2Forum (http://m2f.sourceforge.net) |
|
|
| Back to top |
|
| klacke at bluetail.com |
| Posted: Thu Mar 06, 2003 10:25 am |
|
|
|
Guest
|
The patch I send with lukes code that showed an error in
inet_drv.c wasn't correct.
Martin B and I have spent a couple of days !!! trying
to get the patch right. Hard.
The problem is that it isn't that easy to restart the
input when a single crnl is found in http_message().
We settled on:
***************
*** 1914,1920 ****
int c;
/* start-line = Request-Line | Status-Line */
if (n == 0)
! return 0;
h = 0;
meth_ptr = ptr;
while (n && !is_tspecial((unsigned char)*ptr)) {
--- 1914,1920 ----
int c;
/* start-line = Request-Line | Status-Line */
if (n == 0)
! return -1;
h = 0;
meth_ptr = ptr;
while (n && !is_tspecial((unsigned char)*ptr)) {
So when http_message get crnl (only) it return -1
and {http_error, Socket "
"} will be returned
all the way up to the HTPP app.
> %%%----------------------------------------------------------------------
> %%% File : inetleak.erl
> %%% Author : Luke Gorrie <luke_at_bluetail.com>
> %%% Purpose : Test case for inet_driver leaking file descriptors
> %%% Created : 3 Mar 2003 by Luke Gorrie <luke_at_bluetail.com>
> %%%----------------------------------------------------------------------
>
> %% 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_at_bluetail.com').
>
> -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, "
"),
> ok = gen_tcp:close(S).
>
--
Claes Wikstrom -- Caps lock is nowhere and
Alteon WebSystems -- everything is under control
http://www.bluetail.com/~klacke
cellphone: +46 70 2097763
Post generated using Mail2Forum (http://m2f.sourceforge.net) |
|
|
| Back to top |
|
|
|
All times are GMT
|
|
You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot vote in polls in this forum
You cannot attach files in this forum
You cannot download files in this forum
|
|
|
Digg It
Del.icio.us
Reddit
Facebook
Stumble Upon
Technorati
