Submitted: Fabrice Fontaine ( ffontaine ) - 2012-03-07 02:30:57 PST
attr is not freed if ixmlNode_setNodeProperties or
ixmlNode_setAttributeNode return an error in
Parser_processAttributeName.
Submitted: Fabrice Fontaine ( ffontaine ) - 2012-03-06 07:36:08 PST
Call to strcpy should be replaced by call to memset and strncpy to
avoid getting buffer overflows.
Submitted: Fabrice Fontaine ( ffontaine ) - 2012-03-05 14:20:58 PST
fileSize = ftell( xmlFilePtr ); can return a negative value, in this
case the function should exit (at the moment, the function exits only
if ftell returns 0).
Submitted: Nick Leverton ( leveret ) - 2010-04-19 07:44:10 PDT
Details: The recent codebase merge has removed a significant API call
which is used by several pupnp devices such as mediatomb and gmediaserver.
UpnpSetVirtualDirCallbacks() has been replaced by individual routines to
set each callback. Essentially this means that 1.6.7 will in fact be a majo
bump and 1.6.6 devices can no longer link against it. Could we have the call
reinstated please, perhaps as a wrapper around the individual calls ? As
it is, all distros will have to patch their 1.6.x apps, rebuild and re-link them.
The other removed API calls and external variables don't seem to be used
by any of the apps I have copies of, but UpnpSetVirtualDirCallbacks is
important for maintaining compatibility within 1.6.x.
Submitted: Terry Farnham ( tfarnham ) - 2011-06-23 09:45:54 PDT
Details: The following bit of xml results in a memory leak from the xml
parser:
const char *xmlbuffer="<?xml version=\"1.0\" encoding=\"utf-8\"?>
<root xmlns=\"urn:schemas-upnp-org:device-1-0\" xmlns:dlna=\"urn:schemas-dlna-org:device-1-0\">
<dlna:X_DLNADOC xmlns:dlna=\"urn:schemas-dlna-org:device-1-0\">DMS-1.50</dlna:X_DLNADOC></root>";
When I execute the following code:
IXML_Document *doc = ixmlParseBuffer(xmlbuffer);
ixmlDocument_free(doc);
It results in a memory leak in ixmlparser.c line 2107 where it calls
safe_strdup( newElement->namespaceURI ); It's difficult to figure out why.
Submitted: Sunil ( sunilangadi ) - 2011-10-02 08:28:47 PDT
Details: I observed crash in the below mentioned log statement in
function upnpfinish(file: upnpapi.c).
UpnpPrintf(UPNP_INFO, API, __FILE__, __LINE__, "Exiting UpnpFinish:
UpnpSdkInit is :%d:\n", UpnpSdkInit);
In particular it was crashing in ithread_self in
UpnpDisplayFileAndLine(file upnpdebug.c) on WIN32.
Moving the call ithread_cleanup_library() below the upnp printf call
mentioned above in function upnpfinish fixed the crash but I couldn't get
to the root of the problem.
The problem was observed on WIN32.
Submitted: Fabrice Fontaine ( ffontaine ) - 2012-03-05 06:42:18 PST
gIF_IPV4, gIF_IPV6 and gIF_IPV6_ULA_GUA might be not null terminated.
Moreover, gIF_IPV4 should be 16 characters (INET_ADDRSTRLEN) and not 22
and gIF_IPV6 should be 46 characters (INET6_ADDRSTRLEN) and not 65.
Submitted: Fabrice Fontaine ( ffontaine ) - 2012-03-05 01:55:54 PST
element was not freed if membuffer_append_str(&url_str, "http://") does
not return 0. Moreover addNew was not used.
Submitted: Fabrice Fontaine ( ffontaine ) - 2012-03-05 01:31:16 PST
Memsetting to 0 some of the structures: finfo in process_request,
job in readFromSSDPSocket, request in http_OpenHttpGetEx, job in
genaNotifyThread, job in genaNotifyAllExt, job in genaNotifyAll,
job in genaInitNotifyExt, job in genaInitNotify, LocalAddr in
getlocalhostname.
Submitted: Fabrice Fontaine ( ffontaine ) - 2012-03-05 01:15:34 PST
An out-of-bands access is raised because size of
RespInst->AcceptLanguageHeader is 200 and TmpBuf size is 180.
Submitted: Fabrice Fontaine ( ffontaine ) - 2012-02-29 02:09:43 PST
newAttrNode is not freed if newAttr->n.nodeValue = strdup(value); returns
NULL or if ixmlElement_setAttributeNodeNS(element, newAttr, NULL) does
not return IXML_SUCCESS.
Submitted: Fabrice Fontaine ( ffontaine ) - 2012-02-28 04:27:54 PST
ixmlFreeDOMString(ActNodeName); is called twice if
ixmlParseBufferEx(ActNodeName, RespNode); does not return IXML_SUCCESS.
Submitted: Fabrice Fontaine ( ffontaine ) - 2012-02-28 04:01:02 PST
There is a memory leak in ixmlDocument_createElementEx:
newElement->tagName is not freed if
newElement->n.nodeName = strdup(tagName); returns NULL.
Submitted: Marcelo Roberto jimenez ( mroberto ) - 2012-02-26 16:50:23 PST
src/genlib/net/http/httpparser.c: In function ‘parser_parse_requestline’:
src/genlib/net/http/httpparser.c:1319:28: warning: ‘index’ may be used uninitialized in this function
(cherry picked from commit 9125d82010)
Submitted: Yoichi NAKAYAMA ( yoichi ) - 2012-02-21 07:06:35 PST
In genaSubscribe() (defined in upnp/src/gena/gena_ctrlpt.c),
ActualSID and EventURL will not be freed if ScheduleGenaAutoRenew
returns UPNP_E_SUCCESS.
This fixes the an issue introduced by the previous fix.
Submitted: Yoichi NAKAYAMA ( yoichi ) - 2012-02-21 07:06:35 PST
In genaSubscribe() (defined in upnp/src/gena/gena_ctrlpt.c),
ActualSID and EventURL will not be freed if ScheduleGenaAutoRenew
returns UPNP_E_SUCCESS.
This fixes the original issue.
Submitted by Berend Dekens ( Berend Dekens ) - 2012-02-08 06:24:31 PST
In httpparser.c on line 1385 it says that HTTP 1.0 replies are blocked
because the UPnP verfication tool requires this.
I looked in the specs and as far as I can find, one should only be
carefull to send chunked communication to hosts supporting HTTP 1.1.
There is no requirement to support only HTTP 1.1.
The XBMC media server uses the Platinum UPnP library which replies
using HTTP/1.0 messages. As it is now, libupnp returns an error while
trying to parse the response while the response itself is completely
valid.
Is there a requirement in the UPnP 1.0 spec that I missed or is this
restriction self-imposed? And can it be lifted?
-------------------------------------------------------------------
Comment by Fabrice Fontaine:
Hide
Hi,
You're right, this modification should be removed. This version checking
was wrongly added in parser_parse_responseline function.
...
Attached is a patch that resolved an issue I found with a server that
gave its device description URI without a trailing slash (e.g.
`http://127.0.0.1:5555`).
Small bug fix on IPv6 Control Point: now CP will also send M-SEARCH on
site-scope address (FF05::C) instead of only sending M-SEARCH on
link-scope (FF02::C).
SF Bug tracker, ID: 3469344
Submitted: dimmman ( dimmman ) - 2012-01-04 01:44:29 PST
Details: Looking at the code (v1.6.14, upnptools.c) for UpnpResolveURL
and UpnpResolveURL2 it shows that the ExitFunction: always returns
UPNP_E_SUCCESS.
I'm farily sure it's a simple mistake that should have been "return ret;"
in both cases.
Br,
Jonny
Adding two new functions (UpnpSendAdvertisementLowPower and
UpnpUnRegisterRootDeviceLowPower) which can be used to specify values
for the three SSDP headers defined by UPnP Low Power. Those headers are
Powerstate, SleepPeriod and RegistrationState.
Putting all access to ssdpReqSock4 and ssdpReqSock6 under
INCLUDE_CLIENT_APIS compilation flag to be able to compile when
client part of library is disable.
Add a new UpnpRegisterRootDevice4 which allow user to specify a
description URL to be returned for legacy CPs (for example, CPs
searching for a v1 when the device is v2). Most of those CPs does not
work if they found a v2 in the XML description, so this new function is
only used to solve interoperability issues.
Legacy applications like linux-igd and igd2-for-linux are using those
API to create a thread pool for managing their GENA events.
Leave it to be reworked in 1.8.x.
Major bug fix in miniserver.c for IPv6, bug was introduced when
changing implementation of get_port in November 20th 2010 ("gena:fix
several compiler warnings" commit).
The files FreeList.h, LinkedList.h, ThreadPool.h and TimerThread.h
from the threautil library were being installed in the include
directory of the library, incorrectly exposing internal data structure
of the library.
Add 'requiredThreads' field to the ThreadPool structure, to avoid
a race condition when waiting for a new thread to be created. The
race condition occurs when a thread is destroyed while the master
thread is waiting for a new thread to be created.
Thanks to Chuck Thomason for pointing the problem.
Summary: Race condition can hang miniserver thread - ID: 3158591
Details:
Hello,
I have found a race condition in the thread pool handling of
libupnp-1.6.6 that periodically results in the miniserver thread
getting blocked infinitely.
In my setup, I have the miniserver thread pool configured with 1
job per thread, 2 threads minimum, and 50 threads maximum.
Just before the lockup occurs, the miniserver thread pool contains
2 threads: one worker thread hanging around from a previous HTTP
request job (let's call that thread "old_worker") and the
miniserver thread itself.
A new HTTP request comes in. Accordingly, the miniserver enters
schedule_request_job() and then ThreadPoolAdd(). In
ThreadPoolAdd(), the job gets added to the medium-priority queue,
and AddWorker() is called. In AddWorker(), jobs = 1 and threads =
1, so CreateWorker gets called.
When we enter CreateWorker(), tp->totalThreads is 2, so
currentThreads is 3. The function creates a new thread and then
blocks on tp->start_and_shutdown. The miniserver thread expects
the newly created thread to increment tp->totalThreads and then
signal the condition variable to wake up the miniserver thread and
let it proceed.
The newly created thread starts in the WorkerThread() function. It
increments tp->totalThreads to 3, does a broadcast on the
start_and_shutdown condition, and starts running its job. However,
before the miniserver thread wakes up, "old_worker" times out. It
sees that there are no jobs in any queue and that the total number
of threads (3) is more than the minimum (2). As a result, it
reduces tp->totalThreads to 2 and dies.
Now the miniserver thread finally wakes up. It checks
tp->totalThreads and sees that its value is 2, so it blocks on
tp->start_and_shutdown again. It has now "missed" seeing
tp->totalThreads get incremented to 3 and will never be unblocked
again.
When this issue does occur for a server device, the miniserver
port remains open, but becomes unresponsive since the miniserver
thread is stuck. SSDP alive messages keep getting sent out, as
they are handled by a separate thread. Reproducing the issue is
difficult due to the timing coincidence involved, but in my
environment I am presently seeing it at least once a day. I
figured out the sequence described above through addition of my
own debug logs.
The relevant code involved in this bug has not changed
substantially in libupnp-1.6.10, though I am planning to test
against 1.6.10 as well in the near future.
Do you have any input for an elegant fix for this issue?
Thanks,
Chuck Thomason
Details:
Hello. I trying compile libupnp-1.6.10 on the Fedora 14 MinGW
Environment and get many errors. I create patch to fix it. With this
patch i can get static library. This patch is very raw.
Submitted: Ivan Romanov (ivanromanov) - 2010-12-16 23:29:19 UTC
Currently, http_SendMessage was not able to write to write a buffer
due to a bad use of file_buf instead of buf. This bug was introduced by
the 0197-Doxygen-reformating-compiler-warnings patch.
Currently, Upnp_Event_Subscribe always contains an empty chain in the
Sid parameter. This patch now saves the client Subscription ID in this
parameter so Control Points can see and use the same SID in the
Upnp_Event_Subscribe and in the Upnp_Event structures.
Currently, in notify_send_and_recv function, pupnp waits for
HTTP_DEFAULT_TIMEOUT seconds when trying to send a GENA notification.
When there is a lot of notifications with CPs which was disconnected
without unsusbcribing, all the pupnp threads are blocked on this
timeout. To correct, this issue, this patch adds a new variable,
GENA_NOTIFICATION_SENDING_TIMEOUT, which can be used to lower the
timeout so GENA threads return quickly when writing is impossible. By
the same mean, pupnp waits the CP's answer to the NOTIFY for
HTTP_DEFAULT_TIMEOUT seconds, so this patch adds a new variable,
GENA_NOTIFICATION_ANSWERING_TIMEOUT, to customize this value.
Currently, pupnp is using a blocking connect to sends GENA
notifications. As a result, when there is a lot of notifications with
CPs which were disconnected without unsusbcribing, all the pupnp
threads are blocked for 20s (timeout). To correct this issue, this
patch replace the call to connect with a call to private_connect and add
a compilation flag to disable blocking TCP connections, so if we are not
able to connect to the CP, the notification is lost.
SF Bug Tracker - ID: 3104527
Submitted: OBATA Akio ( obache ) - 2010-11-07 07:10:28 BRST
In threadutil/inc/ithread.h, it is expected that
PTHREAD_MUTEX_RECURSIVE is defined as macro. But on DragonFly BSD,
it is defined as enum, so not works as expected.
Attachment patch treat that DragonFly BSD always
have PTHREAD_MUTEX_RECURSIVE.
SF Bug Tracker - ID: 3104521
Submitted: OBATA Akio ( obache ) - 2010-11-07 07:03:44 BRST
In configure.ac
AC_CHECK_FUNCS(ftime,, [AC_CHECK_LIB(compat, ftime)])
But since version 1.6.3, ftime(3) is not used, so it should be
removed, or introduce unwanted linkage with -lcompat.
Fix for bug introduced in samples code in svn revision 502, commit
git:25c908c558c8e60eb386c155a6b93add447ffec0
Sample device and combo were aborting with the message:
"***** SampleUtil_Initialize was called multiple times!"
Put the loop to send multiple copies of each SSDP advertisements in
ssdp_server.c instead of ssdp_device.c so we have only one call to
imillisleep ( SSDP_PAUSE ) to speed up advertisements.
Previously, NUM_COPY was used in ssdp_device.c to send multiple copies
of each advertisements but also multiple replies to each M-SEARCH
request. As sending multiple replies is not compliant with HTTPU/MU
spec, NUM_COPY has been set to 1 in an older patch. However, as this
variable is not needed and has been replaced with SSDP_COPY, it has
been removed.
Currently, SSDP_COPY is used only to send multiple M-SEARCH requests (in
ssdp_ctrlpt.c). With this patch, SSDP_COPY is also used to send multiple
copies of each advertisements packets (in ssdp_device.c).
By "Carl Benson" <carl.benson@windriver.com>:
I had to do some modifications myself though, because the Android
build system insists on having a file named "util.h" taking precedence
in its include path, libupnp gets confused because of the same filename
in upnp/src/inc/util.h
Fix a bug in miniserver.c, in which maxMiniSock was wrongly declared as
unsigned int and as a result it was beeng set to ((unsigned int)(-1)).
As a result, after beeing incremented, it became zero, and this value
was beeing used in the select() call.
Thanks to Fabrice Fontaine for helping and testing with this issue.
the sleep() call, it was just a workaround.
SF Bug Tracker [ 3086852 ] 99% CPU loop in miniserver.c on a non ipv6
system.
Submitted by: Jin ( jin_eld ) - 2010-10-13 19:29:13 UTC
I cross compiled libupnp 1.6.7 for ARM9 using the --disable-ipv6
option, my system is an ipv4 only setup.
I do not know why this problem only appears when running the app in the
background (for instance using nohup &), but then it starts using 99%
CPU.
I traced the problem down to the select() call in miniserver.c in the
RunMiniServer() function. Select returns code 1, but errno is set to
"Socket operation on non-socket", I also see this when running my app
under strace.
I set all ...Sock6 variables to INVALID_SOCKET to make sure that they
do not get added to the FD_SET and the problem is gone.
Adding a configure flag to disable GENA notification reordering as even
with an imillisleep(1), this mechanism consumes too much CPU on embedded
devices when there is a burst of notifications.
When a device with embedded devices (like IGD) when created and one of
the embedded devices did not have any service, there was a Segmentation
Fault (see SF Tracker [ 2688125 ]).
When a lot of notifications were generated by a device in a short
period of time then 100% of the CPU was used to reorder those
notifications by pushing back the thread in the job queue. This
mechanism has been modified so now thread sleep 1 ms before being
pushed back into the job queue.
Removing DEFAULT_SCHED_PARAM parameter and use
sched_get_priority_min(DEFAULT_POLICY) instead.
