diff --git a/sblim-sfcb-1.3.16-embedded-crash.patch b/sblim-sfcb-1.3.16-embedded-crash.patch new file mode 100644 index 0000000..2aaf3fc --- /dev/null +++ b/sblim-sfcb-1.3.16-embedded-crash.patch @@ -0,0 +1,103 @@ +replaceClStringN() relocates items in ClStrBuf, using strlen() to get +lengths of the items. With embedded instances / objects, strlen() is not +useful at all, we must get length of the item using other means. + +The patch is quite crude, there might be better way, how to get the lenghts +(e.g. by having ClStrBuf.lenPtr[], which would track lenght of each item). + +Version 2: do not get lengths from fb->indexPtr, it is being modified, +use a copy of it instead. + +diff -up sblim-sfcb-1.3.16/objectImpl.c.embedded-crash sblim-sfcb-1.3.16/objectImpl.c +--- sblim-sfcb-1.3.16/objectImpl.c.embedded-crash 2013-04-18 16:49:51.941521957 +0200 ++++ sblim-sfcb-1.3.16/objectImpl.c 2013-04-18 16:39:30.000000000 +0200 +@@ -449,6 +449,29 @@ static void replaceClString(ClObjectHdr + return replaceClStringN(hdr, id, str, 0); + } + ++static int getBufIndexLen(int *indexPtr, int bUsed, int iUsed, int index) ++{ ++ /* ++ * Find length of 'string' in fb at given index. ++ * We cannot use strlen, because some items are not strings but embedded ++ * instances. ++ * We cannot simply substract fb->indexPtr[index+1] - fb->indexPtr[index], ++ * because the entries are not consecutive! They are shuffled by ++ * replaceClStringN(). ++ * Therefore the only way to find a length of our 'string' is to find ++ * string, which starts immediately after it. Let's call it 'nearest' ++ * string. */ ++ int nearest_start = bUsed; ++ int our_start = indexPtr[index]; ++ int i; ++ for (i = 0; i our_start && indexPtr[i] < nearest_start) { ++ nearest_start = indexPtr[i]; ++ } ++ int len = nearest_start - our_start; ++ return len; ++} ++ + static void + replaceClStringN(ClObjectHdr * hdr, int id, const char *str, unsigned int length) + { +@@ -457,15 +480,20 @@ replaceClStringN(ClObjectHdr * hdr, int + char *ts, *fs; + long i, l, u; + ClStrBuf *fb; ++ int *oldIndexPtr; + + fb = getStrBufPtr(hdr); + ts = (char *) malloc(fb->bUsed); + fs = &fb->buf[0]; + ++ /* Copy indexPtr from the buffer, so we can compute lengths of items in it.*/ ++ oldIndexPtr = (int*) malloc(sizeof(int)*fb->iUsed); ++ memcpy(oldIndexPtr, fb->indexPtr, sizeof(int)*fb->iUsed); ++ + for (u = i = 0; i < fb->iUsed; i++) { + if (i != id - 1) { + char *f = fs + fb->indexPtr[i]; +- l = strlen(f) + 1; ++ l = getBufIndexLen(oldIndexPtr, fb->bUsed, fb->iUsed, i); + fb->indexPtr[i] = u; + memcpy(ts + u, f, l); + u += l; +@@ -474,6 +502,7 @@ replaceClStringN(ClObjectHdr * hdr, int + memcpy(fs, ts, u); + fb->bUsed = u; + free(ts); ++ free(oldIndexPtr); + + i = addClStringN(hdr, str, length); + fb = getStrBufPtr(hdr); +@@ -498,16 +527,20 @@ removeClObject(ClObjectHdr * hdr, int id + // char *tmpstr = NULL; + long i, l, u; + ClStrBuf *fb; ++ int *oldIndexPtr; + + fb = getStrBufPtr(hdr); + ts = (char *) malloc(fb->bUsed); /* tmp string buffer */ + fs = &fb->buf[0]; ++ /* Copy indexPtr from the buffer, so we can compute lengths of items in it.*/ ++ oldIndexPtr = (int*) malloc(sizeof(int)*fb->iUsed); ++ memcpy(oldIndexPtr, fb->indexPtr, sizeof(int)*fb->iUsed); + + for (u = i = 0; i < fb->iUsed; i++) { + if (i != id - 1) { /* loop through and copy over all _other_ properties */ + // fprintf(stderr, "replace: keeping %ld\n", i); + char *f = fs + fb->indexPtr[i]; +- l = fb->indexPtr[i+1] - fb->indexPtr[i]; ++ l = getBufIndexLen(oldIndexPtr, fb->bUsed, fb->iUsed, i); + + /* Bugzilla 74159 - Align the string buffer & null terminate */ + /*if (l % sizeof(long) != 0) { +@@ -535,6 +568,7 @@ removeClObject(ClObjectHdr * hdr, int id + memcpy(fs, ts, u); + fb->bUsed = u; + free(ts); ++ free(oldIndexPtr); + + fb->iUsed--; /* fixup the item count, since we have one fewer elements */ + diff --git a/sblim-sfcb-1.3.16-invalid-read.patch b/sblim-sfcb-1.3.16-invalid-read.patch new file mode 100644 index 0000000..cea715b --- /dev/null +++ b/sblim-sfcb-1.3.16-invalid-read.patch @@ -0,0 +1,76 @@ +Function copyStringBuf() uses sizeof(*fb->indexPtr) as size of elements +in fb->indexPtr, while addClStringN() usess 'sizeof(long)' for the same +elements. Both functions copy indexPtr, but each with different size. + +Therefore, if addClStringN() is called after copyStringBuf(), it may copy more +bytes than copyStringBuf() created -> SIGSEGV (or 'Invalid read of size XYZ' +in Valgrind logs). + +'sizeof(*buf->indexPtr)' should be consistently used in ClStrBuf.indexPtr +and ClArrayBuf.indexPtr. + +diff -up sblim-sfcb-1.3.16/objectImpl.c.invalid-read sblim-sfcb-1.3.16/objectImpl.c +--- sblim-sfcb-1.3.16/objectImpl.c.invalid-read 2013-04-19 14:03:04.920602183 +0200 ++++ sblim-sfcb-1.3.16/objectImpl.c 2013-04-19 14:04:10.229391267 +0200 +@@ -208,7 +208,7 @@ addClStringN(ClObjectHdr * hdr, const ch + buf->bMax = nmax; + buf->bUsed = buf->iUsed = 0; + buf->iMax = 16; +- setStrIndexPtr(buf, malloc(sizeof(long) * 16)); ++ setStrIndexPtr(buf, malloc(sizeof(*buf->indexPtr) * 16)); + hdr->flags |= HDR_Rebuild; + } + +@@ -222,17 +222,17 @@ addClStringN(ClObjectHdr * hdr, const ch + if (!isMallocedStrIndex(buf)) { + void *idx = buf->indexPtr; + buf->iMax = nmax * 2; +- setStrIndexPtr(buf, malloc(buf->iMax * sizeof(long))); +- memcpy(buf->indexPtr, idx, nmax * sizeof(long)); ++ setStrIndexPtr(buf, malloc(buf->iMax * sizeof(*buf->indexPtr))); ++ memcpy(buf->indexPtr, idx, nmax * sizeof(*buf->indexPtr)); + } + else { + buf->iMax = nmax * 2; +- setStrIndexPtr(buf, realloc(buf->indexPtr, buf->iMax * sizeof(long))); ++ setStrIndexPtr(buf, realloc(buf->indexPtr, buf->iMax * sizeof(*buf->indexPtr))); + } + } + else { + buf->iMax = 16; +- setStrIndexPtr(buf, malloc(buf->iMax * sizeof(long))); ++ setStrIndexPtr(buf, malloc(buf->iMax * sizeof(*buf->indexPtr))); + } + hdr->flags |= HDR_Rebuild; + } +@@ -289,7 +289,7 @@ static long addClArray(ClObjectHdr * hdr + buf->bMax = nmax; + buf->bUsed = buf->iUsed = 0; + buf->iMax = 16; +- setArrayIndexPtr(buf, malloc(sizeof(long) * 16)); ++ setArrayIndexPtr(buf, malloc(sizeof(*buf->indexPtr) * 16)); + hdr->flags |= HDR_Rebuild; + } + +@@ -303,17 +303,17 @@ static long addClArray(ClObjectHdr * hdr + if (!isMallocedArrayIndex(buf)) { + void *idx = buf->indexPtr; + buf->iMax = nmax * 2; +- setArrayIndexPtr(buf, malloc(buf->iMax * sizeof(long))); +- memcpy(buf->indexPtr, idx, nmax * sizeof(long)); ++ setArrayIndexPtr(buf, malloc(buf->iMax * sizeof(*buf->indexPtr))); ++ memcpy(buf->indexPtr, idx, nmax * sizeof(*buf->indexPtr)); + } + else { + buf->iMax = nmax * 2; +- setArrayIndexPtr(buf, realloc(buf->indexPtr, buf->iMax * sizeof(long))); ++ setArrayIndexPtr(buf, realloc(buf->indexPtr, buf->iMax * sizeof(*buf->indexPtr))); + } + } + else { + buf->iMax = 16; +- setArrayIndexPtr(buf, malloc(buf->iMax * sizeof(long))); ++ setArrayIndexPtr(buf, malloc(buf->iMax * sizeof(*buf->indexPtr))); + } + hdr->flags |= HDR_Rebuild; + } diff --git a/sblim-sfcb-1.3.16-invalid-read2.patch b/sblim-sfcb-1.3.16-invalid-read2.patch new file mode 100644 index 0000000..406a31b --- /dev/null +++ b/sblim-sfcb-1.3.16-invalid-read2.patch @@ -0,0 +1,26 @@ +The memcpy below tried to copy too much data (it's capacity of the section * 2, +max is doubled few lines above). Let's copy only the used data. + + +Unrelated observation: + +I wonder what ensureClSpace() function does at all. How can this check +be ever true: + + else if (sct->used >= max) { + +'max' is basically sct->max, does that mean the sct->used contains already new +size of the section? + +diff -up sblim-sfcb-1.3.16/objectImpl.c.invalid-read2 sblim-sfcb-1.3.16/objectImpl.c +--- sblim-sfcb-1.3.16/objectImpl.c.invalid-read2 2013-04-19 14:42:52.000000000 +0200 ++++ sblim-sfcb-1.3.16/objectImpl.c 2013-04-19 14:43:23.039536156 +0200 +@@ -168,7 +168,7 @@ static void *ensureClSpace(ClObjectHdr * + void *f,*t; + f=((char*)hdr)+sct->sectionOffset; + t=malloc(max*size); +- memcpy(t,f,max*size); ++ memcpy(t,f,sct->used*size); + sct->max=max; + setSectionPtr(sct, t); + } diff --git a/sblim-sfcb.spec b/sblim-sfcb.spec index d86ae61..1711345 100644 --- a/sblim-sfcb.spec +++ b/sblim-sfcb.spec @@ -8,7 +8,7 @@ Name: sblim-sfcb Summary: Small Footprint CIM Broker URL: http://sblim.wiki.sourceforge.net/ Version: 1.3.16 -Release: 2%{?dist} +Release: 3%{?dist} Group: Applications/System License: EPL Source0: http://downloads.sourceforge.net/sblim/%{name}-%{version}.tar.bz2 @@ -23,6 +23,10 @@ Patch3: sblim-sfcb-1.3.14-missing-includes.patch # Patch4: Fix provider debugging - variable for stopping wait-for-debugger # loop must be volatile Patch4: sblim-sfcb-1.3.15-fix-provider-debugging.patch +# Patch5-7: already upstream, http://sourceforge.net/p/sblim/sfcb-tix/37/ +Patch5: sblim-sfcb-1.3.16-invalid-read.patch +Patch6: sblim-sfcb-1.3.16-invalid-read2.patch +Patch7: sblim-sfcb-1.3.16-embedded-crash.patch Source1: sfcb.service Provides: cim-server Requires: cim-schema @@ -52,6 +56,9 @@ Programming Interface (CMPI). %patch2 -p1 -b .CMGetCharPtr %patch3 -p1 -b .missing-includes %patch4 -p1 -b .fix-provider-debugging +%patch5 -p1 -b .invalid-read +%patch6 -p1 -b .invalid-read2 +%patch7 -p1 -b .embedded-crash %build %configure --enable-debug --enable-uds --enable-ssl --enable-pam --enable-ipv6 CFLAGS="$CFLAGS -D_GNU_SOURCE -fPIE -DPIE" LDFLAGS="$LDFLAGS -Wl,-z,now -pie" @@ -106,6 +113,10 @@ fi; %files -f _pkg_list %changelog +* Mon May 20 2013 Vitezslav Crhonek - 1.3.16-3 +- Fix indCIMXmlHandler crash in IndCIMXMLHandlerInvokeMethod with Embedded Instances + Resolves: #957747 + * Tue Jan 29 2013 Vitezslav Crhonek - 1.3.16-2 - Fix URL in the spec file - Remove unused devel part from the spec file