[1/3] libselinux: simplify string copying

Message ID	20221109200939.62525-1-cgzones@googlemail.com (mailing list archive)
State	Accepted
Commit	d3c6828a6a7d
Headers	show Return-Path: <selinux-owner@kernel.org> From: =?utf-8?q?Christian_G=C3=B6ttsche?= <cgzones@googlemail.com> To: selinux@vger.kernel.org Subject: [PATCH 1/3] libselinux: simplify string copying Date: Wed, 9 Nov 2022 21:09:37 +0100 Message-Id: <20221109200939.62525-1-cgzones@googlemail.com> MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Precedence: bulk
Series	[1/3] libselinux: simplify string copying \| expand [1/3] libselinux: simplify string copying [2/3] checkpolicy: simplify string copying [3/3] libsepol: simplify string copying

Message ID

20221109200939.62525-1-cgzones@googlemail.com (mailing list archive)

State

Accepted

Commit

d3c6828a6a7d

Headers

From: =?utf-8?q?Christian_G=C3=B6ttsche?= <cgzones@googlemail.com>
To: selinux@vger.kernel.org
Subject: [PATCH 1/3] libselinux: simplify string copying
Date: Wed,  9 Nov 2022 21:09:37 +0100
Message-Id: <20221109200939.62525-1-cgzones@googlemail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
Precedence: bulk

Series

[1/3] libselinux: simplify string copying | expand

Commit Message

Christian Göttsche Nov. 9, 2022, 8:09 p.m. UTC

Use strdup(3)/strndup(3) instead of allocating memory and then manually
copying the content.

Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
---
 libselinux/src/context.c                     | 11 +++++------
 libselinux/src/get_default_type.c            |  3 +--
 libselinux/src/matchpathcon.c                |  9 +++------
 libselinux/utils/selabel_lookup_best_match.c | 10 ++++------
 4 files changed, 13 insertions(+), 20 deletions(-)

Comments

James Carter Nov. 10, 2022, 1:55 p.m. UTC | #1

On Wed, Nov 9, 2022 at 3:11 PM Christian Göttsche
<cgzones@googlemail.com> wrote:
>
> Use strdup(3)/strndup(3) instead of allocating memory and then manually
> copying the content.
>
> Signed-off-by: Christian Göttsche <cgzones@googlemail.com>

For these three patches:
Acked-by: James Carter <jwcart2@gmail.com>

