[PATCH 1/3] libsepol, libsemanage, libselinux: Fix fallthrough warnings from gcc 7

[PATCH 1/3] libsepol, libsemanage, libselinux: Fix fallthrough warnings from gcc 7

[Stephen Smalley]

https://developers.redhat.com/blog/2017/03/10/wimplicit-fallthrough-in-gcc-7/

Fixes the following warnings by annotating with a /* FALLTHRU */ comment.
Unfortunately, the __attribute__ ((fallthrough)); approach does not appear
to work with older compilers.

../cil/src/cil_parser.c: In function ‘cil_parser’:
../cil/src/cil_parser.c:253:14: warning: this statement may fall through [-Wimplicit-fallthrough=]
    tok.value = tok.value+1;
    ~~~~~~~~~~^~~~~~~~~~~~~
../cil/src/cil_parser.c:254:3: note: here
   case SYMBOL:
   ^~~~
../cil/src/cil_parser.c:275:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
    if (tok.type != END_OF_FILE) {
       ^
../cil/src/cil_parser.c:279:3: note: here
   case END_OF_FILE:
   ^~~~

../cil/src/cil_post.c: In function ‘cil_post_fc_fill_data’:
../cil/src/cil_post.c:104:5: warning: this statement may fall through [-Wimplicit-fallthrough=]
    c++;
    ~^~
../cil/src/cil_post.c:105:3: note: here
   default:
   ^~~~~~~

regex.c: In function ‘regex_format_error’:
regex.c:541:10: warning: this statement may fall through [-Wimplicit-fallthrough=]
   *ptr++ = '.';
   ~~~~~~~^~~~~
regex.c:542:2: note: here
  case 3:
  ^~~~
regex.c:543:10: warning: this statement may fall through [-Wimplicit-fallthrough=]
   *ptr++ = '.';
   ~~~~~~~^~~~~
regex.c:544:2: note: here
  case 2:
  ^~~~
regex.c:545:10: warning: this statement may fall through [-Wimplicit-fallthrough=]
   *ptr++ = '.';
   ~~~~~~~^~~~~
regex.c:546:2: note: here
  case 1:
  ^~~~
regex.c: In function ‘regex_format_error’:
regex.c:541:10: warning: this statement may fall through [-Wimplicit-fallthrough=]
   *ptr++ = '.';
   ~~~~~~~^~~~~
regex.c:542:2: note: here
  case 3:
  ^~~~
regex.c:543:10: warning: this statement may fall through [-Wimplicit-fallthrough=]
   *ptr++ = '.';
   ~~~~~~~^~~~~
regex.c:544:2: note: here
  case 2:
  ^~~~
regex.c:545:10: warning: this statement may fall through [-Wimplicit-fallthrough=]
   *ptr++ = '.';
   ~~~~~~~^~~~~
regex.c:546:2: note: here
  case 1:
  ^~~~

modules.c: In function ‘semanage_module_get_path’:
modules.c:602:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
    if (file == NULL) file = "hll";
       ^
modules.c:603:3: note: here
   case SEMANAGE_MODULE_PATH_CIL:
   ^~~~
modules.c:604:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
    if (file == NULL) file = "cil";
       ^
modules.c:605:3: note: here
   case SEMANAGE_MODULE_PATH_LANG_EXT:
   ^~~~

Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
---
 libselinux/src/regex.c        | 4 ++++
 libsemanage/src/modules.c     | 2 ++
 libsepol/cil/src/cil_parser.c | 2 ++
 libsepol/cil/src/cil_post.c   | 1 +
 4 files changed, 9 insertions(+)

diff --git a/libselinux/src/regex.c b/libselinux/src/regex.c
index 0c5ad27..ec1b0c4 100644
--- a/libselinux/src/regex.c
+++ b/libselinux/src/regex.c
@@ -539,12 +539,16 @@ truncated:
 	/* no break statements, fall-through is intended */
 	case 4:
 		*ptr++ = '.';
+		/* FALLTHRU */
 	case 3:
 		*ptr++ = '.';
+		/* FALLTHRU */
 	case 2:
 		*ptr++ = '.';
+		/* FALLTHRU */
 	case 1:
 		*ptr++ = '\0';
+		/* FALLTHRU */
 	default:
 		break;
 	}
