ACK: [PATCH] lib: fwts_klog: fix vector size and handle errors from pcre_exec (LP: #1401184)

[Alex Hung]

On 12/11/2014 01:23 AM, Colin King wrote:
> From: Colin Ian King <colin.king at 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 at 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 at canonical.com>