diff mbox series

[v2,3/3] depmod: shut up gcc insufficinet buffer warning.

Message ID b45014c93b47d9100d0add3d9458e6ec29d0f188.1544476531.git.msuchanek@suse.de (mailing list archive)
State New, archived
Headers show
Series Fix dependency file corruption with parallel depmod invocation | expand

Commit Message

Michal Suchánek Dec. 10, 2018, 9:29 p.m. UTC 
In a couple of places depmod concatenates the module directory and filename
with snprintf. This can technically overflow creating an unterminated string if
module directory name is long. Use openat instead as is done elsewhere in
depmod. This avoids the snprintf, the extra buffer on stack, and the gcc
warning. It may even fix a corner case when the module direcotry name is just
under PATH_MAX.

Signed-off-by: Michal Suchanek <msuchanek@suse.de>
---
 tools/depmod.c | 51 ++++++++++++++++++++++++++++++++++----------------
 1 file changed, 35 insertions(+), 16 deletions(-)

Comments

Lucas De Marchi Dec. 17, 2018, 6:19 p.m. UTC | #1 
On Mon, Dec 10, 2018 at 2:03 PM Michal Suchanek <msuchanek@suse.de> wrote:
>
> In a couple of places depmod concatenates the module directory and filename
> with snprintf. This can technically overflow creating an unterminated string if
> module directory name is long. Use openat instead as is done elsewhere in
> depmod. This avoids the snprintf, the extra buffer on stack, and the gcc
> warning. It may even fix a corner case when the module direcotry name is just
> under PATH_MAX.
>
> Signed-off-by: Michal Suchanek <msuchanek@suse.de>
> ---
>  tools/depmod.c | 51 ++++++++++++++++++++++++++++++++++----------------
>  1 file changed, 35 insertions(+), 16 deletions(-)
>
> diff --git a/tools/depmod.c b/tools/depmod.c
> index 3b6d16e76160..fb76f823c016 100644
> --- a/tools/depmod.c
> +++ b/tools/depmod.c
> @@ -1389,19 +1389,42 @@ static int depmod_modules_build_array(struct depmod *depmod)
>         return 0;
>  }
>
> +static FILE * dfdopen(const char * dname, const char * filename, int flags, const char * mode)

coding style here is not consistent with the rest. We don't have space
after "*".