diff --git a/libsemanage/src/modules.c b/libsemanage/src/modules.c
index 90c5e49..62af101 100644
--- a/libsemanage/src/modules.c
+++ b/libsemanage/src/modules.c
@@ -600,8 +600,10 @@ int semanage_module_get_path(semanage_handle_t *sh,
 			break;
 		case SEMANAGE_MODULE_PATH_HLL:
 			if (file == NULL) file = "hll";
+			/* FALLTHRU */
 		case SEMANAGE_MODULE_PATH_CIL:
 			if (file == NULL) file = "cil";
+			/* FALLTHRU */
 		case SEMANAGE_MODULE_PATH_LANG_EXT:
 			if (file == NULL) file = "lang_ext";
 
diff --git a/libsepol/cil/src/cil_parser.c b/libsepol/cil/src/cil_parser.c
index 101520c..585ea77 100644
--- a/libsepol/cil/src/cil_parser.c
+++ b/libsepol/cil/src/cil_parser.c
@@ -251,6 +251,7 @@ int cil_parser(char *_path, char *buffer, uint32_t size, struct cil_tree **parse
 		case QSTRING:
 			tok.value[strlen(tok.value) - 1] = '\0';
 			tok.value = tok.value+1;
+			/* FALLTHRU */
 		case SYMBOL:
 			if (paren_count == 0) {
 				cil_log(CIL_ERR, "Symbol not inside parenthesis at line %d of %s\n", tok.line, path);
@@ -275,6 +276,7 @@ int cil_parser(char *_path, char *buffer, uint32_t size, struct cil_tree **parse
 			if (tok.type != END_OF_FILE) {
 				break;
 			}
+			/* FALLTHRU */
 			// Fall through if EOF
 		case END_OF_FILE:
 			if (paren_count > 0) {
diff --git a/libsepol/cil/src/cil_post.c b/libsepol/cil/src/cil_post.c
index 0d494ea..ad073e8 100644
--- a/libsepol/cil/src/cil_post.c
+++ b/libsepol/cil/src/cil_post.c
@@ -102,6 +102,7 @@ void cil_post_fc_fill_data(struct fc_data *fc, char *path)
 			break;
 		case '\\':
 			c++;
+			/* FALLTHRU */
 		default:
 			if (!fc->meta) {
 				fc->stem_len++;
-- 
2.9.4

[PATCH 2/3] libsemanage: Fix snprintf warnings from gcc 7

[Stephen Smalley]

Fixes the following warning from gcc7 by increasing the
buffer size to PATH_MAX.

semanage_store.c: In function ‘semanage_remove_directory’:
semanage_store.c:819:30: warning: ‘%s’ directive output may be truncated writing up to 255 bytes into a region of size 254 [-Wformat-truncation=]
   snprintf(s, sizeof(s), "%s/%s", path, namelist[i]->d_name);
                              ^~
semanage_store.c:819:3: note: ‘snprintf’ output 2 or more bytes (assuming 257) into a destination of size 255
   snprintf(s, sizeof(s), "%s/%s", path, namelist[i]->d_name);
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
---
 libsemanage/src/semanage_store.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libsemanage/src/semanage_store.c b/libsemanage/src/semanage_store.c
index 5642772..6158d08 100644
--- a/libsemanage/src/semanage_store.c
+++ b/libsemanage/src/semanage_store.c
@@ -814,7 +814,7 @@ int semanage_remove_directory(const char *path)
 		return -1;
 	}
 	for (i = 0; i < num_entries; i++) {
-		char s[NAME_MAX];
+		char s[PATH_MAX];
 		struct stat buf;
 		snprintf(s, sizeof(s), "%s/%s", path, namelist[i]->d_name);
 		if (stat(s, &buf) == -1) {
-- 
2.9.4

[PATCH 3/3] libsepol: Fix alloc-size-larger-than warning from gcc 7

[Stephen Smalley]

Fixes the following warning from gcc 7:

In function ‘name_list_to_string’,
    inlined from ‘constraint_expr_to_string’ at module_to_cil.c:1790:8:
module_to_cil.c:1135:6: warning: argument 1 range [18446744071562067968, 18446744073709551615] exceeds maximum object size 9223372036854775807 [-Walloc-size-larger-than=]
  str = malloc(len);
  ~~~~^~~~~~~~~~~~~
In file included from module_to_cil.c:36:0:
module_to_cil.c: In function ‘constraint_expr_to_string’:
/usr/include/stdlib.h:443:14: note: in a call to allocation function ‘malloc’ declared here
 extern void *malloc (size_t __size) __THROW __attribute_malloc__ __wur;
              ^~~~~~

While we are here, fix a few other issues too.
The usage of snprintf was wrong and unnecessary; we just allocated
the string to be the right size, so we should just fill it.

Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>
---
 libsepol/src/module_to_cil.c | 40 +++++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/libsepol/src/module_to_cil.c b/libsepol/src/module_to_cil.c
index 77e1219..31c9aa0 100644
--- a/libsepol/src/module_to_cil.c
+++ b/libsepol/src/module_to_cil.c
@@ -988,8 +988,14 @@ static int ebitmap_to_names(struct ebitmap *map, char **vals_to_names, char ***n
 
 	num = 0;
 	ebitmap_for_each_bit(map, node, i) {
-		if (ebitmap_get_bit(map, i))
+		if (ebitmap_get_bit(map, i)) {
+			if (num >= UINT32_MAX / sizeof(*name_arr)) {
+				log_err("Overflow");
+				rc = -1;
+				goto exit;
+			}
 			num++;
+		}
 	}
 
 	name_arr = malloc(sizeof(*name_arr) * num);
@@ -1117,19 +1123,30 @@ static int name_list_to_string(char **names, int num_names, char **string)
 {
 	// create a space separated string of the names
 	int rc = -1;
-	int len = 0;
+	size_t len = 0;
 	int i;
 	char *str;
 	char *strpos;
-	int name_len;
-	int rlen;
 
 	for (i = 0; i < num_names; i++) {
 		len += strlen(names[i]);
+		if (len < strlen(names[i])) {
+			log_err("Overflow");
+			return -1;
+		}
 	}
 
 	// add spaces + null terminator
-	len += (num_names - 1) + 1;
+	len += num_names;
+	if (len < (size_t)num_names) {
+		log_err("Overflow");
+		return -1;
+	}
+
+	if (!len) {
+		log_err("Empty list");
+		return -1;
+	}
 
 	str = malloc(len);
 	if (str == NULL) {
@@ -1137,22 +1154,15 @@ static int name_list_to_string(char **names, int num_names, char **string)
 		rc = -1;
 		goto exit;
 	}
+	str[0] = 0;
 
 	strpos = str;
 
 	for (i = 0; i < num_names; i++) {
-		name_len = strlen(names[i]);
-		rlen = snprintf(strpos, len - (strpos - str), "%s", names[i]);
-		if (rlen < 0 || rlen >= len) {
-			log_err("Failed to generate name list");
-			rc = -1;
-			goto exit;
-		}
-
+		strpos = stpcpy(strpos, names[i]);
 		if (i < num_names - 1) {
-			strpos[name_len] = ' ';
+			*strpos++ = ' ';
 		}
-		strpos += name_len + 1;
 	}
 
 	*string = str;
-- 
2.9.4

Re: [PATCH 1/3] libsepol,libsemanage,libselinux: Fix fallthrough warnings from gcc 7

[jwcart2]

On 05/31/2017 04:14 PM, Stephen Smalley wrote:
> https://developers.redhat.com/blog/2017/03/10/wimplicit-fallthrough-in-gcc-7/
> 
> Fixes the following warnings by annotating with a /* FALLTHRU */ comment.
> Unfortunately, the __attribute__ ((fallthrough)); approach does not appear
> to work with older compilers.
> 
> ../cil/src/cil_parser.c: In function ‘cil_parser’:
> ../cil/src/cil_parser.c:253:14: warning: this statement may fall through [-Wimplicit-fallthrough=]
>      tok.value = tok.value+1;
>      ~~~~~~~~~~^~~~~~~~~~~~~
> ../cil/src/cil_parser.c:254:3: note: here
>     case SYMBOL:
>     ^~~~
> ../cil/src/cil_parser.c:275:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
>      if (tok.type != END_OF_FILE) {
>         ^
> ../cil/src/cil_parser.c:279:3: note: here
>     case END_OF_FILE:
>     ^~~~
> 
> ../cil/src/cil_post.c: In function ‘cil_post_fc_fill_data’:
> ../cil/src/cil_post.c:104:5: warning: this statement may fall through [-Wimplicit-fallthrough=]
>      c++;
>      ~^~
> ../cil/src/cil_post.c:105:3: note: here
>     default:
>     ^~~~~~~
> 
> regex.c: In function ‘regex_format_error’:
> regex.c:541:10: warning: this statement may fall through [-Wimplicit-fallthrough=]
>     *ptr++ = '.';
>     ~~~~~~~^~~~~
> regex.c:542:2: note: here
>    case 3:
>    ^~~~
> regex.c:543:10: warning: this statement may fall through [-Wimplicit-fallthrough=]
>     *ptr++ = '.';
>     ~~~~~~~^~~~~
> regex.c:544:2: note: here
>    case 2:
>    ^~~~
> regex.c:545:10: warning: this statement may fall through [-Wimplicit-fallthrough=]
>     *ptr++ = '.';
>     ~~~~~~~^~~~~
> regex.c:546:2: note: here
>    case 1:
>    ^~~~
> regex.c: In function ‘regex_format_error’:
> regex.c:541:10: warning: this statement may fall through [-Wimplicit-fallthrough=]
>     *ptr++ = '.';
>     ~~~~~~~^~~~~
> regex.c:542:2: note: here
>    case 3:
>    ^~~~
> regex.c:543:10: warning: this statement may fall through [-Wimplicit-fallthrough=]
>     *ptr++ = '.';
>     ~~~~~~~^~~~~
> regex.c:544:2: note: here
>    case 2:
>    ^~~~
> regex.c:545:10: warning: this statement may fall through [-Wimplicit-fallthrough=]
>     *ptr++ = '.';
>     ~~~~~~~^~~~~
> regex.c:546:2: note: here
>    case 1:
>    ^~~~
> 
> modules.c: In function ‘semanage_module_get_path’:
> modules.c:602:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
>      if (file == NULL) file = "hll";
>         ^
> modules.c:603:3: note: here
>     case SEMANAGE_MODULE_PATH_CIL:
>     ^~~~
> modules.c:604:7: warning: this statement may fall through [-Wimplicit-fallthrough=]
>      if (file == NULL) file = "cil";
>         ^
> modules.c:605:3: note: here
>     case SEMANAGE_MODULE_PATH_LANG_EXT:
>     ^~~~
> 
> Signed-off-by: Stephen Smalley <sds@tycho.nsa.gov>

All three have been applied.

Jim

> ---
>   libselinux/src/regex.c        | 4 ++++
>   libsemanage/src/modules.c     | 2 ++
>   libsepol/cil/src/cil_parser.c | 2 ++
>   libsepol/cil/src/cil_post.c   | 1 +
>   4 files changed, 9 insertions(+)
> 
> diff --git a/libselinux/src/regex.c b/libselinux/src/regex.c
> index 0c5ad27..ec1b0c4 100644
> --- a/libselinux/src/regex.c
> +++ b/libselinux/src/regex.c
> @@ -539,12 +539,16 @@ truncated:
>   	/* no break statements, fall-through is intended */
>   	case 4:
>   		*ptr++ = '.';
> +		/* FALLTHRU */
>   	case 3:
>   		*ptr++ = '.';
> +		/* FALLTHRU */
>   	case 2:
>   		*ptr++ = '.';
> +		/* FALLTHRU */
>   	case 1:
>   		*ptr++ = '\0';
> +		/* FALLTHRU */
>   	default:
>   		break;
>   	}
> diff --git a/libsemanage/src/modules.c b/libsemanage/src/modules.c
> index 90c5e49..62af101 100644
> --- a/libsemanage/src/modules.c
> +++ b/libsemanage/src/modules.c
> @@ -600,8 +600,10 @@ int semanage_module_get_path(semanage_handle_t *sh,
>   			break;
>   		case SEMANAGE_MODULE_PATH_HLL:
>   			if (file == NULL) file = "hll";
> +			/* FALLTHRU */
>   		case SEMANAGE_MODULE_PATH_CIL:
>   			if (file == NULL) file = "cil";
> +			/* FALLTHRU */
>   		case SEMANAGE_MODULE_PATH_LANG_EXT:
>   			if (file == NULL) file = "lang_ext";
>   
> diff --git a/libsepol/cil/src/cil_parser.c b/libsepol/cil/src/cil_parser.c
> index 101520c..585ea77 100644
> --- a/libsepol/cil/src/cil_parser.c
> +++ b/libsepol/cil/src/cil_parser.c
> @@ -251,6 +251,7 @@ int cil_parser(char *_path, char *buffer, uint32_t size, struct cil_tree **parse
>   		case QSTRING:
>   			tok.value[strlen(tok.value) - 1] = '\0';
>   			tok.value = tok.value+1;
> +			/* FALLTHRU */
>   		case SYMBOL:
>   			if (paren_count == 0) {
>   				cil_log(CIL_ERR, "Symbol not inside parenthesis at line %d of %s\n", tok.line, path);
> @@ -275,6 +276,7 @@ int cil_parser(char *_path, char *buffer, uint32_t size, struct cil_tree **parse
>   			if (tok.type != END_OF_FILE) {
>   				break;
>   			}
> +			/* FALLTHRU */
>   			// Fall through if EOF
>   		case END_OF_FILE:
>   			if (paren_count > 0) {
> diff --git a/libsepol/cil/src/cil_post.c b/libsepol/cil/src/cil_post.c
> index 0d494ea..ad073e8 100644
> --- a/libsepol/cil/src/cil_post.c
> +++ b/libsepol/cil/src/cil_post.c
> @@ -102,6 +102,7 @@ void cil_post_fc_fill_data(struct fc_data *fc, char *path)
>   			break;
>   		case '\\':
>   			c++;
> +			/* FALLTHRU */
>   		default:
>   			if (!fc->meta) {
>   				fc->stem_len++;
> 


-- 
James Carter <jwcart2@tycho.nsa.gov>
National Security Agency