From 8ff526111bfc6ca8d2384c4d4962b8e8ff3621f9 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Fri, 4 Jun 2010 13:20:17 -0700 Subject: LINEAR_RGB_InitSCCData: When malloc fails, don't try to free unallocated bits One of the malloc failure checks had a goto to the wrong spot in the list of cleanup free() calls to unwind at the end, and was freeing bits that hadn't been initialized/allocated yet, since they would be stored in the struct that just failed to be allocated. Error: Null pointer dereference (CWE 476) Read from pointer that could be constant 'NULL' at line 805 of /export/alanc/X.Org/sx86/lib/libX11/nx-X11/lib/X11/LRGB.c in function 'LINEAR_RGB_InitSCCData'. Pointer checked against constant 'NULL' at line 754 but does not protect the dereference. [ This bug was found by the Parfait bug checking tool. For more information see http://research.sun.com/projects/parfait ] Signed-off-by: Alan Coopersmith Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/LRGB.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'nx-X11/lib/X11/LRGB.c') diff --git a/nx-X11/lib/X11/LRGB.c b/nx-X11/lib/X11/LRGB.c index dbdddb457..bc1bfd92b 100644 --- a/nx-X11/lib/X11/LRGB.c +++ b/nx-X11/lib/X11/LRGB.c @@ -753,7 +753,7 @@ LINEAR_RGB_InitSCCData( /* Blue Intensity Table */ if (!(pScreenData->pBlueTbl = (IntensityTbl *) Xcalloc (1, sizeof(IntensityTbl)))) { - goto FreeBlueTblElements; + goto FreeGreenTblElements; } if (_XcmsGetTableType1(pScreenData->pBlueTbl, format_return, &pChar, &nitems) == XcmsFailure) { -- cgit v1.2.3 From 8f5a56474671bf3dd4909aa73fec1062fb5a14d8 Mon Sep 17 00:00:00 2001 From: Ander Conselvan de Oliveira Date: Mon, 31 Jan 2011 14:02:08 +0200 Subject: xcms/LRGB: Fix potential resource leak. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit property_return was not free'd if the allocation of pRedTbl failed. Reviewed-by: Erkki Seppälä Signed-off-by: Ander Conselvan de Oliveira Signed-off-by: Alan Coopersmith Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/LRGB.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'nx-X11/lib/X11/LRGB.c') diff --git a/nx-X11/lib/X11/LRGB.c b/nx-X11/lib/X11/LRGB.c index bc1bfd92b..534a7307e 100644 --- a/nx-X11/lib/X11/LRGB.c +++ b/nx-X11/lib/X11/LRGB.c @@ -687,6 +687,7 @@ LINEAR_RGB_InitSCCData( /* Red Intensity Table */ if (!(pScreenData->pRedTbl = (IntensityTbl *) Xcalloc (1, sizeof(IntensityTbl)))) { + XFree ((char * ) property_return); goto FreeSCCData; } if (_XcmsGetTableType0(pScreenData->pRedTbl, format_return, &pChar, @@ -724,6 +725,7 @@ LINEAR_RGB_InitSCCData( /* Red Intensity Table */ if (!(pScreenData->pRedTbl = (IntensityTbl *) Xcalloc (1, sizeof(IntensityTbl)))) { + XFree ((char * ) property_return); goto FreeSCCData; } if (_XcmsGetTableType1(pScreenData->pRedTbl, format_return, &pChar, -- cgit v1.2.3 From 7482afcaa71df7ed60909c3c3f995f4723190d4e Mon Sep 17 00:00:00 2001 From: Ander Conselvan de Oliveira Date: Tue, 1 Feb 2011 11:07:25 +0200 Subject: xcms/LRGB: Add a label for freeing property_return. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The rest of the code uses goto's to free memory allocated later and prevent memory leaks, but there were several paths were property_return was free'd just before a goto. Reviewed-by: Alan Coopersmith Signed-off-by: Ander Conselvan de Oliveira Signed-off-by: Erkki Seppälä Signed-off-by: Alan Coopersmith Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/LRGB.c | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) (limited to 'nx-X11/lib/X11/LRGB.c') diff --git a/nx-X11/lib/X11/LRGB.c b/nx-X11/lib/X11/LRGB.c index 534a7307e..f2027ea3f 100644 --- a/nx-X11/lib/X11/LRGB.c +++ b/nx-X11/lib/X11/LRGB.c @@ -595,8 +595,7 @@ LINEAR_RGB_InitSCCData( * intensity2 */ if (nitems < 9) { - Xfree ((char *)property_return); - goto FreeSCCData; + goto Free_property_return; } count = 3; break; @@ -612,8 +611,7 @@ LINEAR_RGB_InitSCCData( * intensity2 */ if (nitems < 7) { - Xfree ((char *)property_return); - goto FreeSCCData; + goto Free_property_return; } count = 1; break; @@ -628,14 +626,12 @@ LINEAR_RGB_InitSCCData( * intensity2 */ if (nitems < 6) { - Xfree ((char *)property_return); - goto FreeSCCData; + goto Free_property_return; } count = 0; break; default: - Xfree ((char *)property_return); - goto FreeSCCData; + goto Free_property_return; } /* @@ -687,8 +683,7 @@ LINEAR_RGB_InitSCCData( /* Red Intensity Table */ if (!(pScreenData->pRedTbl = (IntensityTbl *) Xcalloc (1, sizeof(IntensityTbl)))) { - XFree ((char * ) property_return); - goto FreeSCCData; + goto Free_property_return; } if (_XcmsGetTableType0(pScreenData->pRedTbl, format_return, &pChar, &nitems) == XcmsFailure) { @@ -725,8 +720,7 @@ LINEAR_RGB_InitSCCData( /* Red Intensity Table */ if (!(pScreenData->pRedTbl = (IntensityTbl *) Xcalloc (1, sizeof(IntensityTbl)))) { - XFree ((char * ) property_return); - goto FreeSCCData; + goto Free_property_return; } if (_XcmsGetTableType1(pScreenData->pRedTbl, format_return, &pChar, &nitems) == XcmsFailure) { @@ -763,8 +757,7 @@ LINEAR_RGB_InitSCCData( } } } else { - Xfree ((char *)property_return); - goto FreeSCCData; + goto Free_property_return; } #ifdef ALLDEBUG @@ -821,6 +814,9 @@ FreeRedTblElements: FreeRedTbl: Xfree((char *)pScreenData->pRedTbl); +Free_property_return: + Xfree ((char *)property_return); + FreeSCCData: Xfree((char *)pScreenData); pPerScrnInfo->state = XcmsInitNone; -- cgit v1.2.3 From ae4e7538e078aff5db75af7b5fafdbb5658cbbb6 Mon Sep 17 00:00:00 2001 From: Ander Conselvan de Oliveira Date: Mon, 31 Jan 2011 14:02:07 +0200 Subject: xcms/LRGB: don't double-free property_return MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit property_return was free'd before and in the case the conditional is true, the call to XcmsGetProperty failed which means that property_return wasn't set so there is no need to free it again. Double free of pointer "property_return" in call to "free" Reviewed-by: Alan Coopersmith Reviewed-by: Erkki Seppälä Signed-off-by: Ander Conselvan de Oliveira Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/LRGB.c | 1 - 1 file changed, 1 deletion(-) (limited to 'nx-X11/lib/X11/LRGB.c') diff --git a/nx-X11/lib/X11/LRGB.c b/nx-X11/lib/X11/LRGB.c index f2027ea3f..08c7105ab 100644 --- a/nx-X11/lib/X11/LRGB.c +++ b/nx-X11/lib/X11/LRGB.c @@ -573,7 +573,6 @@ LINEAR_RGB_InitSCCData( if (CorrectAtom == None || !_XcmsGetProperty (dpy, RootWindow(dpy, screenNumber), CorrectAtom, &format_return, &nitems, &nbytes_return, &property_return)) { - Xfree ((char *)property_return); goto FreeSCCData; } -- cgit v1.2.3 From fefb909a07d633986e9850dbe3baa91cd1531920 Mon Sep 17 00:00:00 2001 From: Ander Conselvan de Oliveira Date: Wed, 30 Mar 2011 15:19:28 +0300 Subject: xcms: Fix error on LINEAR_RGB_InitSCCData error path. Due to what looks like a copy & paste error, pScreenData->pBlueTbl would be accessed after being free'd. Signed-off-by: Ander Conselvan de Oliveira Reviewed-by: Alan Coopersmith Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/LRGB.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'nx-X11/lib/X11/LRGB.c') diff --git a/nx-X11/lib/X11/LRGB.c b/nx-X11/lib/X11/LRGB.c index 08c7105ab..82f5de757 100644 --- a/nx-X11/lib/X11/LRGB.c +++ b/nx-X11/lib/X11/LRGB.c @@ -802,7 +802,7 @@ FreeBlueTbl: Xfree((char *)pScreenData->pBlueTbl); FreeGreenTblElements: - Xfree((char *)pScreenData->pBlueTbl->pBase); + Xfree((char *)pScreenData->pGreenTbl->pBase); FreeGreenTbl: Xfree((char *)pScreenData->pGreenTbl); -- cgit v1.2.3 From f50e651e8acd28f3798d31038b6b9804b8309a89 Mon Sep 17 00:00:00 2001 From: Ander Conselvan de Oliveira Date: Wed, 30 Mar 2011 15:06:10 +0300 Subject: xcms: Fix memory leaks on LINEAR_RGB_InitSCCData error path. pScreenData is replaced when building per visual intensity tables. If malloc failed the old value of pScreenData (stored also in pScreenDefaultData) was being leaked. Also, property_return wasn't free'd in that case. Signed-off-by: Ander Conselvan de Oliveira Reviewed-by: Alan Coopersmith Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/LRGB.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'nx-X11/lib/X11/LRGB.c') diff --git a/nx-X11/lib/X11/LRGB.c b/nx-X11/lib/X11/LRGB.c index 82f5de757..ef1d7fb27 100644 --- a/nx-X11/lib/X11/LRGB.c +++ b/nx-X11/lib/X11/LRGB.c @@ -653,7 +653,7 @@ LINEAR_RGB_InitSCCData( */ if (!(pScreenData = (LINEAR_RGB_SCCData *) Xcalloc (1, sizeof(LINEAR_RGB_SCCData)))) { - return(XcmsFailure); + goto Free_property_return; } /* copy matrices */ memcpy((char *)pScreenData, (char *)pScreenDefaultData, @@ -663,7 +663,7 @@ LINEAR_RGB_InitSCCData( if (!(pNewMap = (XcmsIntensityMap *) Xcalloc (1, sizeof(XcmsIntensityMap)))) { Xfree((char *)pScreenData); - return(XcmsFailure); + goto Free_property_return; } pNewMap->visualID = visualID; pNewMap->screenData = (XPointer)pScreenData; @@ -817,7 +817,7 @@ Free_property_return: Xfree ((char *)property_return); FreeSCCData: - Xfree((char *)pScreenData); + Xfree((char *)pScreenDefaultData); pPerScrnInfo->state = XcmsInitNone; return(XcmsFailure); } -- cgit v1.2.3 From 26256188b498e122e21ce0e05521dfc58f8b60a5 Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sun, 11 Aug 2013 00:07:33 -0700 Subject: Remove unnecessary casts of pointers to (char *) in calls to Xfree() Left one cast behind that is necessary to change from const char * to char * in nx-X11/lib/X11/lcCharSet.c. Signed-off-by: Alan Coopersmith Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/LRGB.c | 36 ++++++++++++++++++------------------ 1 file changed, 18 insertions(+), 18 deletions(-) (limited to 'nx-X11/lib/X11/LRGB.c') diff --git a/nx-X11/lib/X11/LRGB.c b/nx-X11/lib/X11/LRGB.c index ef1d7fb27..f8e0d66fb 100644 --- a/nx-X11/lib/X11/LRGB.c +++ b/nx-X11/lib/X11/LRGB.c @@ -511,7 +511,7 @@ LINEAR_RGB_InitSCCData( *pValue++ = (long)_XcmsGetElement(format_return, &pChar, &nitems) / (XcmsFloat)XDCCC_NUMBER; } - Xfree ((char *)property_return); + Xfree (property_return); pPerScrnInfo->screenWhitePt.spec.CIEXYZ.X = pScreenData->RGBtoXYZmatrix[0][0] + pScreenData->RGBtoXYZmatrix[0][1] + @@ -662,7 +662,7 @@ LINEAR_RGB_InitSCCData( /* Create, initialize, and add map */ if (!(pNewMap = (XcmsIntensityMap *) Xcalloc (1, sizeof(XcmsIntensityMap)))) { - Xfree((char *)pScreenData); + Xfree(pScreenData); goto Free_property_return; } pNewMap->visualID = visualID; @@ -782,7 +782,7 @@ LINEAR_RGB_InitSCCData( #endif /* ALLDEBUG */ } - Xfree ((char *)property_return); + Xfree (property_return); /* Free the old memory and use the new structure created. */ LINEAR_RGB_FreeSCCData(pPerScrnInfo->screenData); @@ -796,28 +796,28 @@ LINEAR_RGB_InitSCCData( return(XcmsSuccess); FreeBlueTblElements: - Xfree((char *)pScreenData->pBlueTbl->pBase); + Xfree(pScreenData->pBlueTbl->pBase); FreeBlueTbl: - Xfree((char *)pScreenData->pBlueTbl); + Xfree(pScreenData->pBlueTbl); FreeGreenTblElements: - Xfree((char *)pScreenData->pGreenTbl->pBase); + Xfree(pScreenData->pGreenTbl->pBase); FreeGreenTbl: - Xfree((char *)pScreenData->pGreenTbl); + Xfree(pScreenData->pGreenTbl); FreeRedTblElements: - Xfree((char *)pScreenData->pRedTbl->pBase); + Xfree(pScreenData->pRedTbl->pBase); FreeRedTbl: - Xfree((char *)pScreenData->pRedTbl); + Xfree(pScreenData->pRedTbl); Free_property_return: - Xfree ((char *)property_return); + Xfree (property_return); FreeSCCData: - Xfree((char *)pScreenDefaultData); + Xfree(pScreenDefaultData); pPerScrnInfo->state = XcmsInitNone; return(XcmsFailure); } @@ -849,30 +849,30 @@ LINEAR_RGB_FreeSCCData( if (pScreenData->pRedTbl->pBase != pScreenData->pGreenTbl->pBase) { if (pScreenData->pGreenTbl->pBase) { - Xfree ((char *)pScreenData->pGreenTbl->pBase); + Xfree (pScreenData->pGreenTbl->pBase); } } if (pScreenData->pGreenTbl != pScreenData->pRedTbl) { - Xfree ((char *)pScreenData->pGreenTbl); + Xfree (pScreenData->pGreenTbl); } } if (pScreenData->pBlueTbl) { if (pScreenData->pRedTbl->pBase != pScreenData->pBlueTbl->pBase) { if (pScreenData->pBlueTbl->pBase) { - Xfree ((char *)pScreenData->pBlueTbl->pBase); + Xfree (pScreenData->pBlueTbl->pBase); } } if (pScreenData->pBlueTbl != pScreenData->pRedTbl) { - Xfree ((char *)pScreenData->pBlueTbl); + Xfree (pScreenData->pBlueTbl); } } if (pScreenData->pRedTbl->pBase) { - Xfree ((char *)pScreenData->pRedTbl->pBase); + Xfree (pScreenData->pRedTbl->pBase); } - Xfree ((char *)pScreenData->pRedTbl); + Xfree (pScreenData->pRedTbl); } - Xfree ((char *)pScreenData); + Xfree (pScreenData); } } -- cgit v1.2.3 From 8d8d5cfd8c4f3d25c85d6ed60298f3aa917651eb Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 6 Feb 2016 13:32:44 -0800 Subject: xcms: use unsigned indexes when looping through unsigned values MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Clears many gcc warnings of the form: uvY.c: In function ‘XcmsCIEuvYToCIEXYZ’: uvY.c:263:19: warning: comparison between signed and unsigned integer expressions [-Wsign-compare] for (i = 0; i < nColors; i++, pColor++) { ^ Signed-off-by: Alan Coopersmith Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/LRGB.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'nx-X11/lib/X11/LRGB.c') diff --git a/nx-X11/lib/X11/LRGB.c b/nx-X11/lib/X11/LRGB.c index f8e0d66fb..55e3aaade 100644 --- a/nx-X11/lib/X11/LRGB.c +++ b/nx-X11/lib/X11/LRGB.c @@ -966,7 +966,7 @@ _XcmsGetTableType1( * */ { - int count; + unsigned int count; unsigned int max_index; IntensityRec *pIRec; -- cgit v1.2.3 From 68e3ee67c07282c11cd144fbdb767b41a6dacc4e Mon Sep 17 00:00:00 2001 From: Alan Coopersmith Date: Sat, 6 Feb 2016 14:18:32 -0800 Subject: xcms: use size_t for pointer offsets passed to strncmp instead of converting to int and back Fixes clang warnings of the form: HVC.c:190:43: warning: implicit conversion changes signedness: 'int' to 'unsigned long' [-Wsign-conversion] if (strncmp(spec, _XcmsTekHVC_prefix, n) != 0) { ~~~~~~~ Signed-off-by: Alan Coopersmith Backported-to-NX-by: Ulrich Sibiller --- nx-X11/lib/X11/LRGB.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'nx-X11/lib/X11/LRGB.c') diff --git a/nx-X11/lib/X11/LRGB.c b/nx-X11/lib/X11/LRGB.c index 55e3aaade..4bfa3cfb1 100644 --- a/nx-X11/lib/X11/LRGB.c +++ b/nx-X11/lib/X11/LRGB.c @@ -1405,13 +1405,13 @@ XcmsLRGB_RGBi_ParseString( * 0 if failed, non-zero otherwise. */ { - int n; + size_t n; char *pchar; if ((pchar = strchr(spec, ':')) == NULL) { return(XcmsFailure); } - n = (int)(pchar - spec); + n = (size_t)(pchar - spec); /* * Check for proper prefix. -- cgit v1.2.3