> ---
>  libselinux/src/context.c                     | 11 +++++------
>  libselinux/src/get_default_type.c            |  3 +--
>  libselinux/src/matchpathcon.c                |  9 +++------
>  libselinux/utils/selabel_lookup_best_match.c | 10 ++++------
>  4 files changed, 13 insertions(+), 20 deletions(-)
>
> diff --git a/libselinux/src/context.c b/libselinux/src/context.c
> index 9dddbc5a..8830bf42 100644
> --- a/libselinux/src/context.c
> +++ b/libselinux/src/context.c
> @@ -149,19 +149,18 @@ static int set_comp(context_private_t * n, int idx, const char *str)
>         char *t = NULL;
>         const char *p;
>         if (str) {
> -               t = (char *)malloc(strlen(str) + 1);
> -               if (!t) {
> -                       return -1;
> -               }
>                 for (p = str; *p; p++) {
>                         if (*p == '\t' || *p == '\n' || *p == '\r' ||
>                             ((*p == ':' || *p == ' ') && idx != COMP_RANGE)) {
> -                               free(t);
>                                 errno = EINVAL;
>                                 return -1;
>                         }
>                 }
> -               strcpy(t, str);
> +
> +               t = strdup(str);
> +               if (!t) {
> +                       return -1;
> +               }
>         }
>         conditional_free(&n->component[idx]);
>         n->component[idx] = t;
> diff --git a/libselinux/src/get_default_type.c b/libselinux/src/get_default_type.c
> index dd7b5d79..766ea4b7 100644
> --- a/libselinux/src/get_default_type.c
> +++ b/libselinux/src/get_default_type.c
> @@ -62,10 +62,9 @@ static int find_default_type(FILE * fp, const char *role, char **type)
>                 return -1;
>         }
>
> -       t = malloc(strlen(buf) - len);
> +       t = strndup(ptr, strlen(buf) - len - 1);
>         if (!t)
>                 return -1;
> -       strcpy(t, ptr);
>         *type = t;
>         return 0;
>  }
> diff --git a/libselinux/src/matchpathcon.c b/libselinux/src/matchpathcon.c
> index ea78a23e..bf2da083 100644
> --- a/libselinux/src/matchpathcon.c
> +++ b/libselinux/src/matchpathcon.c
> @@ -215,10 +215,9 @@ int matchpathcon_filespec_add(ino_t ino, int specind, const char *file)
>                         if (ret < 0 || sb.st_ino != ino) {
>                                 fl->specind = specind;
>                                 free(fl->file);
> -                               fl->file = malloc(strlen(file) + 1);
> +                               fl->file = strdup(file);
>                                 if (!fl->file)
>                                         goto oom;
> -                               strcpy(fl->file, file);
>                                 return fl->specind;
>
>                         }
> @@ -232,10 +231,9 @@ int matchpathcon_filespec_add(ino_t ino, int specind, const char *file)
>                              __FUNCTION__, file, fl->file,
>                              con_array[fl->specind]);
>                         free(fl->file);
> -                       fl->file = malloc(strlen(file) + 1);
> +                       fl->file = strdup(file);
>                         if (!fl->file)
>                                 goto oom;
> -                       strcpy(fl->file, file);
>                         return fl->specind;
>                 }
>
> @@ -248,10 +246,9 @@ int matchpathcon_filespec_add(ino_t ino, int specind, const char *file)
>                 goto oom;
>         fl->ino = ino;
>         fl->specind = specind;
> -       fl->file = malloc(strlen(file) + 1);
> +       fl->file = strdup(file);
>         if (!fl->file)
>                 goto oom_freefl;
> -       strcpy(fl->file, file);
>         fl->next = prevfl->next;
>         prevfl->next = fl;
>         return fl->specind;
> diff --git a/libselinux/utils/selabel_lookup_best_match.c b/libselinux/utils/selabel_lookup_best_match.c
> index a4af0679..e816c04b 100644
> --- a/libselinux/utils/selabel_lookup_best_match.c
> +++ b/libselinux/utils/selabel_lookup_best_match.c
> @@ -30,7 +30,7 @@ static __attribute__ ((__noreturn__)) void usage(const char *progname)
>         exit(1);
>  }
>
> -static mode_t string_to_mode(char *s)
> +static mode_t string_to_mode(const char *s)
>  {
>         switch (s[0]) {
>         case 'b':
> @@ -53,7 +53,7 @@ static mode_t string_to_mode(char *s)
>
>  int main(int argc, char **argv)
>  {
> -       int raw = 0, mode = 0, rc, opt, i, num_links, string_len;
> +       int raw = 0, mode = 0, rc, opt, i, num_links;
>         char *validate = NULL, *path = NULL, *context = NULL, *file = NULL;
>         char **links = NULL;
>
> @@ -101,13 +101,11 @@ int main(int argc, char **argv)
>                 }
>
>                 for (i = optind, num_links = 0; i < argc; i++, num_links++) {
> -                       string_len = strlen(argv[i]) + 1;
> -                       links[num_links] = malloc(string_len);
> +                       links[num_links] = strdup(argv[i]);
>                         if (!links[num_links]) {
> -                               fprintf(stderr, "ERROR: malloc failed.\n");
> +                               fprintf(stderr, "ERROR: strdup failed.\n");
>                                 exit(1);
>                         }
> -                       strcpy(links[num_links], argv[i]);
>                 }
>         }
>
> --
> 2.38.1
>

James Carter Nov. 21, 2022, 8:55 p.m. UTC | #2

On Thu, Nov 10, 2022 at 8:55 AM James Carter <jwcart2@gmail.com> wrote:
>
> On Wed, Nov 9, 2022 at 3:11 PM Christian Göttsche
> <cgzones@googlemail.com> wrote:
> >
> > Use strdup(3)/strndup(3) instead of allocating memory and then manually
> > copying the content.
> >
> > Signed-off-by: Christian Göttsche <cgzones@googlemail.com>
>
> For these three patches:
> Acked-by: James Carter <jwcart2@gmail.com>
>

These three patches have been merged.
Thanks,
Jim

> > ---
> >  libselinux/src/context.c                     | 11 +++++------
> >  libselinux/src/get_default_type.c            |  3 +--
> >  libselinux/src/matchpathcon.c                |  9 +++------
> >  libselinux/utils/selabel_lookup_best_match.c | 10 ++++------
> >  4 files changed, 13 insertions(+), 20 deletions(-)
> >
> > diff --git a/libselinux/src/context.c b/libselinux/src/context.c
> > index 9dddbc5a..8830bf42 100644
> > --- a/libselinux/src/context.c
> > +++ b/libselinux/src/context.c
> > @@ -149,19 +149,18 @@ static int set_comp(context_private_t * n, int idx, const char *str)
> >         char *t = NULL;
> >         const char *p;
> >         if (str) {
> > -               t = (char *)malloc(strlen(str) + 1);
> > -               if (!t) {
> > -                       return -1;
> > -               }
> >                 for (p = str; *p; p++) {
> >                         if (*p == '\t' || *p == '\n' || *p == '\r' ||
> >                             ((*p == ':' || *p == ' ') && idx != COMP_RANGE)) {
> > -                               free(t);
> >                                 errno = EINVAL;
> >                                 return -1;
> >                         }
> >                 }
> > -               strcpy(t, str);
> > +
> > +               t = strdup(str);
> > +               if (!t) {
> > +                       return -1;
> > +               }
> >         }
> >         conditional_free(&n->component[idx]);
> >         n->component[idx] = t;
> > diff --git a/libselinux/src/get_default_type.c b/libselinux/src/get_default_type.c
> > index dd7b5d79..766ea4b7 100644
> > --- a/libselinux/src/get_default_type.c
> > +++ b/libselinux/src/get_default_type.c
> > @@ -62,10 +62,9 @@ static int find_default_type(FILE * fp, const char *role, char **type)
> >                 return -1;
> >         }
> >
> > -       t = malloc(strlen(buf) - len);
> > +       t = strndup(ptr, strlen(buf) - len - 1);
> >         if (!t)
> >                 return -1;
> > -       strcpy(t, ptr);
> >         *type = t;
> >         return 0;
> >  }
> > diff --git a/libselinux/src/matchpathcon.c b/libselinux/src/matchpathcon.c
> > index ea78a23e..bf2da083 100644
> > --- a/libselinux/src/matchpathcon.c
> > +++ b/libselinux/src/matchpathcon.c
> > @@ -215,10 +215,9 @@ int matchpathcon_filespec_add(ino_t ino, int specind, const char *file)
> >                         if (ret < 0 || sb.st_ino != ino) {
> >                                 fl->specind = specind;
> >                                 free(fl->file);
> > -                               fl->file = malloc(strlen(file) + 1);
> > +                               fl->file = strdup(file);
> >                                 if (!fl->file)
> >                                         goto oom;
> > -                               strcpy(fl->file, file);
> >                                 return fl->specind;
> >
> >                         }
> > @@ -232,10 +231,9 @@ int matchpathcon_filespec_add(ino_t ino, int specind, const char *file)
> >                              __FUNCTION__, file, fl->file,
> >                              con_array[fl->specind]);
> >                         free(fl->file);
> > -                       fl->file = malloc(strlen(file) + 1);
> > +                       fl->file = strdup(file);
> >                         if (!fl->file)
> >                                 goto oom;
> > -                       strcpy(fl->file, file);
> >                         return fl->specind;
> >                 }
> >
> > @@ -248,10 +246,9 @@ int matchpathcon_filespec_add(ino_t ino, int specind, const char *file)
> >                 goto oom;
> >         fl->ino = ino;
> >         fl->specind = specind;
> > -       fl->file = malloc(strlen(file) + 1);
> > +       fl->file = strdup(file);
> >         if (!fl->file)
> >                 goto oom_freefl;
> > -       strcpy(fl->file, file);
> >         fl->next = prevfl->next;
> >         prevfl->next = fl;
> >         return fl->specind;
> > diff --git a/libselinux/utils/selabel_lookup_best_match.c b/libselinux/utils/selabel_lookup_best_match.c
> > index a4af0679..e816c04b 100644
> > --- a/libselinux/utils/selabel_lookup_best_match.c
> > +++ b/libselinux/utils/selabel_lookup_best_match.c
> > @@ -30,7 +30,7 @@ static __attribute__ ((__noreturn__)) void usage(const char *progname)
> >         exit(1);
> >  }
> >
> > -static mode_t string_to_mode(char *s)
> > +static mode_t string_to_mode(const char *s)
> >  {
> >         switch (s[0]) {
> >         case 'b':
> > @@ -53,7 +53,7 @@ static mode_t string_to_mode(char *s)
> >
> >  int main(int argc, char **argv)
> >  {
> > -       int raw = 0, mode = 0, rc, opt, i, num_links, string_len;
> > +       int raw = 0, mode = 0, rc, opt, i, num_links;
> >         char *validate = NULL, *path = NULL, *context = NULL, *file = NULL;
> >         char **links = NULL;
> >
> > @@ -101,13 +101,11 @@ int main(int argc, char **argv)
> >                 }
> >
> >                 for (i = optind, num_links = 0; i < argc; i++, num_links++) {
> > -                       string_len = strlen(argv[i]) + 1;
> > -                       links[num_links] = malloc(string_len);
> > +                       links[num_links] = strdup(argv[i]);
> >                         if (!links[num_links]) {
> > -                               fprintf(stderr, "ERROR: malloc failed.\n");
> > +                               fprintf(stderr, "ERROR: strdup failed.\n");
> >                                 exit(1);
> >                         }
> > -                       strcpy(links[num_links], argv[i]);
> >                 }
> >         }
> >
> > --
> > 2.38.1
> >

diff --git a/libselinux/src/context.c b/libselinux/src/context.c
index 9dddbc5a..8830bf42 100644
--- a/libselinux/src/context.c
+++ b/libselinux/src/context.c
@@ -149,19 +149,18 @@  static int set_comp(context_private_t * n, int idx, const char *str)
 	char *t = NULL;
 	const char *p;
 	if (str) {
-		t = (char *)malloc(strlen(str) + 1);
-		if (!t) {
-			return -1;
-		}
 		for (p = str; *p; p++) {
 			if (*p == '\t' || *p == '\n' || *p == '\r' ||
 			    ((*p == ':' || *p == ' ') && idx != COMP_RANGE)) {
-				free(t);
 				errno = EINVAL;
 				return -1;
 			}
 		}
-		strcpy(t, str);
+
+		t = strdup(str);
+		if (!t) {
+			return -1;
+		}
 	}
 	conditional_free(&n->component[idx]);
 	n->component[idx] = t;
diff --git a/libselinux/src/get_default_type.c b/libselinux/src/get_default_type.c
index dd7b5d79..766ea4b7 100644
--- a/libselinux/src/get_default_type.c
+++ b/libselinux/src/get_default_type.c
@@ -62,10 +62,9 @@  static int find_default_type(FILE * fp, const char *role, char **type)
 		return -1;
 	}
 
-	t = malloc(strlen(buf) - len);
+	t = strndup(ptr, strlen(buf) - len - 1);
 	if (!t)
 		return -1;
-	strcpy(t, ptr);
 	*type = t;
 	return 0;
 }
diff --git a/libselinux/src/matchpathcon.c b/libselinux/src/matchpathcon.c
index ea78a23e..bf2da083 100644
--- a/libselinux/src/matchpathcon.c
+++ b/libselinux/src/matchpathcon.c
@@ -215,10 +215,9 @@  int matchpathcon_filespec_add(ino_t ino, int specind, const char *file)
 			if (ret < 0 || sb.st_ino != ino) {
 				fl->specind = specind;
 				free(fl->file);
-				fl->file = malloc(strlen(file) + 1);
+				fl->file = strdup(file);
 				if (!fl->file)
 					goto oom;
-				strcpy(fl->file, file);
 				return fl->specind;
 
 			}
@@ -232,10 +231,9 @@  int matchpathcon_filespec_add(ino_t ino, int specind, const char *file)
 			     __FUNCTION__, file, fl->file,
 			     con_array[fl->specind]);
 			free(fl->file);
-			fl->file = malloc(strlen(file) + 1);
+			fl->file = strdup(file);
 			if (!fl->file)
 				goto oom;