> +       {

and this is wrongly indented.

Other than that this LGTM and I can fixup the styling issues while applying.

Thanks.
Lucas De Marchi

> +       int fd, dfd;
> +       FILE * ret;
> +
> +       dfd = open(dname, O_RDONLY);
> +       if (dfd < 0) {
> +               WRN("could not open directory %s: %m\n", dname);
> +               return NULL;
> +       }
> +
> +       fd = openat(dfd, filename, flags);
> +       if (fd < 0) {
> +               WRN("could not open %s at %s: %m\n", filename, dname);
> +               ret = NULL;
> +       } else {
> +               ret = fdopen(fd, mode);
> +               if (!ret)
> +                       WRN("could not associate stream with %s: %m\n", filename);
> +       }
> +       close(dfd);
> +       return ret;
> +}
> +
> +
> +
>  static void depmod_modules_sort(struct depmod *depmod)
>  {
> -       char order_file[PATH_MAX], line[PATH_MAX];
> +       char line[PATH_MAX];
> +       const char * order_file = "modules.order";
>         FILE *fp;
>         unsigned idx = 0, total = 0;
>
> -       snprintf(order_file, sizeof(order_file), "%s/modules.order",
> -                depmod->cfg->dirname);
> -       fp = fopen(order_file, "r");
> -       if (fp == NULL) {
> -               WRN("could not open %s: %m\n", order_file);
> +       fp = dfdopen(depmod->cfg->dirname, order_file, O_RDONLY, "r");
> +       if (fp == NULL)
>                 return;
> -       }
>
>         while (fgets(line, sizeof(line), fp) != NULL) {
>                 size_t len = strlen(line);
> @@ -1409,8 +1432,8 @@ static void depmod_modules_sort(struct depmod *depmod)
>                 if (len == 0)
>                         continue;
>                 if (line[len - 1] != '\n') {
> -                       ERR("%s:%u corrupted line misses '\\n'\n",
> -                               order_file, idx);
> +                       ERR("%s/%s:%u corrupted line misses '\\n'\n",
> +                               depmod->cfg->dirname, order_file, idx);
>                         goto corrupted;
>                 }
>         }
> @@ -2287,18 +2310,14 @@ static int output_builtin_bin(struct depmod *depmod, FILE *out)
>  {
>         FILE *in;
>         struct index_node *idx;
> -       char infile[PATH_MAX], line[PATH_MAX], modname[PATH_MAX];
> +       char line[PATH_MAX], modname[PATH_MAX];
>
>         if (out == stdout)
>                 return 0;
>
> -       snprintf(infile, sizeof(infile), "%s/modules.builtin",
> -                                                       depmod->cfg->dirname);
> -       in = fopen(infile, "r");
> -       if (in == NULL) {
> -               WRN("could not open %s: %m\n", infile);
> +       in = dfdopen(depmod->cfg->dirname, "modules.builtin", O_RDONLY, "r");
> +       if (in == NULL)
>                 return 0;
> -       }
>
>         idx = index_create();
>         if (idx == NULL) {
> --
> 2.19.2
>
diff mbox series

Patch

diff --git a/tools/depmod.c b/tools/depmod.c
index 3b6d16e76160..fb76f823c016 100644
--- a/tools/depmod.c
+++ b/tools/depmod.c
@@ -1389,19 +1389,42 @@  static int depmod_modules_build_array(struct depmod *depmod)
 	return 0;
 }
 
+static FILE * dfdopen(const char * dname, const char * filename, int flags, const char * mode)
+	{
+	int fd, dfd;
+	FILE * ret;
+
+	dfd = open(dname, O_RDONLY);
+	if (dfd < 0) {
+		WRN("could not open directory %s: %m\n", dname);
+		return NULL;
+	}
+
+	fd = openat(dfd, filename, flags);
+	if (fd < 0) {
+		WRN("could not open %s at %s: %m\n", filename, dname);
+		ret = NULL;
+	} else {
+		ret = fdopen(fd, mode);
+		if (!ret)
+			WRN("could not associate stream with %s: %m\n", filename);
+	}
+	close(dfd);
+	return ret;
+}
+
+
+
 static void depmod_modules_sort(struct depmod *depmod)
 {
-	char order_file[PATH_MAX], line[PATH_MAX];
+	char line[PATH_MAX];
+	const char * order_file = "modules.order";
 	FILE *fp;
 	unsigned idx = 0, total = 0;
 
-	snprintf(order_file, sizeof(order_file), "%s/modules.order",
-		 depmod->cfg->dirname);
-	fp = fopen(order_file, "r");
-	if (fp == NULL) {
-		WRN("could not open %s: %m\n", order_file);
+	fp = dfdopen(depmod->cfg->dirname, order_file, O_RDONLY, "r");
+	if (fp == NULL)
 		return;
-	}
 
 	while (fgets(line, sizeof(line), fp) != NULL) {
 		size_t len = strlen(line);
@@ -1409,8 +1432,8 @@  static void depmod_modules_sort(struct depmod *depmod)
 		if (len == 0)
 			continue;
 		if (line[len - 1] != '\n') {
-			ERR("%s:%u corrupted line misses '\\n'\n",
-				order_file, idx);
+			ERR("%s/%s:%u corrupted line misses '\\n'\n",
+				depmod->cfg->dirname, order_file, idx);
 			goto corrupted;
 		}
 	}
@@ -2287,18 +2310,14 @@  static int output_builtin_bin(struct depmod *depmod, FILE *out)
 {
 	FILE *in;
 	struct index_node *idx;
-	char infile[PATH_MAX], line[PATH_MAX], modname[PATH_MAX];
+	char line[PATH_MAX], modname[PATH_MAX];
 
 	if (out == stdout)
 		return 0;
 
-	snprintf(infile, sizeof(infile), "%s/modules.builtin",
-							depmod->cfg->dirname);
-	in = fopen(infile, "r");
-	if (in == NULL) {
-		WRN("could not open %s: %m\n", infile);
+	in = dfdopen(depmod->cfg->dirname, "modules.builtin", O_RDONLY, "r");
+	if (in == NULL)
 		return 0;
-	}
 
 	idx = index_create();
 	if (idx == NULL) {