| Message ID | 1418232192-20357-1-git-send-email-colin.king@canonical.com |
|---|---|
| State | Accepted |
| Headers | show |
On 12/11/2014 01:23 AM, Colin King wrote: > From: Colin Ian King <colin.king@canonical.com> > > On ARM32 platforms I detected a stack smashing bug where pcre_exec scribbles > over the stack because the vector being passed to pcre_exec is not a multiple > of 3 in size (as the API requires). > > Make the vector overly large multiple of 3 to avoid any future stack smashing > and also handle errors from pcre_exec in case the regular expressions get > broken or miscompiled because of other errors. > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/lib/src/fwts_klog.c | 45 ++++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 42 insertions(+), 3 deletions(-) > > diff --git a/src/lib/src/fwts_klog.c b/src/lib/src/fwts_klog.c > index 8b4a9ed..b38a02e 100644 > --- a/src/lib/src/fwts_klog.c > +++ b/src/lib/src/fwts_klog.c > @@ -258,6 +258,8 @@ static char *fwts_klog_unique_label(const char *str) > return buffer; > } > > +#define VECTOR_SIZE (3) /* Must be a multiple of 3 */ > + > void fwts_klog_scan_patterns(fwts_framework *fw, > char *line, > int repeated, > @@ -266,7 +268,7 @@ void fwts_klog_scan_patterns(fwts_framework *fw, > int *errors) > { > fwts_klog_pattern *pattern = (fwts_klog_pattern *)private; > - int vector[1]; > + int vector[VECTOR_SIZE]; > static char *advice = > "This is a bug picked up by the kernel, but as yet, the " > "firmware test suite has no diagnostic advice for this particular problem."; > @@ -277,8 +279,45 @@ void fwts_klog_scan_patterns(fwts_framework *fw, > bool matched = false; > switch (pattern->compare_mode) { > case FWTS_COMPARE_REGEX: > - if (pattern->re) > - matched = (pcre_exec(pattern->re, pattern->extra, line, strlen(line), 0, 0, vector, 1) == 0); > + if (pattern->re) { > + int ret = pcre_exec(pattern->re, pattern->extra, line, strlen(line), 0, 0, vector, VECTOR_SIZE); > + if (ret < 0) { > + char *errmsg = NULL; > + /* > + * We should handle -ve conditions other > + * than PCRE_ERROR_NOMATCH as pcre internal > + * errors.. > + */ > + switch (ret) { > + case PCRE_ERROR_NOMATCH: > + break; > + case PCRE_ERROR_NULL: > + errmsg = "internal NULL error"; > + break; > + case PCRE_ERROR_BADOPTION: > + errmsg = "invalid option passed to pcre_exec()"; > + break; > + case PCRE_ERROR_BADMAGIC: > + errmsg = "bad magic number, (corrupt regular expression)"; > + break; > + case PCRE_ERROR_UNKNOWN_NODE: > + errmsg = "compiled regular expression broken"; > + break; > + case PCRE_ERROR_NOMEMORY: > + errmsg = "out of memory"; > + break; > + default: > + errmsg = "unknown error"; > + break; > + } > + if (errmsg) > + fwts_log_info(fw, "regular expression engine error: %s.", errmsg); > + } > + else { > + /* A successful regular expression match! */ > + matched = true; > + } > + } > break; > case FWTS_COMPARE_STRING: > default: > Acked-by: Alex Hung <alex.hung@canonical.com>
On Thu, Dec 11, 2014 at 1:23 AM, Colin King <colin.king@canonical.com> wrote: > From: Colin Ian King <colin.king@canonical.com> > > On ARM32 platforms I detected a stack smashing bug where pcre_exec scribbles > over the stack because the vector being passed to pcre_exec is not a multiple > of 3 in size (as the API requires). > > Make the vector overly large multiple of 3 to avoid any future stack smashing > and also handle errors from pcre_exec in case the regular expressions get > broken or miscompiled because of other errors. > > Signed-off-by: Colin Ian King <colin.king@canonical.com> > --- > src/lib/src/fwts_klog.c | 45 ++++++++++++++++++++++++++++++++++++++++++--- > 1 file changed, 42 insertions(+), 3 deletions(-) > > diff --git a/src/lib/src/fwts_klog.c b/src/lib/src/fwts_klog.c > index 8b4a9ed..b38a02e 100644 > --- a/src/lib/src/fwts_klog.c > +++ b/src/lib/src/fwts_klog.c > @@ -258,6 +258,8 @@ static char *fwts_klog_unique_label(const char *str) > return buffer; > } > > +#define VECTOR_SIZE (3) /* Must be a multiple of 3 */ > + > void fwts_klog_scan_patterns(fwts_framework *fw, > char *line, > int repeated, > @@ -266,7 +268,7 @@ void fwts_klog_scan_patterns(fwts_framework *fw, > int *errors) > { > fwts_klog_pattern *pattern = (fwts_klog_pattern *)private; > - int vector[1]; > + int vector[VECTOR_SIZE]; > static char *advice = > "This is a bug picked up by the kernel, but as yet, the " > "firmware test suite has no diagnostic advice for this particular problem."; > @@ -277,8 +279,45 @@ void fwts_klog_scan_patterns(fwts_framework *fw, > bool matched = false; > switch (pattern->compare_mode) { > case FWTS_COMPARE_REGEX: > - if (pattern->re) > - matched = (pcre_exec(pattern->re, pattern->extra, line, strlen(line), 0, 0, vector, 1) == 0); > + if (pattern->re) { > + int ret = pcre_exec(pattern->re, pattern->extra, line, strlen(line), 0, 0, vector, VECTOR_SIZE); > + if (ret < 0) { > + char *errmsg = NULL; > + /* > + * We should handle -ve conditions other > + * than PCRE_ERROR_NOMATCH as pcre internal > + * errors.. > + */ > + switch (ret) { > + case PCRE_ERROR_NOMATCH: > + break; > + case PCRE_ERROR_NULL: > + errmsg = "internal NULL error"; > + break; > + case PCRE_ERROR_BADOPTION: > + errmsg = "invalid option passed to pcre_exec()"; > + break; > + case PCRE_ERROR_BADMAGIC: > + errmsg = "bad magic number, (corrupt regular expression)"; > + break; > + case PCRE_ERROR_UNKNOWN_NODE: > + errmsg = "compiled regular expression broken"; > + break; > + case PCRE_ERROR_NOMEMORY: > + errmsg = "out of memory"; > + break; > + default: > + errmsg = "unknown error"; > + break; > + } > + if (errmsg) > + fwts_log_info(fw, "regular expression engine error: %s.", errmsg); > + } > + else { > + /* A successful regular expression match! */ > + matched = true; > + } > + } > break; > case FWTS_COMPARE_STRING: > default: > -- > 2.1.3 > > definitely, Acked-by: Keng-Yu Lin <kengyu@canonical.com>
diff --git a/src/lib/src/fwts_klog.c b/src/lib/src/fwts_klog.c index 8b4a9ed..b38a02e 100644 --- a/src/lib/src/fwts_klog.c +++ b/src/lib/src/fwts_klog.c @@ -258,6 +258,8 @@ static char *fwts_klog_unique_label(const char *str) return buffer; } +#define VECTOR_SIZE (3) /* Must be a multiple of 3 */ + void fwts_klog_scan_patterns(fwts_framework *fw, char *line, int repeated, @@ -266,7 +268,7 @@ void fwts_klog_scan_patterns(fwts_framework *fw, int *errors) { fwts_klog_pattern *pattern = (fwts_klog_pattern *)private; - int vector[1]; + int vector[VECTOR_SIZE]; static char *advice = "This is a bug picked up by the kernel, but as yet, the " "firmware test suite has no diagnostic advice for this particular problem."; @@ -277,8 +279,45 @@ void fwts_klog_scan_patterns(fwts_framework *fw, bool matched = false; switch (pattern->compare_mode) { case FWTS_COMPARE_REGEX: - if (pattern->re) - matched = (pcre_exec(pattern->re, pattern->extra, line, strlen(line), 0, 0, vector, 1) == 0); + if (pattern->re) { + int ret = pcre_exec(pattern->re, pattern->extra, line, strlen(line), 0, 0, vector, VECTOR_SIZE); + if (ret < 0) { + char *errmsg = NULL; + /* + * We should handle -ve conditions other + * than PCRE_ERROR_NOMATCH as pcre internal + * errors.. + */ + switch (ret) { + case PCRE_ERROR_NOMATCH: + break; + case PCRE_ERROR_NULL: + errmsg = "internal NULL error"; + break; + case PCRE_ERROR_BADOPTION: + errmsg = "invalid option passed to pcre_exec()"; + break; + case PCRE_ERROR_BADMAGIC: + errmsg = "bad magic number, (corrupt regular expression)"; + break; + case PCRE_ERROR_UNKNOWN_NODE: + errmsg = "compiled regular expression broken"; + break; + case PCRE_ERROR_NOMEMORY: + errmsg = "out of memory"; + break; + default: + errmsg = "unknown error"; + break; + } + if (errmsg) + fwts_log_info(fw, "regular expression engine error: %s.", errmsg); + } + else { + /* A successful regular expression match! */ + matched = true; + } + } break; case FWTS_COMPARE_STRING: default: