Fix the error in determining the range of file name length

  From: Jiaying Song <jiaying.song.cn@windriver.com>

The check condition `(strlen(input) + 2) > NAME_MAX` in the code is
incorrect because it does not leave enough buffer space. This is
because, in addition to the `tmp_prefix` generated by appending an "_"
symbol to the `input`, the generated temporary files also have a suffix
of `s00000.o`. Therefore, the final filename adds 9 extra bytes to the
`input`. To ensure adequate space, the conditional statement should be
changed to `if ((strlen(input) + 20) > NAME_MAX)`.

Signed-off-by: Jiaying Song <jiaying.song.cn@windriver.com>
---
 binutils/dlltool.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Hi Martin, Alan, and team,

Thank you for your attention to this issue. Based on recent tests, it appears that the check (strlen(input) + 2) > NAME_MAX in the code is incorrect because it does not leave sufficient buffer space. This is because, in addition to the tmp_prefix generated by appending an "_" symbol to input, the temporary files also have a suffix s00000.o. Consequently, the final filename adds 9 extra bytes to the input.
Given this, the conditional statement should be updated to if ((strlen(input) + 20) > NAME_MAX). However, I am unsure if the 20-byte margin is adequate and would appreciate your feedback on this.
Here are the specifics from the tests:

  *   Input length in bytes: 247
  *   Input content: /buildarea1/jsong3/fkewjfnweqdfaewklfnewkfjwkllllllllllkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkwefwefewwkeifewnfiejinelsinelskndiemojief/libqemu_plugin_api.a
  *   Final tmp_prefix: buildarea1_jsong3_fkewjfnweqdfaewklfnewkfjwkllllllllllkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkwefwefewwkeifewnfiejinelsinelskndiemojief_libqemu_plugin_api_a
  *   Length of tmp_prefix: 248

Error encountered: /buildarea1/jsong3/LTS24/testlongfilenametestlongfilenametestlongfilenametestlongfilenametestlongfilenametestlongfilenametestlongfilenametestlongfilenametestlongfilenametestlong/tmp-glibc/work/i686-nativesdk-mingw32-w64-mingw32/nativesdk-qemu/8.2.3/recipe-sysroot-native/usr/bin/i686-w64-mingw32/i686-w64-mingw32-dlltool: bfd_open failed open stub file: _buildarea1_jsong3_fkewjfnweqdfaewklfnewkfjwkllllllllllkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkkwefwefewwkeifewnfiejinelsinelskndiemojief_libqemu_plugin_api_a_s00000.o: File name too long
Thank you for your assistance and feedback.

Best regards,
Jiaying.
  

On 13.09.2024 05:19, jiaying.song.cn@windriver.com wrote:
> From: Jiaying Song <jiaying.song.cn@windriver.com>
> 
> The check condition `(strlen(input) + 2) > NAME_MAX` in the code is
> incorrect because it does not leave enough buffer space. This is
> because, in addition to the `tmp_prefix` generated by appending an "_"
> symbol to the `input`, the generated temporary files also have a suffix
> of `s00000.o`.

I agree 2 is too little for the set of uses of dlltmp(). I'm not sure,
however, whether right here we ought to account for what e.g.
make_one_lib_file() further appends. That also doesn't look to require
18 additional characters in the first place, when we'd really like to
avoid resorting to the alternative naming. So no, ...

> Therefore, the final filename adds 9 extra bytes to the
> `input`. To ensure adequate space, the conditional statement should be
> changed to `if ((strlen(input) + 20) > NAME_MAX)`.

I don't think we should go beyond what's needed in the worst case.

Jan
  

On Fri, Sep 13, 2024 at 03:41:41AM +0000, Song, Jiaying (CN) wrote:
> Hi Martin, Alan, and team,
> 
> Thank you for your attention to this issue. Based on recent tests,
> it appears that the check (strlen(input) + 2) > NAME_MAX in the code
> is incorrect because it does not leave sufficient buffer space. This
> is because, in addition to the tmp_prefix generated by appending an
> "_" symbol to input, the temporary files also have a suffix
> s00000.o. Consequently, the final filename adds 9 extra bytes to the
> input.

Yes.  Would you mind testing the following?

From 8b75913f44537b51a14f771f616deea747d00c76 Mon Sep 17 00:00:00 2001
From: Alan Modra <amodra@gmail.com>
Date: Wed, 25 Sep 2024 10:42:36 +0930
Subject: Re: dlltool: file name too long

Allow for "snnnnn.o" suffix when testing against NAME_MAX, and tidy
TMP_STUB handling by overwrite prior nnnnn.o string rather than
copying the name.

	* dlltool.c (TMP_STUB): Add "nnnnn.o" to format.
	(make_one_lib_file): Localise variables.  Don't copy TMP_STUB,
	overwrite suffix instead.
	(gen_lib_file): Similarly.
	(main): Allow for max suffix when testing against NAME_MAX.

diff --git a/binutils/dlltool.c b/binutils/dlltool.c
index 6dc16a9ed84..01d6cf7d392 100644
--- a/binutils/dlltool.c
+++ b/binutils/dlltool.c
@@ -505,7 +505,7 @@ static char *tmp_stub_buf;
 #define TMP_HEAD_O	dlltmp (&tmp_head_o_buf, "%sh.o")
 #define TMP_TAIL_S	dlltmp (&tmp_tail_s_buf, "%st.s")
 #define TMP_TAIL_O	dlltmp (&tmp_tail_o_buf, "%st.o")
-#define TMP_STUB	dlltmp (&tmp_stub_buf, "%ss")
+#define TMP_STUB	dlltmp (&tmp_stub_buf, "%ssnnnnn.o")
 
 /* This bit of assembly does jmp * ....  */
 static const unsigned char i386_jtab[] =
@@ -2365,26 +2365,11 @@ make_imp_label (const char *prefix, const char *name)
 static bfd *
 make_one_lib_file (export_type *exp, int i, int delay)
 {
-  bfd *      abfd;
-  asymbol *  exp_label;
-  asymbol *  iname = 0;
-  asymbol *  iname2;
-  asymbol *  iname_lab;
-  asymbol ** iname_lab_pp;
-  asymbol ** iname_pp;
-#ifndef EXTRA
-#define EXTRA    0
-#endif
-  asymbol *  ptrs[NSECS + 4 + EXTRA + 1];
-  flagword   applicable;
-  char *     outname = xmalloc (strlen (TMP_STUB) + 10);
-  int        oidx = 0;
-
-
-  sprintf (outname, "%s%05d.o", TMP_STUB, i);
-
-  abfd = bfd_openw (outname, HOW_BFD_WRITE_TARGET);
+  char *outname = TMP_STUB;
+  size_t name_len = strlen (outname);
+  sprintf (outname + name_len - 7, "%05d.o", i);
 
+  bfd *abfd = bfd_openw (outname, HOW_BFD_WRITE_TARGET);
   if (!abfd)
     /* xgettext:c-format */
     fatal (_("bfd_open failed open stub file: %s: %s"),
@@ -2401,9 +2386,13 @@ make_one_lib_file (export_type *exp, int i, int delay)
     bfd_set_private_flags (abfd, F_INTERWORK);
 #endif
 
-  applicable = bfd_applicable_section_flags (abfd);
-
   /* First make symbols for the sections.  */
+  flagword applicable = bfd_applicable_section_flags (abfd);
+#ifndef EXTRA
+#define EXTRA    0
+#endif
+  asymbol *ptrs[NSECS + 4 + EXTRA + 1];
+  int oidx = 0;
   for (i = 0; i < NSECS; i++)
     {
       sinfo *si = secdata + i;
@@ -2430,7 +2419,7 @@ make_one_lib_file (export_type *exp, int i, int delay)
 
   if (! exp->data)
     {
-      exp_label = bfd_make_empty_symbol (abfd);
+      asymbol *exp_label = bfd_make_empty_symbol (abfd);
       exp_label->name = make_imp_label ("", exp->name);
       exp_label->section = secdata[TEXT].sec;
       exp_label->flags = BSF_GLOBAL;
@@ -2446,6 +2435,7 @@ make_one_lib_file (export_type *exp, int i, int delay)
   /* Generate imp symbols with one underscore for Microsoft
      compatibility, and with two underscores for backward
      compatibility with old versions of cygwin.  */
+  asymbol *iname = NULL;
   if (create_compat_implib)
     {
       iname = bfd_make_empty_symbol (abfd);
@@ -2455,25 +2445,24 @@ make_one_lib_file (export_type *exp, int i, int delay)
       iname->value = 0;
     }
 
-  iname2 = bfd_make_empty_symbol (abfd);
+  asymbol *iname2 = bfd_make_empty_symbol (abfd);
   iname2->name = make_imp_label ("__imp_", exp->name);
   iname2->section = secdata[IDATA5].sec;
   iname2->flags = BSF_GLOBAL;
   iname2->value = 0;
 
-  iname_lab = bfd_make_empty_symbol (abfd);
-
+  asymbol *iname_lab = bfd_make_empty_symbol (abfd);
   iname_lab->name = head_label;
   iname_lab->section = bfd_und_section_ptr;
   iname_lab->flags = 0;
   iname_lab->value = 0;
 
-  iname_pp = ptrs + oidx;
+  asymbol **iname_pp = ptrs + oidx;
   if (create_compat_implib)
     ptrs[oidx++] = iname;
   ptrs[oidx++] = iname2;
 
-  iname_lab_pp = ptrs + oidx;
+  asymbol **iname_lab_pp = ptrs + oidx;
   ptrs[oidx++] = iname_lab;
 
   ptrs[oidx] = 0;
@@ -3056,29 +3045,26 @@ gen_lib_file (int delay)
 
   if (dontdeltemps < 2)
     {
-      char *name;
-      size_t stub_len = strlen (TMP_STUB);
+      char *name = TMP_STUB;
+      size_t name_len = strlen (name);
 
-      name = xmalloc (stub_len + 10);
-      memcpy (name, TMP_STUB, stub_len);
       for (i = 0; (exp = d_exports_lexically[i]); i++)
 	{
 	  /* Don't delete non-existent stubs for PRIVATE entries.  */
 	  if (exp->private)
 	    continue;
-	  sprintf (name + stub_len, "%05d.o", i);
+	  sprintf (name + name_len - 7, "%05d.o", i);
 	  if (unlink (name) < 0)
 	    /* xgettext:c-format */
 	    non_fatal (_("cannot delete %s: %s"), name, strerror (errno));
 	  if (ext_prefix_alias)
 	    {
-	      sprintf (name + stub_len, "%05d.o", i + PREFIX_ALIAS_BASE);
+	      sprintf (name + name_len - 7, "%05d.o", i + PREFIX_ALIAS_BASE);
 	      if (unlink (name) < 0)
 		/* xgettext:c-format */
 		non_fatal (_("cannot delete %s: %s"), name, strerror (errno));
 	    }
 	}
-      free (name);
     }
 
   inform (_("Created lib file"));
@@ -4069,7 +4055,7 @@ main (int ac, char **av)
     {
       /* If possible use a deterministic prefix.  */
       const char *input = imp_name ? imp_name : delayimp_name;
-      if (input && strlen (input) + 2 <= NAME_MAX)
+      if (input && strlen (input) + sizeof ("_snnnnn.o") - 1 <= NAME_MAX)
 	{
 	  tmp_prefix = xmalloc (strlen (input) + 2);
 	  sprintf (tmp_prefix, "%s_", input);