-			strcpy(fl->file, file);
 			return fl->specind;
 		}
 
@@ -248,10 +246,9 @@  int matchpathcon_filespec_add(ino_t ino, int specind, const char *file)
 		goto oom;
 	fl->ino = ino;
 	fl->specind = specind;
-	fl->file = malloc(strlen(file) + 1);
+	fl->file = strdup(file);
 	if (!fl->file)
 		goto oom_freefl;
-	strcpy(fl->file, file);
 	fl->next = prevfl->next;
 	prevfl->next = fl;
 	return fl->specind;
diff --git a/libselinux/utils/selabel_lookup_best_match.c b/libselinux/utils/selabel_lookup_best_match.c
index a4af0679..e816c04b 100644
--- a/libselinux/utils/selabel_lookup_best_match.c
+++ b/libselinux/utils/selabel_lookup_best_match.c
@@ -30,7 +30,7 @@  static __attribute__ ((__noreturn__)) void usage(const char *progname)
 	exit(1);
 }
 
-static mode_t string_to_mode(char *s)
+static mode_t string_to_mode(const char *s)
 {
 	switch (s[0]) {
 	case 'b':
@@ -53,7 +53,7 @@  static mode_t string_to_mode(char *s)
 
 int main(int argc, char **argv)
 {
-	int raw = 0, mode = 0, rc, opt, i, num_links, string_len;
+	int raw = 0, mode = 0, rc, opt, i, num_links;
 	char *validate = NULL, *path = NULL, *context = NULL, *file = NULL;
 	char **links = NULL;
 
@@ -101,13 +101,11 @@  int main(int argc, char **argv)
 		}
 
 		for (i = optind, num_links = 0; i < argc; i++, num_links++) {
-			string_len = strlen(argv[i]) + 1;
-			links[num_links] = malloc(string_len);
+			links[num_links] = strdup(argv[i]);
 			if (!links[num_links]) {
-				fprintf(stderr, "ERROR: malloc failed.\n");
+				fprintf(stderr, "ERROR: strdup failed.\n");
 				exit(1);
 			}
-			strcpy(links[num_links], argv[i]);
 		}
 	}

[1/3] libselinux: simplify string copying

Commit Message

Comments

Patch