#80 Possible segmentation faults problem in agcep

[Ryuichi Yamamoto]

I found that there is a possible segfaults problem in the function agcep, which is defined in _agcep.c. The problem will happen when agcep are called multiple times with different stage values.

A quick illustration of the problem:

line #101

eg[m] = d[stage * m - 1];

When agcep are called multiple times with difference stage values, this line may cause segfault, since memory will not be re-allocated when the larger value is specified to stage (e.g, stage = 1 for the first call and stage = 2 for the second call).

My fix:

line #89 ~ 96

-      size = m;
+      size = m * stage;
    }
-   if (m > size) {
+   if (m * stage > size) {
       free(eg);
       eg = dgetmem(2 * (m + 1) + m * stage);
       ep = eg + m + 1;
       d = ep + m + 1;
-      size = m;
+      size = m * stage;

I think this will fix the problem, though the function agcep is never used in the agcep command which algorithm is written in agcep.c as a separate (duplicate?) program and the problem doesn't appear for most users.

I hope you can take a look at the code and my fix as well.

[Ryuichi Yamamoto]

I found the same issue in _amcep.c.

Line #86 ~ 100

+   int expected_size = 3 * (m + 1) + 3 * (pd + 1) + pd * (m + 2);
    if (bb == NULL) {
-      bb = dgetmem(3 * (m + 1) + 3 * (pd + 1) + pd * (m + 2));
+      bb = dgetmem(expected_size);
       e = bb + m + 1;
       ep = e + m + 1;
       d = ep + m + 1;
-      size = m;
+      size = expected_size;
    }
-   if (m > size) {
+   if (expected_size > size) {
       free(bb);
-      bb = dgetmem(3 * (m + 1) + 3 * (pd + 1) + pd * (m + 2));
+      bb = dgetmem(expected_size);
       e = bb + m + 1;
       ep = e + m + 1;
       d = ep + m + 1;
-      size = m;
+      size = expected_size;

This should fix the problem.

[Ryuichi Yamamoto]

I also found the same problem in _acep.c (perhaps in others, I guess). I think this would be a serious problem for embedded programs that link with SPTK internally.

[Keiichiro Oura]

Hi,

Thank you for using SPTK and your kindly report.
I'll check them.

Keiichiro

[Shikano Motoki]

Hi,

Thank you for your report.
It was fixed.
http://sp-tk.cvs.sourceforge.net/viewvc/sp-tk/SPTK/src/bin/agcep/_agcep.c?r1=1.18&r2=1.19

[Ryuichi Yamamoto]

Hi, this doesn't seem to be fixed entirely. I've quickly looked into the SPTK-3.10 and there was still this kind of bug, at least in _amcep.c, _acep.c and _fftcep.c. Please consider the following pathces:

_amcep.c

--- bin/amcep/_amcep.c  2016-12-25 14:01:31.000000000 +0900
+++ bin/amcep/_amcep_patch.c    2017-04-22 20:51:46.000000000 +0900
@@ -83,20 +83,21 @@
    static int size;
    double mu, tx;


+   int expected_size = 3 * (m + 1) + 3 * (pd + 1) + pd * (m + 2);
    if (bb == NULL) {
-      bb = dgetmem(3 * (m + 1) + 3 * (pd + 1) + pd * (m + 2));
+      bb = dgetmem(expected_size);
       e = bb + m + 1;
       ep = e + m + 1;
       d = ep + m + 1;
-      size = m;
+      size = expected_size;
    }
-   if (m > size) {
+   if (expected_size > size) {
       free(bb);
-      bb = dgetmem(3 * (m + 1) + 3 * (pd + 1) + pd * (m + 2));
+      bb = dgetmem(expected_size);
       e = bb + m + 1;
       ep = e + m + 1;
       d = ep + m + 1;
-      size = m;
+      size = expected_size;
    }

_acep.c

--- bin/acep/_acep.c    2016-12-25 14:01:31.000000000 +0900
+++ bin/acep/_acep_patch.c      2017-04-22 21:01:26.000000000 +0900
@@ -80,21 +80,22 @@
    static int size;
    double mu, tx;

+   int expected_size = m + m + m + 3 + (m + 1) * pd * 2;
    if (cc == NULL) {
-      cc = dgetmem(m + m + m + 3 + (m + 1) * pd * 2);
+      cc = dgetmem(expected_size);
       e = cc + m + 1;
       ep = e + m + 1;
       d = ep + m + 1;
-      size = m;
+      size = expected_size;
    }

-   if (m > size) {
+   if (expected_size > size) {
       free(cc);
-      cc = dgetmem(m + m + m + 3 + (m + 1) * pd * 2);
+      cc = dgetmem(expected_size);
       e = cc + m + 1;
       ep = e + m + 1;
       d = ep + m + 1;
-      size = m;
+      size = expected_size;
    }

_fftcep.c

--- bin/fftcep/_fftcep.c        2017-04-22 20:30:29.000000000 +0900
+++ bin/fftcep/_fftcep_patch.c  2017-04-22 20:51:58.000000000 +0900
@@ -79,6 +79,7 @@
    if (x == NULL) {
       x = dgetmem(flng + flng);
       y = x + flng;
+      size = flng;
    }
    if (flng > size) {
       free(x);

[Keiichiro Oura]

Thank you very much. It was fixed.