* [PATCH] conf-parse.y checks (with style fixes removed)
@ 2019-05-18 16:57 Jokke Hämäläinen
2019-05-24 17:07 ` Stephen Smalley
0 siblings, 1 reply; 3+ messages in thread
From: Jokke Hämäläinen @ 2019-05-18 16:57 UTC (permalink / raw)
To: selinux
diff --git a/libsemanage/src/conf-parse.y b/libsemanage/src/conf-parse.y
index b527e893..64b53c2d 100644
--- a/libsemanage/src/conf-parse.y
+++ b/libsemanage/src/conf-parse.y
@@ -221,6 +221,9 @@ usepasswd: USEPASSWD '=' ARG {
ignoredirs: IGNOREDIRS '=' ARG {
current_conf->ignoredirs = strdup($3);
+ if (current_conf->ignoredirs == NULL) {
+ yyerror("could not allocate memory for current_conf->ignoredirs");
+ }
free($3);
}
@@ -240,8 +243,8 @@ handle_unknown: HANDLE_UNKNOWN '=' ARG {
bzip_blocksize: BZIP_BLOCKSIZE '=' ARG {
int blocksize = atoi($3);
free($3);
- if (blocksize > 9)
- yyerror("bzip-blocksize can only be in the range 0-9");
+ if (blocksize < 1 || blocksize > 9)
+ yyerror("bzip-blocksize can only be in the range 1-9");
else
current_conf->bzip_blocksize = blocksize;
}
@@ -339,9 +342,18 @@ static int semanage_conf_init(semanage_conf_t * conf)
{
conf->store_type = SEMANAGE_CON_DIRECT;
conf->store_path = strdup(basename(selinux_policy_root()));
+ if (conf->store_path == NULL) {
+ return -1;
+ }
conf->ignoredirs = NULL;
conf->store_root_path = strdup("/var/lib/selinux");
+ if (conf->store_root_path == NULL) {
+ return -1;
+ }
conf->compiler_directory_path = strdup("/usr/libexec/selinux/hll");
+ if (conf->compiler_directory_path == NULL) {
+ return -1;
+ }
conf->policyvers = sepol_policy_kern_vers_max();
conf->target_platform = SEPOL_TARGET_SELINUX;
conf->expand_check = 1;
@@ -515,7 +527,10 @@ static int parse_module_store(char *arg)
}
}
}
- return 0;
+
+ if (current_conf->store_path)
+ return 0;
+ return -3;
}
static int parse_store_root_path(char *arg)
@@ -526,6 +541,10 @@ static int parse_store_root_path(char *arg)
free(current_conf->store_root_path);
current_conf->store_root_path = strdup(arg);
+ if (current_conf->store_root_path == NULL) {
+ return -2;
+ }
+
return 0;
}
@@ -536,6 +555,9 @@ static int parse_compiler_path(char *arg)
}
free(current_conf->compiler_directory_path);
current_conf->compiler_directory_path = strdup(arg);
+ if (current_conf->compiler_directory_path = NULL) {
+ return -2;
+ }
return 0;
}
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] conf-parse.y checks (with style fixes removed)
2019-05-18 16:57 [PATCH] conf-parse.y checks (with style fixes removed) Jokke Hämäläinen
@ 2019-05-24 17:07 ` Stephen Smalley
2019-05-24 17:40 ` selinux
0 siblings, 1 reply; 3+ messages in thread
From: Stephen Smalley @ 2019-05-24 17:07 UTC (permalink / raw)
To: Jokke Hämäläinen, selinux
On 5/18/19 12:57 PM, Jokke Hämäläinen wrote:
>
> diff --git a/libsemanage/src/conf-parse.y b/libsemanage/src/conf-parse.y
> index b527e893..64b53c2d 100644
> --- a/libsemanage/src/conf-parse.y
> +++ b/libsemanage/src/conf-parse.y
> @@ -221,6 +221,9 @@ usepasswd: USEPASSWD '=' ARG {
>
> ignoredirs: IGNOREDIRS '=' ARG {
> current_conf->ignoredirs = strdup($3);
> + if (current_conf->ignoredirs == NULL) {
> + yyerror("could not allocate memory for current_conf->ignoredirs");
I wouldn't put the variable name/field name into the error message.
"could not allocate memory for ignoredirs" would suffice.
> + }
> free($3);
> }
>
> @@ -240,8 +243,8 @@ handle_unknown: HANDLE_UNKNOWN '=' ARG {
> bzip_blocksize: BZIP_BLOCKSIZE '=' ARG {
> int blocksize = atoi($3);
> free($3);
> - if (blocksize > 9)
> - yyerror("bzip-blocksize can only be in the range 0-9");
> + if (blocksize < 1 || blocksize > 9)
> + yyerror("bzip-blocksize can only be in the range 1-9");
> else
> current_conf->bzip_blocksize = blocksize;
> }
> @@ -339,9 +342,18 @@ static int semanage_conf_init(semanage_conf_t * conf)
> {
> conf->store_type = SEMANAGE_CON_DIRECT;
> conf->store_path = strdup(basename(selinux_policy_root()));
> + if (conf->store_path == NULL) {
> + return -1;
> + }
> conf->ignoredirs = NULL;
> conf->store_root_path = strdup("/var/lib/selinux");
> + if (conf->store_root_path == NULL) {
> + return -1;
> + }
> conf->compiler_directory_path = strdup("/usr/libexec/selinux/hll");
> + if (conf->compiler_directory_path == NULL) {
> + return -1;
> + }
> conf->policyvers = sepol_policy_kern_vers_max();
> conf->target_platform = SEPOL_TARGET_SELINUX;
> conf->expand_check = 1;
> @@ -515,7 +527,10 @@ static int parse_module_store(char *arg)
> }
> }
> }
> - return 0;
> +
> + if (current_conf->store_path)
> + return 0;
> + return -3;
> }
>
> static int parse_store_root_path(char *arg)
> @@ -526,6 +541,10 @@ static int parse_store_root_path(char *arg)
>
> free(current_conf->store_root_path);
> current_conf->store_root_path = strdup(arg);
> + if (current_conf->store_root_path == NULL) {
> + return -2;
Not sure why we are using multiple error return codes here or what they
mean.
> + }
> +
> return 0;
> }
>
> @@ -536,6 +555,9 @@ static int parse_compiler_path(char *arg)
> }
> free(current_conf->compiler_directory_path);
> current_conf->compiler_directory_path = strdup(arg);
> + if (current_conf->compiler_directory_path = NULL) {
> + return -2;
> + }
> return 0;
> }
>
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] conf-parse.y checks (with style fixes removed)
2019-05-24 17:07 ` Stephen Smalley
@ 2019-05-24 17:40 ` selinux
0 siblings, 0 replies; 3+ messages in thread
From: selinux @ 2019-05-24 17:40 UTC (permalink / raw)
To: Stephen Smalley; +Cc: selinux
[-- Attachment #1: Type: text/plain, Size: 2472 bytes --]
On Fri, 24 May 2019, Stephen Smalley wrote:
> I wouldn't put the variable name/field name into the error message. "could not
> allocate memory for ignoredirs" would suffice.
Agreed. I will fix it.
> > + }
> > free($3);
> > }
> > @@ -240,8 +243,8 @@ handle_unknown: HANDLE_UNKNOWN '=' ARG {
> > bzip_blocksize: BZIP_BLOCKSIZE '=' ARG {
> > int blocksize = atoi($3);
> > free($3);
> > - if (blocksize > 9)
> > - yyerror("bzip-blocksize can only be in the range 0-9");
> > + if (blocksize < 1 || blocksize > 9)
> > + yyerror("bzip-blocksize can only be in the range 1-9");
> > else
> > current_conf->bzip_blocksize = blocksize;
> > }
> > @@ -339,9 +342,18 @@ static int semanage_conf_init(semanage_conf_t * conf)
> > {
> > conf->store_type = SEMANAGE_CON_DIRECT;
> > conf->store_path = strdup(basename(selinux_policy_root()));
> > + if (conf->store_path == NULL) {
> > + return -1;
> > + }
> > conf->ignoredirs = NULL;
> > conf->store_root_path = strdup("/var/lib/selinux");
> > + if (conf->store_root_path == NULL) {
> > + return -1;
> > + }
> > conf->compiler_directory_path = strdup("/usr/libexec/selinux/hll");
> > + if (conf->compiler_directory_path == NULL) {
> > + return -1;
> > + }
> > conf->policyvers = sepol_policy_kern_vers_max();
> > conf->target_platform = SEPOL_TARGET_SELINUX;
> > conf->expand_check = 1;
> > @@ -515,7 +527,10 @@ static int parse_module_store(char *arg)
> > }
> > }
> > }
> > - return 0;
> > +
> > + if (current_conf->store_path)
> > + return 0;
> > + return -3;
> > }
> >
> > static int parse_store_root_path(char *arg)
> > @@ -526,6 +541,10 @@ static int parse_store_root_path(char *arg)
> >
> > free(current_conf->store_root_path);
> > current_conf->store_root_path = strdup(arg);
> > + if (current_conf->store_root_path == NULL) {
> > + return -2;
>
> Not sure why we are using multiple error return codes here or what they mean.
I think some of the existing code in this file used "return -1" and
"return -2", so I followed that choice just in case someone would be
interested in inspecting why the functions failed.
As of now, the callers do not care more specifically than checking for
non-zero, but if someone would run the code in e.g. a debugger, it might
be good to have separate return values for different errors - I am not
sure, though.
If you want all errors to be -1, I can of course do that?
Regards,
Jokke Hämäläinen
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-05-24 18:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-05-18 16:57 [PATCH] conf-parse.y checks (with style fixes removed) Jokke Hämäläinen
2019-05-24 17:07 ` Stephen Smalley
2019-05-24 17:40 ` selinux
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).