Comment 15 for bug 1903817

Revision history for this message
bugproxy (bugproxy) wrote : Comment bridged from LTC Bugzilla

------- Comment From <email address hidden> 2020-11-20 10:37 EDT-------
The problem boils down to a buffer overflow. Consider the following code snippet from file pair_dist.c

int iChunkStarts[iNumberOfThreads];
int iChunkEnds[iNumberOfThreads];

// ...

for(iChunk = 0; iChunk <= iNumberOfThreads; iChunk++)
{
iChunkEnd = iChunkStart;
if (iChunk == iNumberOfThreads - 1){
iChunkStart = 0;
}
else if (iend == jend){
iChunkStart = iend - ((double)(iend - istart) * sqrt(((double)iChunk + 1.0)/(double)iNumberOfThreads));
}
else {
iChunkStart = iend - (iend - istart) * (iChunk + 1) / (double)(iNumberOfThreads);
}
iChunkStarts[iChunk] = iChunkStart;
iChunkEnds[iChunk] = iChunkEnd;
/*printf("%s:%d: C=%d, ie=%d, is=%d, je=%d, js=%d, Cstart=%d, Cend=%d, diff=%d\n",
__FILE__, __LINE__, iChunk, iend, istart, jend, jstart, iChunkStart, iChunkEnd, iChunkEnd-iChunkStart);*/
}

if (PAIRDIST_KTUPLE == pairdist_type) {

Log(&rLog, LOG_INFO, "Calculating pairwise ktuple-distances...");

NewProgress(&prProgress, LogGetFP(&rLog, LOG_INFO),
"Ktuple-distance calculation progress", bPrintCR);
#ifdef HAVE_OPENMP
#pragma omp parallel for private(iChunk) schedule(dynamic)
#endif
for(iChunk = 0; iChunk < iNumberOfThreads; iChunk++)
{
KTuplePairDist((*distmat), mseq, iChunkStarts[iChunk],
iChunkEnds[iChunk], jstart, jend, NULL, prProgress,
&ulStepNo, ulTotalStepNo);
}

The first loop writes behind VLAs iChunkStarts and iChunkEnds.
The closure for the upcoming OMP loop

.omp_data_o.21.iChunkEnds.3 = iChunkEnds.3_72;
.omp_data_o.21.iChunkStarts.1 = iChunkStarts.1_70;
.omp_data_o.21.ulTotalStepNo = ulTotalStepNo_81;
.omp_data_o.21.jend = jend_77(D);
.omp_data_o.21.jstart = jstart_91(D);
.omp_data_o.21.mseq = mseq_93(D);
.omp_data_o.21.distmat = distmat_78(D);
.omp_data_o.21.ulStepNo = &ulStepNo;
.omp_data_o.21.prProgress = &prProgress;
__builtin_GOMP_parallel (PairDistances._omp_fn.0, &.omp_data_o.21, 0, 0);

is placed adjacent to those VLAs iChunkStarts and iChunkEnds on S/390. The overflow results in overwriting the high part of

.omp_data_o.21.iChunkEnds.3

which finally results in a SEGVAULT once the corresponding OMP function is executed and the pointer is dereferenced.

Long story short:

diff --git a/src/clustal/pair_dist.c.orig b/src/clustal/pair_dist.c
index e6dbdc3..bb79e61 100644
--- a/src/clustal/pair_dist.c.orig
+++ b/src/clustal/pair_dist.c
@@ -321,7 +321,7 @@ PairDistances(symmatrix_t **distmat, mseq_t *mseq, int pairdist_type, bool bPerc

/* FIXME: can get rid of iChunkStart, iChunkEnd now that we're using the arrays */
iChunkStart = iend;
- for(iChunk = 0; iChunk <= iNumberOfThreads; iChunk++)
+ for(iChunk = 0; iChunk < iNumberOfThreads; iChunk++)
{
iChunkEnd = iChunkStart;
if (iChunk == iNumberOfThreads - 1){

should fix this.