[pupnp-tracker] [ pupnp-Bugs-3157126 ] Fix segfault in httpreadwrite.c

SourceForge Headquarters 225 Broadway Suite 1600 San Diego, CA 92101 +1 (858) 454-5900

Bugs item #3157126, was opened at 2011-01-13 09:08
Message generated for change (Settings changed) made by mroberto
You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=841026&aid=3157126&group_id=166957

Please note that this message will contain a full copy of the comment thread,
including the initial issue submission, for this request,
not just the latest update.
Category: upnp
Group: trunk
>Status: Closed
Resolution: Fixed
Priority: 9
Private: No
Submitted By: Chandra (inactiveneurons)
Assigned to: Marcelo Roberto Jimenez (mroberto)
Summary: Fix segfault in httpreadwrite.c

Initial Comment:
For last modified times, the actual value instead of the pointer is being passed in. Therefore, dereferencing the last modified time, which is in fact a value and not a pointer was causing a segfault. 

Thanks,
Chandra

----------------------------------------------------------------------

Comment By: Marcelo Roberto Jimenez (mroberto)
Date: 2011-01-16 15:16

Message:
Hi Chandra,

Indeed, you caught a bug in 1.8.0. I have fixed it in the git repository,
please try it now. Feel free to reopen the issue if there's any problem.

I would rather not change the http_MakeMessage() API, because it is both
stable and clumsy. Printf like functions are really not a good idea, but
that's legacy.

Regards,
Marcelo.

----------------------------------------------------------------------

Comment By: Chandra (inactiveneurons)
Date: 2011-01-14 12:09

Message:
Hi Marcello,

Admittedly I'm not a 100% sure of the validity of the fix, but without it
our stack is crashing reliably. I think the root problem might be in the
recent API changes in UpnpFileInfo.

Anyways, here's the backtrace:

#0  0x00007fff86febd34 in timesub ()
#1  0x00007fff8701db6c in gmtsub ()
#2  0x0000000100015205 in http_MakeMessage (buf=0x1007159a0,
http_major_version=1, http_minor_version=1, fmt=0x100071a9d "cSXcsCc") at
/Users/Chandu/McnTech/code/trunk/dlna/servers/xcode/miu/../../../pupnp/upnp/src/genlib/net/http/httpreadwrite.c:1642
#3  0x0000000100066eac in process_request (req=0x100715c20,
rtype=0x1007159c4, headers=0x1007159a0, filename=0x100715980,
alias=0x100715930, RespInstr=0x1007159d0) at
/Users/Chandu/McnTech/code/trunk/dlna/servers/xcode/miu/../../../pupnp/upnp/src/genlib/net/http/webserver.c:1255
#4  0x0000000100067642 in web_server_callback (parser=0x100715c20,
req=0x100715c20, info=0x100715e10) at
/Users/Chandu/McnTech/code/trunk/dlna/servers/xcode/miu/../../../pupnp/upnp/src/genlib/net/http/webserver.c:1463
#5  0x0000000100044bb0 in dispatch_request (info=0x100715e10,
hparser=0x100715c20) at
/Users/Chandu/McnTech/code/trunk/dlna/servers/xcode/miu/../../../pupnp/upnp/src/genlib/miniserver/miniserver.c:154
#6  0x0000000100044d8d in handle_request (args=0x100205220) at
/Users/Chandu/McnTech/code/trunk/dlna/servers/xcode/miu/../../../pupnp/upnp/src/genlib/miniserver/miniserver.c:227

Here's the relevant function call from webserver.c which is causing the
problem:

******* start snippet ******

			if (http_MakeMessage(headers, resp_major, resp_minor,
			    "R" "N" "TLD" "s" "tcS" "Xc" "sCc",
			    HTTP_OK,	/* status code */
			    RespInstr->ReadSendSize,	/* content length */
			    UpnpFileInfo_get_ContentType(finfo), /* content type */
			    RespInstr,	/* language info */
			    "LAST-MODIFIED: ",
			    UpnpFileInfo_get_LastModified(finfo),
			    X_USER_AGENT,
			    extra_headers) != 0) {
				goto error_handler;
			}

******* end snippet ******

Note that the "tcS" part of the format string causes the last modified
time to be printed. However, UpnpFileInfo_get_LastModified(finfo) returns
'time_t' and not the 'time_t*' that http_MakeMessage expects. All other
calls to http_MakeMessage, at least in webserver.c, seem to be consistent
in the usage of this API, so the fix should be valid at least for that
file. I haven't checked uses in other files, so things might still be
inconsistent there. However, our testing for the past month hasn't revealed
any problems, and we've had that fix in there for a while. Please let me
know if you want further feedback on this.

Thx,
Chandra

----------------------------------------------------------------------

Comment By: Marcelo Roberto Jimenez (mroberto)
Date: 2011-01-14 10:58

Message:
Hi Chandra,

I don't believe this is a proper fix. As far as I can remember, the
http_MakeMessage API has not changed, the other source files have not been
consistently changed according to what you imply, i.e. changing "time_t *"
to "time" in the variable argument list.

Could you please post a gdb backtrace of this segfault with some relevant
variables?

Regards,
Marcelo.

----------------------------------------------------------------------

You can respond by visiting: 
https://sourceforge.net/tracker/?func=detail&atid=841026&aid=3157126&group_id=166957