All of lore.kernel.org
 help / color / mirror / Atom feed
* [hardknott][PATCH] Binutils: Fix CVE-2021-20197
@ 2021-04-29 11:30 Vinay Kumar
  2021-05-05 11:32 ` Vinay Kumar
  2021-05-06  1:57 ` Anuj Mittal
  0 siblings, 2 replies; 18+ messages in thread
From: Vinay Kumar @ 2021-04-29 11:30 UTC (permalink / raw)
  To: openembedded-core; +Cc: vinay.kumar, rwmacleod, umesh.kalappa0, Vinay Kumar

Source: git://sourceware.org/git/binutils-gdb.git
Tracking -- https://sourceware.org/bugzilla/show_bug.cgi?id=26945

Backported upstream commit d3edaa91d4cf7202ec14342410194841e2f67f12 to
binutils-2.36 source along with commit id dependencies
(8e03235147a9e774d3ba084e93c2da1aa94d1cec and 8b69e61d4be276bb862698aaafddc3e779d23c8f).

Upstream-Status: Backport [https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d3edaa91d4cf7202ec14342410194841e2f67f12]

Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
---
This patch is verified by building both Linux and Mingw toolchains for ARM target, and executed "binutils" regresssion testing.

 .../binutils/binutils-2.36.inc                |   1 +
 .../binutils/binutils/CVE-2021-20197.patch    | 388 ++++++++++++++++++
 2 files changed, 389 insertions(+)
 create mode 100644 meta/recipes-devtools/binutils/binutils/CVE-2021-20197.patch

diff --git a/meta/recipes-devtools/binutils/binutils-2.36.inc b/meta/recipes-devtools/binutils/binutils-2.36.inc
index beddf601fd..66080dfcee 100644
--- a/meta/recipes-devtools/binutils/binutils-2.36.inc
+++ b/meta/recipes-devtools/binutils/binutils-2.36.inc
@@ -36,5 +36,6 @@ SRC_URI = "\
      file://0015-sync-with-OE-libtool-changes.patch \
      file://0016-Check-for-clang-before-checking-gcc-version.patch \
      file://0017-Add-support-for-the-DW_FORM_strx-forms-to-the-BFD-li.patch \
+     file://CVE-2021-20197.patch \
 "
 S  = "${WORKDIR}/git"
diff --git a/meta/recipes-devtools/binutils/binutils/CVE-2021-20197.patch b/meta/recipes-devtools/binutils/binutils/CVE-2021-20197.patch
new file mode 100644
index 0000000000..d6117b65a4
--- /dev/null
+++ b/meta/recipes-devtools/binutils/binutils/CVE-2021-20197.patch
@@ -0,0 +1,388 @@
+From d3edaa91d4cf7202ec14342410194841e2f67f12 Mon Sep 17 00:00:00 2001
+From: Alan Modra <amodra@gmail.com>
+Date: Fri, 26 Feb 2021 11:30:32 +1030
+Subject: [PATCH] Reinstate various pieces backed out from smart_rename changes
+
+In the interests of a stable release various last minute smart_rename
+patches were backed out of the 2.36 branch.  The main reason to
+reinstate some of those backed out changes here is to make necessary
+followup fixes to commit 8e03235147a9 simple cherry-picks from
+mainline.  A secondary reason is that ar -M support isn't fixed for
+pr26945 without this patch.
+
+        PR 26945
+        * ar.c: Don't include libbfd.h.
+        (write_archive): Replace xmalloc+strcpy with xstrdup.
+        * arsup.c (temp_name, real_ofd): New static variables.
+        (ar_open): Use make_tempname and bfd_fdopenw.
+        (ar_save): Adjust to suit ar_open changes.
+        * objcopy.c: Don't include libbfd.h.
+        * rename.c: Rename and reorder variables.
+
+(cherry picked from commit 95b91a043aeaeb546d2fea556d84a2de1e917770)
+
+Upstream-Status: Backport [https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d3edaa91d4cf7202ec14342410194841e2f67f12]
+CVE: CVE-2021-20197
+Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
+---
+ binutils/ar.c      |   6 +-
+ binutils/arsup.c   |  37 ++++++++----
+ binutils/bucomm.h  |   3 +-
+ binutils/objcopy.c |   9 +--
+ binutils/rename.c  | 148 +++++++++++----------------------------------
+ 5 files changed, 67 insertions(+), 136 deletions(-)
+
+diff --git a/binutils/ar.c b/binutils/ar.c
+index 45a34e3a6cf..44df48c5c67 100644
+--- a/binutils/ar.c
++++ b/binutils/ar.c
+@@ -25,7 +25,6 @@
+ 
+ #include "sysdep.h"
+ #include "bfd.h"
+-#include "libbfd.h"
+ #include "libiberty.h"
+ #include "progress.h"
+ #include "getopt.h"
+@@ -1255,8 +1254,7 @@ write_archive (bfd *iarch)
+   bfd *contents_head = iarch->archive_next;
+   int ofd = -1;
+ 
+-  old_name = (char *) xmalloc (strlen (bfd_get_filename (iarch)) + 1);
+-  strcpy (old_name, bfd_get_filename (iarch));
++  old_name = xstrdup (bfd_get_filename (iarch));
+   new_name = make_tempname (old_name, &ofd);
+ 
+   if (new_name == NULL)
+@@ -1308,7 +1306,7 @@ write_archive (bfd *iarch)
+   /* We don't care if this fails; we might be creating the archive.  */
+   bfd_close (iarch);
+ 
+-  if (smart_rename (new_name, old_name, 0) != 0)
++  if (smart_rename (new_name, old_name, NULL) != 0)
+     xexit (1);
+   free (old_name);
+   free (new_name);
+diff --git a/binutils/arsup.c b/binutils/arsup.c
+index 5403a0c5d74..f7ce8f0bc82 100644
+--- a/binutils/arsup.c
++++ b/binutils/arsup.c
+@@ -42,6 +42,8 @@ extern int deterministic;
+ 
+ static bfd *obfd;
+ static char *real_name;
++static char *temp_name;
++static int real_ofd;
+ static FILE *outfile;
+ 
+ static void
+@@ -149,27 +151,24 @@ maybequit (void)
+ void
+ ar_open (char *name, int t)
+ {
+-  char *tname;
+-  const char *bname = lbasename (name);
+-  real_name = name;
++  real_name = xstrdup (name);
++  temp_name = make_tempname (real_name, &real_ofd);
+ 
+-  /* Prepend tmp- to the beginning, to avoid file-name clashes after
+-     truncation on filesystems with limited namespaces (DOS).  */
+-  if (asprintf (&tname, "%.*stmp-%s", (int) (bname - name), name, bname) == -1)
++  if (temp_name == NULL)
+     {
+-      fprintf (stderr, _("%s: Can't allocate memory for temp name (%s)\n"),
++      fprintf (stderr, _("%s: Can't open temporary file (%s)\n"),
+ 	       program_name, strerror(errno));
+       maybequit ();
+       return;
+     }
+ 
+-  obfd = bfd_openw (tname, NULL);
++  obfd = bfd_fdopenw (temp_name, NULL, real_ofd);
+ 
+   if (!obfd)
+     {
+       fprintf (stderr,
+ 	       _("%s: Can't open output archive %s\n"),
+-	       program_name,  tname);
++	       program_name, temp_name);
+ 
+       maybequit ();
+     }
+@@ -344,16 +343,30 @@ ar_save (void)
+     }
+   else
+     {
+-      char *ofilename = xstrdup (bfd_get_filename (obfd));
++      struct stat target_stat;
+ 
+       if (deterministic > 0)
+         obfd->flags |= BFD_DETERMINISTIC_OUTPUT;
+ 
+       bfd_close (obfd);
+ 
+-      smart_rename (ofilename, real_name, 0);
++      if (stat (real_name, &target_stat) != 0)
++	{
++	  /* The temp file created in ar_open has mode 0600 as per mkstemp.
++	     Create the real empty output file here so smart_rename will
++	     update the mode according to the process umask.  */
++	  obfd = bfd_openw (real_name, NULL);
++	  if (obfd != NULL)
++	    {
++	      bfd_set_format (obfd, bfd_archive);
++	      bfd_close (obfd);
++	    }
++	}
++
++      smart_rename (temp_name, real_name, NULL);
+       obfd = 0;
+-      free (ofilename);
++      free (temp_name);
++      free (real_name);
+     }
+ }
+ 
+diff --git a/binutils/bucomm.h b/binutils/bucomm.h
+index 91f6a5b228f..aa7e33d8cd1 100644
+--- a/binutils/bucomm.h
++++ b/binutils/bucomm.h
+@@ -71,7 +71,8 @@ extern void print_version (const char *);
+ /* In rename.c.  */
+ extern void set_times (const char *, const struct stat *);
+ 
+-extern int smart_rename (const char *, const char *, int);
++extern int smart_rename (const char *, const char *, struct stat *);
++
+ 
+ /* In libiberty.  */
+ void *xmalloc (size_t);
+diff --git a/binutils/objcopy.c b/binutils/objcopy.c
+index eab3b6db585..73aa8bc2514 100644
+--- a/binutils/objcopy.c
++++ b/binutils/objcopy.c
+@@ -20,7 +20,6 @@
+ \f
+ #include "sysdep.h"
+ #include "bfd.h"
+-#include "libbfd.h"
+ #include "progress.h"
+ #include "getopt.h"
+ #include "libiberty.h"
+@@ -4861,12 +4860,10 @@ strip_main (int argc, char *argv[])
+ 		 output_target, NULL);
+       if (status == 0)
+ 	{
+-	  if (preserve_dates)
+-	    set_times (tmpname, &statbuf);
+ 	  if (output_file != tmpname)
+ 	    status = (smart_rename (tmpname,
+ 				    output_file ? output_file : argv[i],
+-				    preserve_dates) != 0);
++				    preserve_dates ? &statbuf : NULL) != 0);
+ 	  if (status == 0)
+ 	    status = hold_status;
+ 	}
+@@ -5931,11 +5928,9 @@ copy_main (int argc, char *argv[])
+ 	     output_target, input_arch);
+   if (status == 0)
+     {
+-      if (preserve_dates)
+-	set_times (tmpname, &statbuf);
+       if (tmpname != output_filename)
+ 	status = (smart_rename (tmpname, input_filename,
+-				preserve_dates) != 0);
++				preserve_dates ? &statbuf : NULL) != 0);
+     }
+   else
+     unlink_if_ordinary (tmpname);
+diff --git a/binutils/rename.c b/binutils/rename.c
+index 65ad5bf52c4..72a9323d72c 100644
+--- a/binutils/rename.c
++++ b/binutils/rename.c
+@@ -24,14 +24,9 @@
+ 
+ #ifdef HAVE_GOOD_UTIME_H
+ #include <utime.h>
+-#else /* ! HAVE_GOOD_UTIME_H */
+-#ifdef HAVE_UTIMES
++#elif defined HAVE_UTIMES
+ #include <sys/time.h>
+-#endif /* HAVE_UTIMES */
+-#endif /* ! HAVE_GOOD_UTIME_H */
+-
+-#if ! defined (_WIN32) || defined (__CYGWIN32__)
+-static int simple_copy (const char *, const char *);
++#endif
+ 
+ /* The number of bytes to copy at once.  */
+ #define COPY_BUF 8192
+@@ -82,7 +77,6 @@ simple_copy (const char *from, const char *to)
+     }
+   return 0;
+ }
+-#endif /* __CYGWIN32__ or not _WIN32 */
+ 
+ /* Set the times of the file DESTINATION to be the same as those in
+    STATBUF.  */
+@@ -91,122 +85,52 @@ void
+ set_times (const char *destination, const struct stat *statbuf)
+ {
+   int result;
+-
+-  {
+ #ifdef HAVE_GOOD_UTIME_H
+-    struct utimbuf tb;
+-
+-    tb.actime = statbuf->st_atime;
+-    tb.modtime = statbuf->st_mtime;
+-    result = utime (destination, &tb);
+-#else /* ! HAVE_GOOD_UTIME_H */
+-#ifndef HAVE_UTIMES
+-    long tb[2];
+-
+-    tb[0] = statbuf->st_atime;
+-    tb[1] = statbuf->st_mtime;
+-    result = utime (destination, tb);
+-#else /* HAVE_UTIMES */
+-    struct timeval tv[2];
+-
+-    tv[0].tv_sec = statbuf->st_atime;
+-    tv[0].tv_usec = 0;
+-    tv[1].tv_sec = statbuf->st_mtime;
+-    tv[1].tv_usec = 0;
+-    result = utimes (destination, tv);
+-#endif /* HAVE_UTIMES */
+-#endif /* ! HAVE_GOOD_UTIME_H */
+-  }
++  struct utimbuf tb;
++
++  tb.actime = statbuf->st_atime;
++  tb.modtime = statbuf->st_mtime;
++  result = utime (destination, &tb);
++#elif defined HAVE_UTIMES
++  struct timeval tv[2];
++
++  tv[0].tv_sec = statbuf->st_atime;
++  tv[0].tv_usec = 0;
++  tv[1].tv_sec = statbuf->st_mtime;
++  tv[1].tv_usec = 0;
++  result = utimes (destination, tv);
++#else
++  long tb[2];
++
++  tb[0] = statbuf->st_atime;
++  tb[1] = statbuf->st_mtime;
++  result = utime (destination, tb);
++#endif
+ 
+   if (result != 0)
+     non_fatal (_("%s: cannot set time: %s"), destination, strerror (errno));
+ }
+ 
+-#ifndef S_ISLNK
+-#ifdef S_IFLNK
+-#define S_ISLNK(m) (((m) & S_IFMT) == S_IFLNK)
+-#else
+-#define S_ISLNK(m) 0
+-#define lstat stat
+-#endif
+-#endif
+-
+-/* Rename FROM to TO, copying if TO is a link.
+-   Return 0 if ok, -1 if error.  */
++/* Copy FROM to TO.  TARGET_STAT has the file status that, if non-NULL,
++   is used to fix up timestamps.  Return 0 if ok, -1 if error.
++   At one time this function renamed files, but file permissions are
++   tricky to update given the number of different schemes used by
++   various systems.  So now we just copy.  */
+ 
+ int
+-smart_rename (const char *from, const char *to, int preserve_dates ATTRIBUTE_UNUSED)
++smart_rename (const char *from, const char *to,
++	      struct stat *target_stat)
+ {
+-  bfd_boolean exists;
+-  struct stat s;
+-  int ret = 0;
+-
+-  exists = lstat (to, &s) == 0;
+-
+-#if defined (_WIN32) && !defined (__CYGWIN32__)
+-  /* Win32, unlike unix, will not erase `to' in `rename(from, to)' but
+-     fail instead.  Also, chown is not present.  */
++  int ret;
+ 
+-  if (exists)
+-    remove (to);
+-
+-  ret = rename (from, to);
++  ret = simple_copy (from, to);
+   if (ret != 0)
+-    {
+-      /* We have to clean up here.  */
+-      non_fatal (_("unable to rename '%s'; reason: %s"), to, strerror (errno));
+-      unlink (from);
+-    }
+-#else
+-  /* Use rename only if TO is not a symbolic link and has
+-     only one hard link, and we have permission to write to it.  */
+-  if (! exists
+-      || (!S_ISLNK (s.st_mode)
+-	  && S_ISREG (s.st_mode)
+-	  && (s.st_mode & S_IWUSR)
+-	  && s.st_nlink == 1)
+-      )
+-    {
+-      ret = rename (from, to);
+-      if (ret == 0)
+-	{
+-	  if (exists)
+-	    {
+-	      /* Try to preserve the permission bits and ownership of
+-		 TO.  First get the mode right except for the setuid
+-		 bit.  Then change the ownership.  Then fix the setuid
+-		 bit.  We do the chmod before the chown because if the
+-		 chown succeeds, and we are a normal user, we won't be
+-		 able to do the chmod afterward.  We don't bother to
+-		 fix the setuid bit first because that might introduce
+-		 a fleeting security problem, and because the chown
+-		 will clear the setuid bit anyhow.  We only fix the
+-		 setuid bit if the chown succeeds, because we don't
+-		 want to introduce an unexpected setuid file owned by
+-		 the user running objcopy.  */
+-	      chmod (to, s.st_mode & 0777);
+-	      if (chown (to, s.st_uid, s.st_gid) >= 0)
+-		chmod (to, s.st_mode & 07777);
+-	    }
+-	}
+-      else
+-	{
+-	  /* We have to clean up here.  */
+-	  non_fatal (_("unable to rename '%s'; reason: %s"), to, strerror (errno));
+-	  unlink (from);
+-	}
+-    }
+-  else
+-    {
+-      ret = simple_copy (from, to);
+-      if (ret != 0)
+-	non_fatal (_("unable to copy file '%s'; reason: %s"), to, strerror (errno));
++    non_fatal (_("unable to copy file '%s'; reason: %s"),
++	       to, strerror (errno));
+ 
+-      if (preserve_dates)
+-	set_times (to, &s);
+-      unlink (from);
+-    }
+-#endif /* _WIN32 && !__CYGWIN32__ */
++  if (target_stat != NULL)
++    set_times (to, target_stat);
++  unlink (from);
+ 
+   return ret;
+ }
+-- 
+2.17.1
+
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [hardknott][PATCH] Binutils: Fix CVE-2021-20197
  2021-04-29 11:30 [hardknott][PATCH] Binutils: Fix CVE-2021-20197 Vinay Kumar
@ 2021-05-05 11:32 ` Vinay Kumar
  2021-05-05 13:21   ` [OE-core] " Richard Purdie
  2021-05-06  1:57 ` Anuj Mittal
  1 sibling, 1 reply; 18+ messages in thread
From: Vinay Kumar @ 2021-05-05 11:32 UTC (permalink / raw)
  To: Steve Sakoman
  Cc: vinay.kumar, Randy MacLeod, umesh kalappa0,
	Patches and discussions about the oe-core layer

Hi Steve,

Any update on considering this patch in the "hardknott" branch ?

Regards,
Vinay

On Thu, Apr 29, 2021 at 5:01 PM Vinay Kumar <vinay.m.engg@gmail.com> wrote:
>
> Source: git://sourceware.org/git/binutils-gdb.git
> Tracking -- https://sourceware.org/bugzilla/show_bug.cgi?id=26945
>
> Backported upstream commit d3edaa91d4cf7202ec14342410194841e2f67f12 to
> binutils-2.36 source along with commit id dependencies
> (8e03235147a9e774d3ba084e93c2da1aa94d1cec and 8b69e61d4be276bb862698aaafddc3e779d23c8f).
>
> Upstream-Status: Backport [https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d3edaa91d4cf7202ec14342410194841e2f67f12]
>
> Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
> ---
> This patch is verified by building both Linux and Mingw toolchains for ARM target, and executed "binutils" regresssion testing.
>
>  .../binutils/binutils-2.36.inc                |   1 +
>  .../binutils/binutils/CVE-2021-20197.patch    | 388 ++++++++++++++++++
>  2 files changed, 389 insertions(+)
>  create mode 100644 meta/recipes-devtools/binutils/binutils/CVE-2021-20197.patch
>
> diff --git a/meta/recipes-devtools/binutils/binutils-2.36.inc b/meta/recipes-devtools/binutils/binutils-2.36.inc
> index beddf601fd..66080dfcee 100644
> --- a/meta/recipes-devtools/binutils/binutils-2.36.inc
> +++ b/meta/recipes-devtools/binutils/binutils-2.36.inc
> @@ -36,5 +36,6 @@ SRC_URI = "\
>       file://0015-sync-with-OE-libtool-changes.patch \
>       file://0016-Check-for-clang-before-checking-gcc-version.patch \
>       file://0017-Add-support-for-the-DW_FORM_strx-forms-to-the-BFD-li.patch \
> +     file://CVE-2021-20197.patch \
>  "
>  S  = "${WORKDIR}/git"
> diff --git a/meta/recipes-devtools/binutils/binutils/CVE-2021-20197.patch b/meta/recipes-devtools/binutils/binutils/CVE-2021-20197.patch
> new file mode 100644
> index 0000000000..d6117b65a4
> --- /dev/null
> +++ b/meta/recipes-devtools/binutils/binutils/CVE-2021-20197.patch
> @@ -0,0 +1,388 @@
> +From d3edaa91d4cf7202ec14342410194841e2f67f12 Mon Sep 17 00:00:00 2001
> +From: Alan Modra <amodra@gmail.com>
> +Date: Fri, 26 Feb 2021 11:30:32 +1030
> +Subject: [PATCH] Reinstate various pieces backed out from smart_rename changes
> +
> +In the interests of a stable release various last minute smart_rename
> +patches were backed out of the 2.36 branch.  The main reason to
> +reinstate some of those backed out changes here is to make necessary
> +followup fixes to commit 8e03235147a9 simple cherry-picks from
> +mainline.  A secondary reason is that ar -M support isn't fixed for
> +pr26945 without this patch.
> +
> +        PR 26945
> +        * ar.c: Don't include libbfd.h.
> +        (write_archive): Replace xmalloc+strcpy with xstrdup.
> +        * arsup.c (temp_name, real_ofd): New static variables.
> +        (ar_open): Use make_tempname and bfd_fdopenw.
> +        (ar_save): Adjust to suit ar_open changes.
> +        * objcopy.c: Don't include libbfd.h.
> +        * rename.c: Rename and reorder variables.
> +
> +(cherry picked from commit 95b91a043aeaeb546d2fea556d84a2de1e917770)
> +
> +Upstream-Status: Backport [https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d3edaa91d4cf7202ec14342410194841e2f67f12]
> +CVE: CVE-2021-20197
> +Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
> +---
> + binutils/ar.c      |   6 +-
> + binutils/arsup.c   |  37 ++++++++----
> + binutils/bucomm.h  |   3 +-
> + binutils/objcopy.c |   9 +--
> + binutils/rename.c  | 148 +++++++++++----------------------------------
> + 5 files changed, 67 insertions(+), 136 deletions(-)
> +
> +diff --git a/binutils/ar.c b/binutils/ar.c
> +index 45a34e3a6cf..44df48c5c67 100644
> +--- a/binutils/ar.c
> ++++ b/binutils/ar.c
> +@@ -25,7 +25,6 @@
> +
> + #include "sysdep.h"
> + #include "bfd.h"
> +-#include "libbfd.h"
> + #include "libiberty.h"
> + #include "progress.h"
> + #include "getopt.h"
> +@@ -1255,8 +1254,7 @@ write_archive (bfd *iarch)
> +   bfd *contents_head = iarch->archive_next;
> +   int ofd = -1;
> +
> +-  old_name = (char *) xmalloc (strlen (bfd_get_filename (iarch)) + 1);
> +-  strcpy (old_name, bfd_get_filename (iarch));
> ++  old_name = xstrdup (bfd_get_filename (iarch));
> +   new_name = make_tempname (old_name, &ofd);
> +
> +   if (new_name == NULL)
> +@@ -1308,7 +1306,7 @@ write_archive (bfd *iarch)
> +   /* We don't care if this fails; we might be creating the archive.  */
> +   bfd_close (iarch);
> +
> +-  if (smart_rename (new_name, old_name, 0) != 0)
> ++  if (smart_rename (new_name, old_name, NULL) != 0)
> +     xexit (1);
> +   free (old_name);
> +   free (new_name);
> +diff --git a/binutils/arsup.c b/binutils/arsup.c
> +index 5403a0c5d74..f7ce8f0bc82 100644
> +--- a/binutils/arsup.c
> ++++ b/binutils/arsup.c
> +@@ -42,6 +42,8 @@ extern int deterministic;
> +
> + static bfd *obfd;
> + static char *real_name;
> ++static char *temp_name;
> ++static int real_ofd;
> + static FILE *outfile;
> +
> + static void
> +@@ -149,27 +151,24 @@ maybequit (void)
> + void
> + ar_open (char *name, int t)
> + {
> +-  char *tname;
> +-  const char *bname = lbasename (name);
> +-  real_name = name;
> ++  real_name = xstrdup (name);
> ++  temp_name = make_tempname (real_name, &real_ofd);
> +
> +-  /* Prepend tmp- to the beginning, to avoid file-name clashes after
> +-     truncation on filesystems with limited namespaces (DOS).  */
> +-  if (asprintf (&tname, "%.*stmp-%s", (int) (bname - name), name, bname) == -1)
> ++  if (temp_name == NULL)
> +     {
> +-      fprintf (stderr, _("%s: Can't allocate memory for temp name (%s)\n"),
> ++      fprintf (stderr, _("%s: Can't open temporary file (%s)\n"),
> +              program_name, strerror(errno));
> +       maybequit ();
> +       return;
> +     }
> +
> +-  obfd = bfd_openw (tname, NULL);
> ++  obfd = bfd_fdopenw (temp_name, NULL, real_ofd);
> +
> +   if (!obfd)
> +     {
> +       fprintf (stderr,
> +              _("%s: Can't open output archive %s\n"),
> +-             program_name,  tname);
> ++             program_name, temp_name);
> +
> +       maybequit ();
> +     }
> +@@ -344,16 +343,30 @@ ar_save (void)
> +     }
> +   else
> +     {
> +-      char *ofilename = xstrdup (bfd_get_filename (obfd));
> ++      struct stat target_stat;
> +
> +       if (deterministic > 0)
> +         obfd->flags |= BFD_DETERMINISTIC_OUTPUT;
> +
> +       bfd_close (obfd);
> +
> +-      smart_rename (ofilename, real_name, 0);
> ++      if (stat (real_name, &target_stat) != 0)
> ++      {
> ++        /* The temp file created in ar_open has mode 0600 as per mkstemp.
> ++           Create the real empty output file here so smart_rename will
> ++           update the mode according to the process umask.  */
> ++        obfd = bfd_openw (real_name, NULL);
> ++        if (obfd != NULL)
> ++          {
> ++            bfd_set_format (obfd, bfd_archive);
> ++            bfd_close (obfd);
> ++          }
> ++      }
> ++
> ++      smart_rename (temp_name, real_name, NULL);
> +       obfd = 0;
> +-      free (ofilename);
> ++      free (temp_name);
> ++      free (real_name);
> +     }
> + }
> +
> +diff --git a/binutils/bucomm.h b/binutils/bucomm.h
> +index 91f6a5b228f..aa7e33d8cd1 100644
> +--- a/binutils/bucomm.h
> ++++ b/binutils/bucomm.h
> +@@ -71,7 +71,8 @@ extern void print_version (const char *);
> + /* In rename.c.  */
> + extern void set_times (const char *, const struct stat *);
> +
> +-extern int smart_rename (const char *, const char *, int);
> ++extern int smart_rename (const char *, const char *, struct stat *);
> ++
> +
> + /* In libiberty.  */
> + void *xmalloc (size_t);
> +diff --git a/binutils/objcopy.c b/binutils/objcopy.c
> +index eab3b6db585..73aa8bc2514 100644
> +--- a/binutils/objcopy.c
> ++++ b/binutils/objcopy.c
> +@@ -20,7 +20,6 @@
> +
> + #include "sysdep.h"
> + #include "bfd.h"
> +-#include "libbfd.h"
> + #include "progress.h"
> + #include "getopt.h"
> + #include "libiberty.h"
> +@@ -4861,12 +4860,10 @@ strip_main (int argc, char *argv[])
> +                output_target, NULL);
> +       if (status == 0)
> +       {
> +-        if (preserve_dates)
> +-          set_times (tmpname, &statbuf);
> +         if (output_file != tmpname)
> +           status = (smart_rename (tmpname,
> +                                   output_file ? output_file : argv[i],
> +-                                  preserve_dates) != 0);
> ++                                  preserve_dates ? &statbuf : NULL) != 0);
> +         if (status == 0)
> +           status = hold_status;
> +       }
> +@@ -5931,11 +5928,9 @@ copy_main (int argc, char *argv[])
> +            output_target, input_arch);
> +   if (status == 0)
> +     {
> +-      if (preserve_dates)
> +-      set_times (tmpname, &statbuf);
> +       if (tmpname != output_filename)
> +       status = (smart_rename (tmpname, input_filename,
> +-                              preserve_dates) != 0);
> ++                              preserve_dates ? &statbuf : NULL) != 0);
> +     }
> +   else
> +     unlink_if_ordinary (tmpname);
> +diff --git a/binutils/rename.c b/binutils/rename.c
> +index 65ad5bf52c4..72a9323d72c 100644
> +--- a/binutils/rename.c
> ++++ b/binutils/rename.c
> +@@ -24,14 +24,9 @@
> +
> + #ifdef HAVE_GOOD_UTIME_H
> + #include <utime.h>
> +-#else /* ! HAVE_GOOD_UTIME_H */
> +-#ifdef HAVE_UTIMES
> ++#elif defined HAVE_UTIMES
> + #include <sys/time.h>
> +-#endif /* HAVE_UTIMES */
> +-#endif /* ! HAVE_GOOD_UTIME_H */
> +-
> +-#if ! defined (_WIN32) || defined (__CYGWIN32__)
> +-static int simple_copy (const char *, const char *);
> ++#endif
> +
> + /* The number of bytes to copy at once.  */
> + #define COPY_BUF 8192
> +@@ -82,7 +77,6 @@ simple_copy (const char *from, const char *to)
> +     }
> +   return 0;
> + }
> +-#endif /* __CYGWIN32__ or not _WIN32 */
> +
> + /* Set the times of the file DESTINATION to be the same as those in
> +    STATBUF.  */
> +@@ -91,122 +85,52 @@ void
> + set_times (const char *destination, const struct stat *statbuf)
> + {
> +   int result;
> +-
> +-  {
> + #ifdef HAVE_GOOD_UTIME_H
> +-    struct utimbuf tb;
> +-
> +-    tb.actime = statbuf->st_atime;
> +-    tb.modtime = statbuf->st_mtime;
> +-    result = utime (destination, &tb);
> +-#else /* ! HAVE_GOOD_UTIME_H */
> +-#ifndef HAVE_UTIMES
> +-    long tb[2];
> +-
> +-    tb[0] = statbuf->st_atime;
> +-    tb[1] = statbuf->st_mtime;
> +-    result = utime (destination, tb);
> +-#else /* HAVE_UTIMES */
> +-    struct timeval tv[2];
> +-
> +-    tv[0].tv_sec = statbuf->st_atime;
> +-    tv[0].tv_usec = 0;
> +-    tv[1].tv_sec = statbuf->st_mtime;
> +-    tv[1].tv_usec = 0;
> +-    result = utimes (destination, tv);
> +-#endif /* HAVE_UTIMES */
> +-#endif /* ! HAVE_GOOD_UTIME_H */
> +-  }
> ++  struct utimbuf tb;
> ++
> ++  tb.actime = statbuf->st_atime;
> ++  tb.modtime = statbuf->st_mtime;
> ++  result = utime (destination, &tb);
> ++#elif defined HAVE_UTIMES
> ++  struct timeval tv[2];
> ++
> ++  tv[0].tv_sec = statbuf->st_atime;
> ++  tv[0].tv_usec = 0;
> ++  tv[1].tv_sec = statbuf->st_mtime;
> ++  tv[1].tv_usec = 0;
> ++  result = utimes (destination, tv);
> ++#else
> ++  long tb[2];
> ++
> ++  tb[0] = statbuf->st_atime;
> ++  tb[1] = statbuf->st_mtime;
> ++  result = utime (destination, tb);
> ++#endif
> +
> +   if (result != 0)
> +     non_fatal (_("%s: cannot set time: %s"), destination, strerror (errno));
> + }
> +
> +-#ifndef S_ISLNK
> +-#ifdef S_IFLNK
> +-#define S_ISLNK(m) (((m) & S_IFMT) == S_IFLNK)
> +-#else
> +-#define S_ISLNK(m) 0
> +-#define lstat stat
> +-#endif
> +-#endif
> +-
> +-/* Rename FROM to TO, copying if TO is a link.
> +-   Return 0 if ok, -1 if error.  */
> ++/* Copy FROM to TO.  TARGET_STAT has the file status that, if non-NULL,
> ++   is used to fix up timestamps.  Return 0 if ok, -1 if error.
> ++   At one time this function renamed files, but file permissions are
> ++   tricky to update given the number of different schemes used by
> ++   various systems.  So now we just copy.  */
> +
> + int
> +-smart_rename (const char *from, const char *to, int preserve_dates ATTRIBUTE_UNUSED)
> ++smart_rename (const char *from, const char *to,
> ++            struct stat *target_stat)
> + {
> +-  bfd_boolean exists;
> +-  struct stat s;
> +-  int ret = 0;
> +-
> +-  exists = lstat (to, &s) == 0;
> +-
> +-#if defined (_WIN32) && !defined (__CYGWIN32__)
> +-  /* Win32, unlike unix, will not erase `to' in `rename(from, to)' but
> +-     fail instead.  Also, chown is not present.  */
> ++  int ret;
> +
> +-  if (exists)
> +-    remove (to);
> +-
> +-  ret = rename (from, to);
> ++  ret = simple_copy (from, to);
> +   if (ret != 0)
> +-    {
> +-      /* We have to clean up here.  */
> +-      non_fatal (_("unable to rename '%s'; reason: %s"), to, strerror (errno));
> +-      unlink (from);
> +-    }
> +-#else
> +-  /* Use rename only if TO is not a symbolic link and has
> +-     only one hard link, and we have permission to write to it.  */
> +-  if (! exists
> +-      || (!S_ISLNK (s.st_mode)
> +-        && S_ISREG (s.st_mode)
> +-        && (s.st_mode & S_IWUSR)
> +-        && s.st_nlink == 1)
> +-      )
> +-    {
> +-      ret = rename (from, to);
> +-      if (ret == 0)
> +-      {
> +-        if (exists)
> +-          {
> +-            /* Try to preserve the permission bits and ownership of
> +-               TO.  First get the mode right except for the setuid
> +-               bit.  Then change the ownership.  Then fix the setuid
> +-               bit.  We do the chmod before the chown because if the
> +-               chown succeeds, and we are a normal user, we won't be
> +-               able to do the chmod afterward.  We don't bother to
> +-               fix the setuid bit first because that might introduce
> +-               a fleeting security problem, and because the chown
> +-               will clear the setuid bit anyhow.  We only fix the
> +-               setuid bit if the chown succeeds, because we don't
> +-               want to introduce an unexpected setuid file owned by
> +-               the user running objcopy.  */
> +-            chmod (to, s.st_mode & 0777);
> +-            if (chown (to, s.st_uid, s.st_gid) >= 0)
> +-              chmod (to, s.st_mode & 07777);
> +-          }
> +-      }
> +-      else
> +-      {
> +-        /* We have to clean up here.  */
> +-        non_fatal (_("unable to rename '%s'; reason: %s"), to, strerror (errno));
> +-        unlink (from);
> +-      }
> +-    }
> +-  else
> +-    {
> +-      ret = simple_copy (from, to);
> +-      if (ret != 0)
> +-      non_fatal (_("unable to copy file '%s'; reason: %s"), to, strerror (errno));
> ++    non_fatal (_("unable to copy file '%s'; reason: %s"),
> ++             to, strerror (errno));
> +
> +-      if (preserve_dates)
> +-      set_times (to, &s);
> +-      unlink (from);
> +-    }
> +-#endif /* _WIN32 && !__CYGWIN32__ */
> ++  if (target_stat != NULL)
> ++    set_times (to, target_stat);
> ++  unlink (from);
> +
> +   return ret;
> + }
> +--
> +2.17.1
> +
> --
> 2.17.1
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [OE-core] [hardknott][PATCH] Binutils: Fix CVE-2021-20197
  2021-05-05 11:32 ` Vinay Kumar
@ 2021-05-05 13:21   ` Richard Purdie
  2021-05-05 14:03     ` Vinay Kumar
  0 siblings, 1 reply; 18+ messages in thread
From: Richard Purdie @ 2021-05-05 13:21 UTC (permalink / raw)
  To: Vinay Kumar, Steve Sakoman, Mittal, Anuj
  Cc: vinay.kumar, Randy MacLeod, umesh kalappa0,
	Patches and discussions about the oe-core layer

On Wed, 2021-05-05 at 17:02 +0530, Vinay Kumar wrote:
> Hi Steve,
> 
> Any update on considering this patch in the "hardknott" branch ?

Anuj is maintaining hardknott, not Steve. Also, doesn't this patch
need to be applied to master first before we can backport it?

Cheers,

RIchard


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [OE-core] [hardknott][PATCH] Binutils: Fix CVE-2021-20197
  2021-05-05 13:21   ` [OE-core] " Richard Purdie
@ 2021-05-05 14:03     ` Vinay Kumar
  0 siblings, 0 replies; 18+ messages in thread
From: Vinay Kumar @ 2021-05-05 14:03 UTC (permalink / raw)
  To: Richard Purdie
  Cc: Steve Sakoman, Mittal, Anuj, vinay.kumar, Randy MacLeod,
	umesh kalappa0, Patches and discussions about the oe-core layer

Hi RIchard,

Will send patch for the Master branch.

Regards,
Vinay

On Wed, May 5, 2021 at 6:51 PM Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
>
> On Wed, 2021-05-05 at 17:02 +0530, Vinay Kumar wrote:
> > Hi Steve,
> >
> > Any update on considering this patch in the "hardknott" branch ?
>
> Anuj is maintaining hardknott, not Steve. Also, doesn't this patch
> need to be applied to master first before we can backport it?
>
> Cheers,
>
> RIchard
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [OE-core] [hardknott][PATCH] Binutils: Fix CVE-2021-20197
  2021-04-29 11:30 [hardknott][PATCH] Binutils: Fix CVE-2021-20197 Vinay Kumar
  2021-05-05 11:32 ` Vinay Kumar
@ 2021-05-06  1:57 ` Anuj Mittal
  2021-05-06 12:25   ` Vinay Kumar
  1 sibling, 1 reply; 18+ messages in thread
From: Anuj Mittal @ 2021-05-06  1:57 UTC (permalink / raw)
  To: openembedded-core, vinay.m.engg; +Cc: vinay.kumar, rwmacleod, umesh.kalappa0

On Thu, 2021-04-29 at 04:30 -0700, Vinay Kumar wrote:
> Source: git://sourceware.org/git/binutils-gdb.git
> Tracking -- https://sourceware.org/bugzilla/show_bug.cgi?id=26945
> 
> Backported upstream commit d3edaa91d4cf7202ec14342410194841e2f67f12 to
> binutils-2.36 source along with commit id dependencies
> (8e03235147a9e774d3ba084e93c2da1aa94d1cec and
> 8b69e61d4be276bb862698aaafddc3e779d23c8f).
> 
> Upstream-Status: Backport [
> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d3edaa91d4cf7202ec14342410194841e2f67f12
> ]
> 
> Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
> ---
> This patch is verified by building both Linux and Mingw toolchains for
> ARM target, and executed "binutils" regresssion testing.
> 
>  .../binutils/binutils-2.36.inc                |   1 +
>  .../binutils/binutils/CVE-2021-20197.patch    | 388 ++++++++++++++++++
>  2 files changed, 389 insertions(+)

This doesn't look like a cherry pick of:

https://github.com/bminor/binutils-gdb/commit/d3edaa91d4cf7202ec14342410194841e2f67f12

What else is in there? 

Also this didn't apply on hardknott and I had to rebase it. Please also
use b in the subject for binutils.

Thanks,

Anuj

>  create mode 100644 meta/recipes-devtools/binutils/binutils/CVE-2021-
> 20197.patch
> 
> diff --git a/meta/recipes-devtools/binutils/binutils-2.36.inc
> b/meta/recipes-devtools/binutils/binutils-2.36.inc
> index beddf601fd..66080dfcee 100644
> --- a/meta/recipes-devtools/binutils/binutils-2.36.inc
> +++ b/meta/recipes-devtools/binutils/binutils-2.36.inc
> @@ -36,5 +36,6 @@ SRC_URI = "\
>       file://0015-sync-with-OE-libtool-changes.patch \
>       file://0016-Check-for-clang-before-checking-gcc-version.patch \
>       
> file://0017-Add-support-for-the-DW_FORM_strx-forms-to-the-BFD-li.patch 
> \
> +     file://CVE-2021-20197.patch \
>  "
>  S  = "${WORKDIR}/git"
> diff --git a/meta/recipes-devtools/binutils/binutils/CVE-2021-
> 20197.patch b/meta/recipes-devtools/binutils/binutils/CVE-2021-
> 20197.patch
> new file mode 100644
> index 0000000000..d6117b65a4
> --- /dev/null
> +++ b/meta/recipes-devtools/binutils/binutils/CVE-2021-20197.patch
> @@ -0,0 +1,388 @@
> +From d3edaa91d4cf7202ec14342410194841e2f67f12 Mon Sep 17 00:00:00 2001
> +From: Alan Modra <amodra@gmail.com>
> +Date: Fri, 26 Feb 2021 11:30:32 +1030
> +Subject: [PATCH] Reinstate various pieces backed out from smart_rename
> changes
> +
> +In the interests of a stable release various last minute smart_rename
> +patches were backed out of the 2.36 branch.  The main reason to
> +reinstate some of those backed out changes here is to make necessary
> +followup fixes to commit 8e03235147a9 simple cherry-picks from
> +mainline.  A secondary reason is that ar -M support isn't fixed for
> +pr26945 without this patch.
> +
> +        PR 26945
> +        * ar.c: Don't include libbfd.h.
> +        (write_archive): Replace xmalloc+strcpy with xstrdup.
> +        * arsup.c (temp_name, real_ofd): New static variables.
> +        (ar_open): Use make_tempname and bfd_fdopenw.
> +        (ar_save): Adjust to suit ar_open changes.
> +        * objcopy.c: Don't include libbfd.h.
> +        * rename.c: Rename and reorder variables.
> +
> +(cherry picked from commit 95b91a043aeaeb546d2fea556d84a2de1e917770)
> +
> +Upstream-Status: Backport [
> https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d3edaa91d4cf7202ec14342410194841e2f67f12
> ]
> +CVE: CVE-2021-20197
> +Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
> +---
> + binutils/ar.c      |   6 +-
> + binutils/arsup.c   |  37 ++++++++----
> + binutils/bucomm.h  |   3 +-
> + binutils/objcopy.c |   9 +--
> + binutils/rename.c  | 148 +++++++++++---------------------------------
> -
> + 5 files changed, 67 insertions(+), 136 deletions(-)
> +
> +diff --git a/binutils/ar.c b/binutils/ar.c
> +index 45a34e3a6cf..44df48c5c67 100644
> +--- a/binutils/ar.c
> ++++ b/binutils/ar.c
> +@@ -25,7 +25,6 @@
> + 
> + #include "sysdep.h"
> + #include "bfd.h"
> +-#include "libbfd.h"
> + #include "libiberty.h"
> + #include "progress.h"
> + #include "getopt.h"
> +@@ -1255,8 +1254,7 @@ write_archive (bfd *iarch)
> +   bfd *contents_head = iarch->archive_next;
> +   int ofd = -1;
> + 
> +-  old_name = (char *) xmalloc (strlen (bfd_get_filename (iarch)) +
> 1);
> +-  strcpy (old_name, bfd_get_filename (iarch));
> ++  old_name = xstrdup (bfd_get_filename (iarch));
> +   new_name = make_tempname (old_name, &ofd);
> + 
> +   if (new_name == NULL)
> +@@ -1308,7 +1306,7 @@ write_archive (bfd *iarch)
> +   /* We don't care if this fails; we might be creating the archive. 
> */
> +   bfd_close (iarch);
> + 
> +-  if (smart_rename (new_name, old_name, 0) != 0)
> ++  if (smart_rename (new_name, old_name, NULL) != 0)
> +     xexit (1);
> +   free (old_name);
> +   free (new_name);
> +diff --git a/binutils/arsup.c b/binutils/arsup.c
> +index 5403a0c5d74..f7ce8f0bc82 100644
> +--- a/binutils/arsup.c
> ++++ b/binutils/arsup.c
> +@@ -42,6 +42,8 @@ extern int deterministic;
> + 
> + static bfd *obfd;
> + static char *real_name;
> ++static char *temp_name;
> ++static int real_ofd;
> + static FILE *outfile;
> + 
> + static void
> +@@ -149,27 +151,24 @@ maybequit (void)
> + void
> + ar_open (char *name, int t)
> + {
> +-  char *tname;
> +-  const char *bname = lbasename (name);
> +-  real_name = name;
> ++  real_name = xstrdup (name);
> ++  temp_name = make_tempname (real_name, &real_ofd);
> + 
> +-  /* Prepend tmp- to the beginning, to avoid file-name clashes after
> +-     truncation on filesystems with limited namespaces (DOS).  */
> +-  if (asprintf (&tname, "%.*stmp-%s", (int) (bname - name), name,
> bname) == -1)
> ++  if (temp_name == NULL)
> +     {
> +-      fprintf (stderr, _("%s: Can't allocate memory for temp name
> (%s)\n"),
> ++      fprintf (stderr, _("%s: Can't open temporary file (%s)\n"),
> +              program_name, strerror(errno));
> +       maybequit ();
> +       return;
> +     }
> + 
> +-  obfd = bfd_openw (tname, NULL);
> ++  obfd = bfd_fdopenw (temp_name, NULL, real_ofd);
> + 
> +   if (!obfd)
> +     {
> +       fprintf (stderr,
> +              _("%s: Can't open output archive %s\n"),
> +-             program_name,  tname);
> ++             program_name, temp_name);
> + 
> +       maybequit ();
> +     }
> +@@ -344,16 +343,30 @@ ar_save (void)
> +     }
> +   else
> +     {
> +-      char *ofilename = xstrdup (bfd_get_filename (obfd));
> ++      struct stat target_stat;
> + 
> +       if (deterministic > 0)
> +         obfd->flags |= BFD_DETERMINISTIC_OUTPUT;
> + 
> +       bfd_close (obfd);
> + 
> +-      smart_rename (ofilename, real_name, 0);
> ++      if (stat (real_name, &target_stat) != 0)
> ++      {
> ++        /* The temp file created in ar_open has mode 0600 as per
> mkstemp.
> ++           Create the real empty output file here so smart_rename
> will
> ++           update the mode according to the process umask.  */
> ++        obfd = bfd_openw (real_name, NULL);
> ++        if (obfd != NULL)
> ++          {
> ++            bfd_set_format (obfd, bfd_archive);
> ++            bfd_close (obfd);
> ++          }
> ++      }
> ++
> ++      smart_rename (temp_name, real_name, NULL);
> +       obfd = 0;
> +-      free (ofilename);
> ++      free (temp_name);
> ++      free (real_name);
> +     }
> + }
> + 
> +diff --git a/binutils/bucomm.h b/binutils/bucomm.h
> +index 91f6a5b228f..aa7e33d8cd1 100644
> +--- a/binutils/bucomm.h
> ++++ b/binutils/bucomm.h
> +@@ -71,7 +71,8 @@ extern void print_version (const char *);
> + /* In rename.c.  */
> + extern void set_times (const char *, const struct stat *);
> + 
> +-extern int smart_rename (const char *, const char *, int);
> ++extern int smart_rename (const char *, const char *, struct stat *);
> ++
> + 
> + /* In libiberty.  */
> + void *xmalloc (size_t);
> +diff --git a/binutils/objcopy.c b/binutils/objcopy.c
> +index eab3b6db585..73aa8bc2514 100644
> +--- a/binutils/objcopy.c
> ++++ b/binutils/objcopy.c
> +@@ -20,7 +20,6 @@
> + \f
> + #include "sysdep.h"
> + #include "bfd.h"
> +-#include "libbfd.h"
> + #include "progress.h"
> + #include "getopt.h"
> + #include "libiberty.h"
> +@@ -4861,12 +4860,10 @@ strip_main (int argc, char *argv[])
> +                output_target, NULL);
> +       if (status == 0)
> +       {
> +-        if (preserve_dates)
> +-          set_times (tmpname, &statbuf);
> +         if (output_file != tmpname)
> +           status = (smart_rename (tmpname,
> +                                   output_file ? output_file :
> argv[i],
> +-                                  preserve_dates) != 0);
> ++                                  preserve_dates ? &statbuf : NULL)
> != 0);
> +         if (status == 0)
> +           status = hold_status;
> +       }
> +@@ -5931,11 +5928,9 @@ copy_main (int argc, char *argv[])
> +            output_target, input_arch);
> +   if (status == 0)
> +     {
> +-      if (preserve_dates)
> +-      set_times (tmpname, &statbuf);
> +       if (tmpname != output_filename)
> +       status = (smart_rename (tmpname, input_filename,
> +-                              preserve_dates) != 0);
> ++                              preserve_dates ? &statbuf : NULL) !=
> 0);
> +     }
> +   else
> +     unlink_if_ordinary (tmpname);
> +diff --git a/binutils/rename.c b/binutils/rename.c
> +index 65ad5bf52c4..72a9323d72c 100644
> +--- a/binutils/rename.c
> ++++ b/binutils/rename.c
> +@@ -24,14 +24,9 @@
> + 
> + #ifdef HAVE_GOOD_UTIME_H
> + #include <utime.h>
> +-#else /* ! HAVE_GOOD_UTIME_H */
> +-#ifdef HAVE_UTIMES
> ++#elif defined HAVE_UTIMES
> + #include <sys/time.h>
> +-#endif /* HAVE_UTIMES */
> +-#endif /* ! HAVE_GOOD_UTIME_H */
> +-
> +-#if ! defined (_WIN32) || defined (__CYGWIN32__)
> +-static int simple_copy (const char *, const char *);
> ++#endif
> + 
> + /* The number of bytes to copy at once.  */
> + #define COPY_BUF 8192
> +@@ -82,7 +77,6 @@ simple_copy (const char *from, const char *to)
> +     }
> +   return 0;
> + }
> +-#endif /* __CYGWIN32__ or not _WIN32 */
> + 
> + /* Set the times of the file DESTINATION to be the same as those in
> +    STATBUF.  */
> +@@ -91,122 +85,52 @@ void
> + set_times (const char *destination, const struct stat *statbuf)
> + {
> +   int result;
> +-
> +-  {
> + #ifdef HAVE_GOOD_UTIME_H
> +-    struct utimbuf tb;
> +-
> +-    tb.actime = statbuf->st_atime;
> +-    tb.modtime = statbuf->st_mtime;
> +-    result = utime (destination, &tb);
> +-#else /* ! HAVE_GOOD_UTIME_H */
> +-#ifndef HAVE_UTIMES
> +-    long tb[2];
> +-
> +-    tb[0] = statbuf->st_atime;
> +-    tb[1] = statbuf->st_mtime;
> +-    result = utime (destination, tb);
> +-#else /* HAVE_UTIMES */
> +-    struct timeval tv[2];
> +-
> +-    tv[0].tv_sec = statbuf->st_atime;
> +-    tv[0].tv_usec = 0;
> +-    tv[1].tv_sec = statbuf->st_mtime;
> +-    tv[1].tv_usec = 0;
> +-    result = utimes (destination, tv);
> +-#endif /* HAVE_UTIMES */
> +-#endif /* ! HAVE_GOOD_UTIME_H */
> +-  }
> ++  struct utimbuf tb;
> ++
> ++  tb.actime = statbuf->st_atime;
> ++  tb.modtime = statbuf->st_mtime;
> ++  result = utime (destination, &tb);
> ++#elif defined HAVE_UTIMES
> ++  struct timeval tv[2];
> ++
> ++  tv[0].tv_sec = statbuf->st_atime;
> ++  tv[0].tv_usec = 0;
> ++  tv[1].tv_sec = statbuf->st_mtime;
> ++  tv[1].tv_usec = 0;
> ++  result = utimes (destination, tv);
> ++#else
> ++  long tb[2];
> ++
> ++  tb[0] = statbuf->st_atime;
> ++  tb[1] = statbuf->st_mtime;
> ++  result = utime (destination, tb);
> ++#endif
> + 
> +   if (result != 0)
> +     non_fatal (_("%s: cannot set time: %s"), destination, strerror
> (errno));
> + }
> + 
> +-#ifndef S_ISLNK
> +-#ifdef S_IFLNK
> +-#define S_ISLNK(m) (((m) & S_IFMT) == S_IFLNK)
> +-#else
> +-#define S_ISLNK(m) 0
> +-#define lstat stat
> +-#endif
> +-#endif
> +-
> +-/* Rename FROM to TO, copying if TO is a link.
> +-   Return 0 if ok, -1 if error.  */
> ++/* Copy FROM to TO.  TARGET_STAT has the file status that, if non-
> NULL,
> ++   is used to fix up timestamps.  Return 0 if ok, -1 if error.
> ++   At one time this function renamed files, but file permissions are
> ++   tricky to update given the number of different schemes used by
> ++   various systems.  So now we just copy.  */
> + 
> + int
> +-smart_rename (const char *from, const char *to, int preserve_dates
> ATTRIBUTE_UNUSED)
> ++smart_rename (const char *from, const char *to,
> ++            struct stat *target_stat)
> + {
> +-  bfd_boolean exists;
> +-  struct stat s;
> +-  int ret = 0;
> +-
> +-  exists = lstat (to, &s) == 0;
> +-
> +-#if defined (_WIN32) && !defined (__CYGWIN32__)
> +-  /* Win32, unlike unix, will not erase `to' in `rename(from, to)'
> but
> +-     fail instead.  Also, chown is not present.  */
> ++  int ret;
> + 
> +-  if (exists)
> +-    remove (to);
> +-
> +-  ret = rename (from, to);
> ++  ret = simple_copy (from, to);
> +   if (ret != 0)
> +-    {
> +-      /* We have to clean up here.  */
> +-      non_fatal (_("unable to rename '%s'; reason: %s"), to, strerror
> (errno));
> +-      unlink (from);
> +-    }
> +-#else
> +-  /* Use rename only if TO is not a symbolic link and has
> +-     only one hard link, and we have permission to write to it.  */
> +-  if (! exists
> +-      || (!S_ISLNK (s.st_mode)
> +-        && S_ISREG (s.st_mode)
> +-        && (s.st_mode & S_IWUSR)
> +-        && s.st_nlink == 1)
> +-      )
> +-    {
> +-      ret = rename (from, to);
> +-      if (ret == 0)
> +-      {
> +-        if (exists)
> +-          {
> +-            /* Try to preserve the permission bits and ownership of
> +-               TO.  First get the mode right except for the setuid
> +-               bit.  Then change the ownership.  Then fix the setuid
> +-               bit.  We do the chmod before the chown because if the
> +-               chown succeeds, and we are a normal user, we won't be
> +-               able to do the chmod afterward.  We don't bother to
> +-               fix the setuid bit first because that might introduce
> +-               a fleeting security problem, and because the chown
> +-               will clear the setuid bit anyhow.  We only fix the
> +-               setuid bit if the chown succeeds, because we don't
> +-               want to introduce an unexpected setuid file owned by
> +-               the user running objcopy.  */
> +-            chmod (to, s.st_mode & 0777);
> +-            if (chown (to, s.st_uid, s.st_gid) >= 0)
> +-              chmod (to, s.st_mode & 07777);
> +-          }
> +-      }
> +-      else
> +-      {
> +-        /* We have to clean up here.  */
> +-        non_fatal (_("unable to rename '%s'; reason: %s"), to,
> strerror (errno));
> +-        unlink (from);
> +-      }
> +-    }
> +-  else
> +-    {
> +-      ret = simple_copy (from, to);
> +-      if (ret != 0)
> +-      non_fatal (_("unable to copy file '%s'; reason: %s"), to,
> strerror (errno));
> ++    non_fatal (_("unable to copy file '%s'; reason: %s"),
> ++             to, strerror (errno));
> + 
> +-      if (preserve_dates)
> +-      set_times (to, &s);
> +-      unlink (from);
> +-    }
> +-#endif /* _WIN32 && !__CYGWIN32__ */
> ++  if (target_stat != NULL)
> ++    set_times (to, target_stat);
> ++  unlink (from);
> + 
> +   return ret;
> + }
> +-- 
> +2.17.1
> +
> 
> 
> 


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [OE-core] [hardknott][PATCH] Binutils: Fix CVE-2021-20197
  2021-05-06  1:57 ` Anuj Mittal
@ 2021-05-06 12:25   ` Vinay Kumar
  2021-05-06 12:49     ` Richard Purdie
  2021-05-07  1:00     ` Anuj Mittal
  0 siblings, 2 replies; 18+ messages in thread
From: Vinay Kumar @ 2021-05-06 12:25 UTC (permalink / raw)
  To: Mittal, Anuj; +Cc: openembedded-core, vinay.kumar, rwmacleod, umesh.kalappa0

Hi Anuj,

Thanks for the feedback.

>> This doesn't look like a cherry pick of: https://github.com/bminor/binutils-gdb/commit/d3edaa91d4cf7202ec14342410194841e2f67f12
>> What else is in there?

cherry picked commit ids in the below order to "SRCREV" of binutils-2.36.inc.

1. 8e03235147a9e774d3ba084e93c2da1aa94d1cec
2. d3edaa91d4cf7202ec14342410194841e2f67f12
3. 8b69e61d4be276bb862698aaafddc3e779d23c8f

Direct cherry-pick of "d3edaa91d4cf7202ec14342410194841e2f67f12" into
"SRCREV" gives errors due to the changes in arsup.c file. So used
8e03235147a9e774d3ba084e93c2da1aa94d1cec (binutils: Avoid renaming
over existing files).

The commit id 8b69e61d4be276bb862698aaafddc3e779d23c8f is
to fix the binutils build error with Mingw.

>> Also this didn't apply on hardknott and I had to rebase it. Please also
>> use b in the subject for binutils.

Will send a v2 patch with corrections.

Regards,
Vinay

On Thu, May 6, 2021 at 7:27 AM Mittal, Anuj <anuj.mittal@intel.com> wrote:
>
> On Thu, 2021-04-29 at 04:30 -0700, Vinay Kumar wrote:
> > Source: git://sourceware.org/git/binutils-gdb.git
> > Tracking -- https://sourceware.org/bugzilla/show_bug.cgi?id=26945
> >
> > Backported upstream commit d3edaa91d4cf7202ec14342410194841e2f67f12 to
> > binutils-2.36 source along with commit id dependencies
> > (8e03235147a9e774d3ba084e93c2da1aa94d1cec and
> > 8b69e61d4be276bb862698aaafddc3e779d23c8f).
> >
> > Upstream-Status: Backport [
> > https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d3edaa91d4cf7202ec14342410194841e2f67f12
> > ]
> >
> > Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
> > ---
> > This patch is verified by building both Linux and Mingw toolchains for
> > ARM target, and executed "binutils" regresssion testing.
> >
> >  .../binutils/binutils-2.36.inc                |   1 +
> >  .../binutils/binutils/CVE-2021-20197.patch    | 388 ++++++++++++++++++
> >  2 files changed, 389 insertions(+)
>
> This doesn't look like a cherry pick of:
>
> https://github.com/bminor/binutils-gdb/commit/d3edaa91d4cf7202ec14342410194841e2f67f12
>
> What else is in there?
>
> Also this didn't apply on hardknott and I had to rebase it. Please also
> use b in the subject for binutils.
>
> Thanks,
>
> Anuj
>
> >  create mode 100644 meta/recipes-devtools/binutils/binutils/CVE-2021-
> > 20197.patch
> >
> > diff --git a/meta/recipes-devtools/binutils/binutils-2.36.inc
> > b/meta/recipes-devtools/binutils/binutils-2.36.inc
> > index beddf601fd..66080dfcee 100644
> > --- a/meta/recipes-devtools/binutils/binutils-2.36.inc
> > +++ b/meta/recipes-devtools/binutils/binutils-2.36.inc
> > @@ -36,5 +36,6 @@ SRC_URI = "\
> >       file://0015-sync-with-OE-libtool-changes.patch \
> >       file://0016-Check-for-clang-before-checking-gcc-version.patch \
> >
> > file://0017-Add-support-for-the-DW_FORM_strx-forms-to-the-BFD-li.patch
> > \
> > +     file://CVE-2021-20197.patch \
> >  "
> >  S  = "${WORKDIR}/git"
> > diff --git a/meta/recipes-devtools/binutils/binutils/CVE-2021-
> > 20197.patch b/meta/recipes-devtools/binutils/binutils/CVE-2021-
> > 20197.patch
> > new file mode 100644
> > index 0000000000..d6117b65a4
> > --- /dev/null
> > +++ b/meta/recipes-devtools/binutils/binutils/CVE-2021-20197.patch
> > @@ -0,0 +1,388 @@
> > +From d3edaa91d4cf7202ec14342410194841e2f67f12 Mon Sep 17 00:00:00 2001
> > +From: Alan Modra <amodra@gmail.com>
> > +Date: Fri, 26 Feb 2021 11:30:32 +1030
> > +Subject: [PATCH] Reinstate various pieces backed out from smart_rename
> > changes
> > +
> > +In the interests of a stable release various last minute smart_rename
> > +patches were backed out of the 2.36 branch.  The main reason to
> > +reinstate some of those backed out changes here is to make necessary
> > +followup fixes to commit 8e03235147a9 simple cherry-picks from
> > +mainline.  A secondary reason is that ar -M support isn't fixed for
> > +pr26945 without this patch.
> > +
> > +        PR 26945
> > +        * ar.c: Don't include libbfd.h.
> > +        (write_archive): Replace xmalloc+strcpy with xstrdup.
> > +        * arsup.c (temp_name, real_ofd): New static variables.
> > +        (ar_open): Use make_tempname and bfd_fdopenw.
> > +        (ar_save): Adjust to suit ar_open changes.
> > +        * objcopy.c: Don't include libbfd.h.
> > +        * rename.c: Rename and reorder variables.
> > +
> > +(cherry picked from commit 95b91a043aeaeb546d2fea556d84a2de1e917770)
> > +
> > +Upstream-Status: Backport [
> > https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d3edaa91d4cf7202ec14342410194841e2f67f12
> > ]
> > +CVE: CVE-2021-20197
> > +Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
> > +---
> > + binutils/ar.c      |   6 +-
> > + binutils/arsup.c   |  37 ++++++++----
> > + binutils/bucomm.h  |   3 +-
> > + binutils/objcopy.c |   9 +--
> > + binutils/rename.c  | 148 +++++++++++---------------------------------
> > -
> > + 5 files changed, 67 insertions(+), 136 deletions(-)
> > +
> > +diff --git a/binutils/ar.c b/binutils/ar.c
> > +index 45a34e3a6cf..44df48c5c67 100644
> > +--- a/binutils/ar.c
> > ++++ b/binutils/ar.c
> > +@@ -25,7 +25,6 @@
> > +
> > + #include "sysdep.h"
> > + #include "bfd.h"
> > +-#include "libbfd.h"
> > + #include "libiberty.h"
> > + #include "progress.h"
> > + #include "getopt.h"
> > +@@ -1255,8 +1254,7 @@ write_archive (bfd *iarch)
> > +   bfd *contents_head = iarch->archive_next;
> > +   int ofd = -1;
> > +
> > +-  old_name = (char *) xmalloc (strlen (bfd_get_filename (iarch)) +
> > 1);
> > +-  strcpy (old_name, bfd_get_filename (iarch));
> > ++  old_name = xstrdup (bfd_get_filename (iarch));
> > +   new_name = make_tempname (old_name, &ofd);
> > +
> > +   if (new_name == NULL)
> > +@@ -1308,7 +1306,7 @@ write_archive (bfd *iarch)
> > +   /* We don't care if this fails; we might be creating the archive.
> > */
> > +   bfd_close (iarch);
> > +
> > +-  if (smart_rename (new_name, old_name, 0) != 0)
> > ++  if (smart_rename (new_name, old_name, NULL) != 0)
> > +     xexit (1);
> > +   free (old_name);
> > +   free (new_name);
> > +diff --git a/binutils/arsup.c b/binutils/arsup.c
> > +index 5403a0c5d74..f7ce8f0bc82 100644
> > +--- a/binutils/arsup.c
> > ++++ b/binutils/arsup.c
> > +@@ -42,6 +42,8 @@ extern int deterministic;
> > +
> > + static bfd *obfd;
> > + static char *real_name;
> > ++static char *temp_name;
> > ++static int real_ofd;
> > + static FILE *outfile;
> > +
> > + static void
> > +@@ -149,27 +151,24 @@ maybequit (void)
> > + void
> > + ar_open (char *name, int t)
> > + {
> > +-  char *tname;
> > +-  const char *bname = lbasename (name);
> > +-  real_name = name;
> > ++  real_name = xstrdup (name);
> > ++  temp_name = make_tempname (real_name, &real_ofd);
> > +
> > +-  /* Prepend tmp- to the beginning, to avoid file-name clashes after
> > +-     truncation on filesystems with limited namespaces (DOS).  */
> > +-  if (asprintf (&tname, "%.*stmp-%s", (int) (bname - name), name,
> > bname) == -1)
> > ++  if (temp_name == NULL)
> > +     {
> > +-      fprintf (stderr, _("%s: Can't allocate memory for temp name
> > (%s)\n"),
> > ++      fprintf (stderr, _("%s: Can't open temporary file (%s)\n"),
> > +              program_name, strerror(errno));
> > +       maybequit ();
> > +       return;
> > +     }
> > +
> > +-  obfd = bfd_openw (tname, NULL);
> > ++  obfd = bfd_fdopenw (temp_name, NULL, real_ofd);
> > +
> > +   if (!obfd)
> > +     {
> > +       fprintf (stderr,
> > +              _("%s: Can't open output archive %s\n"),
> > +-             program_name,  tname);
> > ++             program_name, temp_name);
> > +
> > +       maybequit ();
> > +     }
> > +@@ -344,16 +343,30 @@ ar_save (void)
> > +     }
> > +   else
> > +     {
> > +-      char *ofilename = xstrdup (bfd_get_filename (obfd));
> > ++      struct stat target_stat;
> > +
> > +       if (deterministic > 0)
> > +         obfd->flags |= BFD_DETERMINISTIC_OUTPUT;
> > +
> > +       bfd_close (obfd);
> > +
> > +-      smart_rename (ofilename, real_name, 0);
> > ++      if (stat (real_name, &target_stat) != 0)
> > ++      {
> > ++        /* The temp file created in ar_open has mode 0600 as per
> > mkstemp.
> > ++           Create the real empty output file here so smart_rename
> > will
> > ++           update the mode according to the process umask.  */
> > ++        obfd = bfd_openw (real_name, NULL);
> > ++        if (obfd != NULL)
> > ++          {
> > ++            bfd_set_format (obfd, bfd_archive);
> > ++            bfd_close (obfd);
> > ++          }
> > ++      }
> > ++
> > ++      smart_rename (temp_name, real_name, NULL);
> > +       obfd = 0;
> > +-      free (ofilename);
> > ++      free (temp_name);
> > ++      free (real_name);
> > +     }
> > + }
> > +
> > +diff --git a/binutils/bucomm.h b/binutils/bucomm.h
> > +index 91f6a5b228f..aa7e33d8cd1 100644
> > +--- a/binutils/bucomm.h
> > ++++ b/binutils/bucomm.h
> > +@@ -71,7 +71,8 @@ extern void print_version (const char *);
> > + /* In rename.c.  */
> > + extern void set_times (const char *, const struct stat *);
> > +
> > +-extern int smart_rename (const char *, const char *, int);
> > ++extern int smart_rename (const char *, const char *, struct stat *);
> > ++
> > +
> > + /* In libiberty.  */
> > + void *xmalloc (size_t);
> > +diff --git a/binutils/objcopy.c b/binutils/objcopy.c
> > +index eab3b6db585..73aa8bc2514 100644
> > +--- a/binutils/objcopy.c
> > ++++ b/binutils/objcopy.c
> > +@@ -20,7 +20,6 @@
> > +
> > + #include "sysdep.h"
> > + #include "bfd.h"
> > +-#include "libbfd.h"
> > + #include "progress.h"
> > + #include "getopt.h"
> > + #include "libiberty.h"
> > +@@ -4861,12 +4860,10 @@ strip_main (int argc, char *argv[])
> > +                output_target, NULL);
> > +       if (status == 0)
> > +       {
> > +-        if (preserve_dates)
> > +-          set_times (tmpname, &statbuf);
> > +         if (output_file != tmpname)
> > +           status = (smart_rename (tmpname,
> > +                                   output_file ? output_file :
> > argv[i],
> > +-                                  preserve_dates) != 0);
> > ++                                  preserve_dates ? &statbuf : NULL)
> > != 0);
> > +         if (status == 0)
> > +           status = hold_status;
> > +       }
> > +@@ -5931,11 +5928,9 @@ copy_main (int argc, char *argv[])
> > +            output_target, input_arch);
> > +   if (status == 0)
> > +     {
> > +-      if (preserve_dates)
> > +-      set_times (tmpname, &statbuf);
> > +       if (tmpname != output_filename)
> > +       status = (smart_rename (tmpname, input_filename,
> > +-                              preserve_dates) != 0);
> > ++                              preserve_dates ? &statbuf : NULL) !=
> > 0);
> > +     }
> > +   else
> > +     unlink_if_ordinary (tmpname);
> > +diff --git a/binutils/rename.c b/binutils/rename.c
> > +index 65ad5bf52c4..72a9323d72c 100644
> > +--- a/binutils/rename.c
> > ++++ b/binutils/rename.c
> > +@@ -24,14 +24,9 @@
> > +
> > + #ifdef HAVE_GOOD_UTIME_H
> > + #include <utime.h>
> > +-#else /* ! HAVE_GOOD_UTIME_H */
> > +-#ifdef HAVE_UTIMES
> > ++#elif defined HAVE_UTIMES
> > + #include <sys/time.h>
> > +-#endif /* HAVE_UTIMES */
> > +-#endif /* ! HAVE_GOOD_UTIME_H */
> > +-
> > +-#if ! defined (_WIN32) || defined (__CYGWIN32__)
> > +-static int simple_copy (const char *, const char *);
> > ++#endif
> > +
> > + /* The number of bytes to copy at once.  */
> > + #define COPY_BUF 8192
> > +@@ -82,7 +77,6 @@ simple_copy (const char *from, const char *to)
> > +     }
> > +   return 0;
> > + }
> > +-#endif /* __CYGWIN32__ or not _WIN32 */
> > +
> > + /* Set the times of the file DESTINATION to be the same as those in
> > +    STATBUF.  */
> > +@@ -91,122 +85,52 @@ void
> > + set_times (const char *destination, const struct stat *statbuf)
> > + {
> > +   int result;
> > +-
> > +-  {
> > + #ifdef HAVE_GOOD_UTIME_H
> > +-    struct utimbuf tb;
> > +-
> > +-    tb.actime = statbuf->st_atime;
> > +-    tb.modtime = statbuf->st_mtime;
> > +-    result = utime (destination, &tb);
> > +-#else /* ! HAVE_GOOD_UTIME_H */
> > +-#ifndef HAVE_UTIMES
> > +-    long tb[2];
> > +-
> > +-    tb[0] = statbuf->st_atime;
> > +-    tb[1] = statbuf->st_mtime;
> > +-    result = utime (destination, tb);
> > +-#else /* HAVE_UTIMES */
> > +-    struct timeval tv[2];
> > +-
> > +-    tv[0].tv_sec = statbuf->st_atime;
> > +-    tv[0].tv_usec = 0;
> > +-    tv[1].tv_sec = statbuf->st_mtime;
> > +-    tv[1].tv_usec = 0;
> > +-    result = utimes (destination, tv);
> > +-#endif /* HAVE_UTIMES */
> > +-#endif /* ! HAVE_GOOD_UTIME_H */
> > +-  }
> > ++  struct utimbuf tb;
> > ++
> > ++  tb.actime = statbuf->st_atime;
> > ++  tb.modtime = statbuf->st_mtime;
> > ++  result = utime (destination, &tb);
> > ++#elif defined HAVE_UTIMES
> > ++  struct timeval tv[2];
> > ++
> > ++  tv[0].tv_sec = statbuf->st_atime;
> > ++  tv[0].tv_usec = 0;
> > ++  tv[1].tv_sec = statbuf->st_mtime;
> > ++  tv[1].tv_usec = 0;
> > ++  result = utimes (destination, tv);
> > ++#else
> > ++  long tb[2];
> > ++
> > ++  tb[0] = statbuf->st_atime;
> > ++  tb[1] = statbuf->st_mtime;
> > ++  result = utime (destination, tb);
> > ++#endif
> > +
> > +   if (result != 0)
> > +     non_fatal (_("%s: cannot set time: %s"), destination, strerror
> > (errno));
> > + }
> > +
> > +-#ifndef S_ISLNK
> > +-#ifdef S_IFLNK
> > +-#define S_ISLNK(m) (((m) & S_IFMT) == S_IFLNK)
> > +-#else
> > +-#define S_ISLNK(m) 0
> > +-#define lstat stat
> > +-#endif
> > +-#endif
> > +-
> > +-/* Rename FROM to TO, copying if TO is a link.
> > +-   Return 0 if ok, -1 if error.  */
> > ++/* Copy FROM to TO.  TARGET_STAT has the file status that, if non-
> > NULL,
> > ++   is used to fix up timestamps.  Return 0 if ok, -1 if error.
> > ++   At one time this function renamed files, but file permissions are
> > ++   tricky to update given the number of different schemes used by
> > ++   various systems.  So now we just copy.  */
> > +
> > + int
> > +-smart_rename (const char *from, const char *to, int preserve_dates
> > ATTRIBUTE_UNUSED)
> > ++smart_rename (const char *from, const char *to,
> > ++            struct stat *target_stat)
> > + {
> > +-  bfd_boolean exists;
> > +-  struct stat s;
> > +-  int ret = 0;
> > +-
> > +-  exists = lstat (to, &s) == 0;
> > +-
> > +-#if defined (_WIN32) && !defined (__CYGWIN32__)
> > +-  /* Win32, unlike unix, will not erase `to' in `rename(from, to)'
> > but
> > +-     fail instead.  Also, chown is not present.  */
> > ++  int ret;
> > +
> > +-  if (exists)
> > +-    remove (to);
> > +-
> > +-  ret = rename (from, to);
> > ++  ret = simple_copy (from, to);
> > +   if (ret != 0)
> > +-    {
> > +-      /* We have to clean up here.  */
> > +-      non_fatal (_("unable to rename '%s'; reason: %s"), to, strerror
> > (errno));
> > +-      unlink (from);
> > +-    }
> > +-#else
> > +-  /* Use rename only if TO is not a symbolic link and has
> > +-     only one hard link, and we have permission to write to it.  */
> > +-  if (! exists
> > +-      || (!S_ISLNK (s.st_mode)
> > +-        && S_ISREG (s.st_mode)
> > +-        && (s.st_mode & S_IWUSR)
> > +-        && s.st_nlink == 1)
> > +-      )
> > +-    {
> > +-      ret = rename (from, to);
> > +-      if (ret == 0)
> > +-      {
> > +-        if (exists)
> > +-          {
> > +-            /* Try to preserve the permission bits and ownership of
> > +-               TO.  First get the mode right except for the setuid
> > +-               bit.  Then change the ownership.  Then fix the setuid
> > +-               bit.  We do the chmod before the chown because if the
> > +-               chown succeeds, and we are a normal user, we won't be
> > +-               able to do the chmod afterward.  We don't bother to
> > +-               fix the setuid bit first because that might introduce
> > +-               a fleeting security problem, and because the chown
> > +-               will clear the setuid bit anyhow.  We only fix the
> > +-               setuid bit if the chown succeeds, because we don't
> > +-               want to introduce an unexpected setuid file owned by
> > +-               the user running objcopy.  */
> > +-            chmod (to, s.st_mode & 0777);
> > +-            if (chown (to, s.st_uid, s.st_gid) >= 0)
> > +-              chmod (to, s.st_mode & 07777);
> > +-          }
> > +-      }
> > +-      else
> > +-      {
> > +-        /* We have to clean up here.  */
> > +-        non_fatal (_("unable to rename '%s'; reason: %s"), to,
> > strerror (errno));
> > +-        unlink (from);
> > +-      }
> > +-    }
> > +-  else
> > +-    {
> > +-      ret = simple_copy (from, to);
> > +-      if (ret != 0)
> > +-      non_fatal (_("unable to copy file '%s'; reason: %s"), to,
> > strerror (errno));
> > ++    non_fatal (_("unable to copy file '%s'; reason: %s"),
> > ++             to, strerror (errno));
> > +
> > +-      if (preserve_dates)
> > +-      set_times (to, &s);
> > +-      unlink (from);
> > +-    }
> > +-#endif /* _WIN32 && !__CYGWIN32__ */
> > ++  if (target_stat != NULL)
> > ++    set_times (to, target_stat);
> > ++  unlink (from);
> > +
> > +   return ret;
> > + }
> > +--
> > +2.17.1
> > +
> >
> > 
> >
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [OE-core] [hardknott][PATCH] Binutils: Fix CVE-2021-20197
  2021-05-06 12:25   ` Vinay Kumar
@ 2021-05-06 12:49     ` Richard Purdie
  2021-05-06 18:19       ` [hardknott][PATCH v2] binutils: " Vinay Kumar
  2021-05-06 18:26       ` [OE-core] [hardknott][PATCH] Binutils: " Vinay Kumar
  2021-05-07  1:00     ` Anuj Mittal
  1 sibling, 2 replies; 18+ messages in thread
From: Richard Purdie @ 2021-05-06 12:49 UTC (permalink / raw)
  To: Vinay Kumar, Mittal, Anuj
  Cc: openembedded-core, vinay.kumar, rwmacleod, umesh.kalappa0

On Thu, 2021-05-06 at 17:55 +0530, Vinay Kumar wrote:
> Hi Anuj,
> 
> Thanks for the feedback.
> 
> > > This doesn't look like a cherry pick of: https://github.com/bminor/binutils-gdb/commit/d3edaa91d4cf7202ec14342410194841e2f67f12
> > > What else is in there?
> 
> cherry picked commit ids in the below order to "SRCREV" of binutils-2.36.inc.
> 
> 1. 8e03235147a9e774d3ba084e93c2da1aa94d1cec
> 2. d3edaa91d4cf7202ec14342410194841e2f67f12
> 3. 8b69e61d4be276bb862698aaafddc3e779d23c8f
> 
> Direct cherry-pick of "d3edaa91d4cf7202ec14342410194841e2f67f12" into
> "SRCREV" gives errors due to the changes in arsup.c file. So used
> 8e03235147a9e774d3ba084e93c2da1aa94d1cec (binutils: Avoid renaming
> over existing files).
> 
> The commit id 8b69e61d4be276bb862698aaafddc3e779d23c8f is
> to fix the binutils build error with Mingw.
> 
> > > Also this didn't apply on hardknott and I had to rebase it. Please also
> > > use b in the subject for binutils.
> 
> Will send a v2 patch with corrections.

The patch was tested and then merged but we can update the patch header with 
more info if we have it.

I'd assumed the differences were due to the smart_rename disabling/renabling 
the patch mentioned.

Cheers,

Richard


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [hardknott][PATCH v2] binutils: Fix CVE-2021-20197
  2021-05-06 12:49     ` Richard Purdie
@ 2021-05-06 18:19       ` Vinay Kumar
  2021-05-06 18:26       ` [OE-core] [hardknott][PATCH] Binutils: " Vinay Kumar
  1 sibling, 0 replies; 18+ messages in thread
From: Vinay Kumar @ 2021-05-06 18:19 UTC (permalink / raw)
  To: openembedded-core, anuj.mittal
  Cc: vinay.kumar, rwmacleod, umesh.kalappa0, richard.purdie, Vinay Kumar

Source: git://sourceware.org/git/binutils-gdb.git
Tracking -- https://sourceware.org/bugzilla/show_bug.cgi?id=26945

Backported upstream commit d3edaa91d4cf7202ec14342410194841e2f67f12 to
binutils-2.36 source along with commit id dependencies
(8e03235147a9e774d3ba084e93c2da1aa94d1cec and 8b69e61d4be276bb862698aaafddc3e779d23c8f).

This patch is cherry picked from commit 95b91a043aeaeb546d2fea556d84a2de1e917770 on binutils-2_36-branch.

The original patches are discussed at,
https://sourceware.org/bugzilla/show_bug.cgi?id=26945#c14
https://sourceware.org/bugzilla/show_bug.cgi?id=26945#c15

Upstream-Status: Backport [https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d3edaa91d4cf7202ec14342410194841e2f67f12]

Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
---
The patch is verified by Mingw and Linux toolchain builds.

 .../binutils/binutils-2.36.inc                |   1 +
 .../binutils/binutils/CVE-2021-20197.patch    | 388 ++++++++++++++++++
 2 files changed, 389 insertions(+)
 create mode 100644 meta/recipes-devtools/binutils/binutils/CVE-2021-20197.patch

diff --git a/meta/recipes-devtools/binutils/binutils-2.36.inc b/meta/recipes-devtools/binutils/binutils-2.36.inc
index 2968291889..6d0293b4bc 100644
--- a/meta/recipes-devtools/binutils/binutils-2.36.inc
+++ b/meta/recipes-devtools/binutils/binutils-2.36.inc
@@ -41,5 +41,6 @@ SRC_URI = "\
      file://0014-Fix-rpath-in-libtool-when-sysroot-is-enabled.patch \
      file://0015-sync-with-OE-libtool-changes.patch \
      file://0016-Check-for-clang-before-checking-gcc-version.patch \
+     file://CVE-2021-20197.patch \
 "
 S  = "${WORKDIR}/git"
diff --git a/meta/recipes-devtools/binutils/binutils/CVE-2021-20197.patch b/meta/recipes-devtools/binutils/binutils/CVE-2021-20197.patch
new file mode 100644
index 0000000000..d6117b65a4
--- /dev/null
+++ b/meta/recipes-devtools/binutils/binutils/CVE-2021-20197.patch
@@ -0,0 +1,388 @@
+From d3edaa91d4cf7202ec14342410194841e2f67f12 Mon Sep 17 00:00:00 2001
+From: Alan Modra <amodra@gmail.com>
+Date: Fri, 26 Feb 2021 11:30:32 +1030
+Subject: [PATCH] Reinstate various pieces backed out from smart_rename changes
+
+In the interests of a stable release various last minute smart_rename
+patches were backed out of the 2.36 branch.  The main reason to
+reinstate some of those backed out changes here is to make necessary
+followup fixes to commit 8e03235147a9 simple cherry-picks from
+mainline.  A secondary reason is that ar -M support isn't fixed for
+pr26945 without this patch.
+
+        PR 26945
+        * ar.c: Don't include libbfd.h.
+        (write_archive): Replace xmalloc+strcpy with xstrdup.
+        * arsup.c (temp_name, real_ofd): New static variables.
+        (ar_open): Use make_tempname and bfd_fdopenw.
+        (ar_save): Adjust to suit ar_open changes.
+        * objcopy.c: Don't include libbfd.h.
+        * rename.c: Rename and reorder variables.
+
+(cherry picked from commit 95b91a043aeaeb546d2fea556d84a2de1e917770)
+
+Upstream-Status: Backport [https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d3edaa91d4cf7202ec14342410194841e2f67f12]
+CVE: CVE-2021-20197
+Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
+---
+ binutils/ar.c      |   6 +-
+ binutils/arsup.c   |  37 ++++++++----
+ binutils/bucomm.h  |   3 +-
+ binutils/objcopy.c |   9 +--
+ binutils/rename.c  | 148 +++++++++++----------------------------------
+ 5 files changed, 67 insertions(+), 136 deletions(-)
+
+diff --git a/binutils/ar.c b/binutils/ar.c
+index 45a34e3a6cf..44df48c5c67 100644
+--- a/binutils/ar.c
++++ b/binutils/ar.c
+@@ -25,7 +25,6 @@
+ 
+ #include "sysdep.h"
+ #include "bfd.h"
+-#include "libbfd.h"
+ #include "libiberty.h"
+ #include "progress.h"
+ #include "getopt.h"
+@@ -1255,8 +1254,7 @@ write_archive (bfd *iarch)
+   bfd *contents_head = iarch->archive_next;
+   int ofd = -1;
+ 
+-  old_name = (char *) xmalloc (strlen (bfd_get_filename (iarch)) + 1);
+-  strcpy (old_name, bfd_get_filename (iarch));
++  old_name = xstrdup (bfd_get_filename (iarch));
+   new_name = make_tempname (old_name, &ofd);
+ 
+   if (new_name == NULL)
+@@ -1308,7 +1306,7 @@ write_archive (bfd *iarch)
+   /* We don't care if this fails; we might be creating the archive.  */
+   bfd_close (iarch);
+ 
+-  if (smart_rename (new_name, old_name, 0) != 0)
++  if (smart_rename (new_name, old_name, NULL) != 0)
+     xexit (1);
+   free (old_name);
+   free (new_name);
+diff --git a/binutils/arsup.c b/binutils/arsup.c
+index 5403a0c5d74..f7ce8f0bc82 100644
+--- a/binutils/arsup.c
++++ b/binutils/arsup.c
+@@ -42,6 +42,8 @@ extern int deterministic;
+ 
+ static bfd *obfd;
+ static char *real_name;
++static char *temp_name;
++static int real_ofd;
+ static FILE *outfile;
+ 
+ static void
+@@ -149,27 +151,24 @@ maybequit (void)
+ void
+ ar_open (char *name, int t)
+ {
+-  char *tname;
+-  const char *bname = lbasename (name);
+-  real_name = name;
++  real_name = xstrdup (name);
++  temp_name = make_tempname (real_name, &real_ofd);
+ 
+-  /* Prepend tmp- to the beginning, to avoid file-name clashes after
+-     truncation on filesystems with limited namespaces (DOS).  */
+-  if (asprintf (&tname, "%.*stmp-%s", (int) (bname - name), name, bname) == -1)
++  if (temp_name == NULL)
+     {
+-      fprintf (stderr, _("%s: Can't allocate memory for temp name (%s)\n"),
++      fprintf (stderr, _("%s: Can't open temporary file (%s)\n"),
+ 	       program_name, strerror(errno));
+       maybequit ();
+       return;
+     }
+ 
+-  obfd = bfd_openw (tname, NULL);
++  obfd = bfd_fdopenw (temp_name, NULL, real_ofd);
+ 
+   if (!obfd)
+     {
+       fprintf (stderr,
+ 	       _("%s: Can't open output archive %s\n"),
+-	       program_name,  tname);
++	       program_name, temp_name);
+ 
+       maybequit ();
+     }
+@@ -344,16 +343,30 @@ ar_save (void)
+     }
+   else
+     {
+-      char *ofilename = xstrdup (bfd_get_filename (obfd));
++      struct stat target_stat;
+ 
+       if (deterministic > 0)
+         obfd->flags |= BFD_DETERMINISTIC_OUTPUT;
+ 
+       bfd_close (obfd);
+ 
+-      smart_rename (ofilename, real_name, 0);
++      if (stat (real_name, &target_stat) != 0)
++	{
++	  /* The temp file created in ar_open has mode 0600 as per mkstemp.
++	     Create the real empty output file here so smart_rename will
++	     update the mode according to the process umask.  */
++	  obfd = bfd_openw (real_name, NULL);
++	  if (obfd != NULL)
++	    {
++	      bfd_set_format (obfd, bfd_archive);
++	      bfd_close (obfd);
++	    }
++	}
++
++      smart_rename (temp_name, real_name, NULL);
+       obfd = 0;
+-      free (ofilename);
++      free (temp_name);
++      free (real_name);
+     }
+ }
+ 
+diff --git a/binutils/bucomm.h b/binutils/bucomm.h
+index 91f6a5b228f..aa7e33d8cd1 100644
+--- a/binutils/bucomm.h
++++ b/binutils/bucomm.h
+@@ -71,7 +71,8 @@ extern void print_version (const char *);
+ /* In rename.c.  */
+ extern void set_times (const char *, const struct stat *);
+ 
+-extern int smart_rename (const char *, const char *, int);
++extern int smart_rename (const char *, const char *, struct stat *);
++
+ 
+ /* In libiberty.  */
+ void *xmalloc (size_t);
+diff --git a/binutils/objcopy.c b/binutils/objcopy.c
+index eab3b6db585..73aa8bc2514 100644
+--- a/binutils/objcopy.c
++++ b/binutils/objcopy.c
+@@ -20,7 +20,6 @@
+ \f
+ #include "sysdep.h"
+ #include "bfd.h"
+-#include "libbfd.h"
+ #include "progress.h"
+ #include "getopt.h"
+ #include "libiberty.h"
+@@ -4861,12 +4860,10 @@ strip_main (int argc, char *argv[])
+ 		 output_target, NULL);
+       if (status == 0)
+ 	{
+-	  if (preserve_dates)
+-	    set_times (tmpname, &statbuf);
+ 	  if (output_file != tmpname)
+ 	    status = (smart_rename (tmpname,
+ 				    output_file ? output_file : argv[i],
+-				    preserve_dates) != 0);
++				    preserve_dates ? &statbuf : NULL) != 0);
+ 	  if (status == 0)
+ 	    status = hold_status;
+ 	}
+@@ -5931,11 +5928,9 @@ copy_main (int argc, char *argv[])
+ 	     output_target, input_arch);
+   if (status == 0)
+     {
+-      if (preserve_dates)
+-	set_times (tmpname, &statbuf);
+       if (tmpname != output_filename)
+ 	status = (smart_rename (tmpname, input_filename,
+-				preserve_dates) != 0);
++				preserve_dates ? &statbuf : NULL) != 0);
+     }
+   else
+     unlink_if_ordinary (tmpname);
+diff --git a/binutils/rename.c b/binutils/rename.c
+index 65ad5bf52c4..72a9323d72c 100644
+--- a/binutils/rename.c
++++ b/binutils/rename.c
+@@ -24,14 +24,9 @@
+ 
+ #ifdef HAVE_GOOD_UTIME_H
+ #include <utime.h>
+-#else /* ! HAVE_GOOD_UTIME_H */
+-#ifdef HAVE_UTIMES
++#elif defined HAVE_UTIMES
+ #include <sys/time.h>
+-#endif /* HAVE_UTIMES */
+-#endif /* ! HAVE_GOOD_UTIME_H */
+-
+-#if ! defined (_WIN32) || defined (__CYGWIN32__)
+-static int simple_copy (const char *, const char *);
++#endif
+ 
+ /* The number of bytes to copy at once.  */
+ #define COPY_BUF 8192
+@@ -82,7 +77,6 @@ simple_copy (const char *from, const char *to)
+     }
+   return 0;
+ }
+-#endif /* __CYGWIN32__ or not _WIN32 */
+ 
+ /* Set the times of the file DESTINATION to be the same as those in
+    STATBUF.  */
+@@ -91,122 +85,52 @@ void
+ set_times (const char *destination, const struct stat *statbuf)
+ {
+   int result;
+-
+-  {
+ #ifdef HAVE_GOOD_UTIME_H
+-    struct utimbuf tb;
+-
+-    tb.actime = statbuf->st_atime;
+-    tb.modtime = statbuf->st_mtime;
+-    result = utime (destination, &tb);
+-#else /* ! HAVE_GOOD_UTIME_H */
+-#ifndef HAVE_UTIMES
+-    long tb[2];
+-
+-    tb[0] = statbuf->st_atime;
+-    tb[1] = statbuf->st_mtime;
+-    result = utime (destination, tb);
+-#else /* HAVE_UTIMES */
+-    struct timeval tv[2];
+-
+-    tv[0].tv_sec = statbuf->st_atime;
+-    tv[0].tv_usec = 0;
+-    tv[1].tv_sec = statbuf->st_mtime;
+-    tv[1].tv_usec = 0;
+-    result = utimes (destination, tv);
+-#endif /* HAVE_UTIMES */
+-#endif /* ! HAVE_GOOD_UTIME_H */
+-  }
++  struct utimbuf tb;
++
++  tb.actime = statbuf->st_atime;
++  tb.modtime = statbuf->st_mtime;
++  result = utime (destination, &tb);
++#elif defined HAVE_UTIMES
++  struct timeval tv[2];
++
++  tv[0].tv_sec = statbuf->st_atime;
++  tv[0].tv_usec = 0;
++  tv[1].tv_sec = statbuf->st_mtime;
++  tv[1].tv_usec = 0;
++  result = utimes (destination, tv);
++#else
++  long tb[2];
++
++  tb[0] = statbuf->st_atime;
++  tb[1] = statbuf->st_mtime;
++  result = utime (destination, tb);
++#endif
+ 
+   if (result != 0)
+     non_fatal (_("%s: cannot set time: %s"), destination, strerror (errno));
+ }
+ 
+-#ifndef S_ISLNK
+-#ifdef S_IFLNK
+-#define S_ISLNK(m) (((m) & S_IFMT) == S_IFLNK)
+-#else
+-#define S_ISLNK(m) 0
+-#define lstat stat
+-#endif
+-#endif
+-
+-/* Rename FROM to TO, copying if TO is a link.
+-   Return 0 if ok, -1 if error.  */
++/* Copy FROM to TO.  TARGET_STAT has the file status that, if non-NULL,
++   is used to fix up timestamps.  Return 0 if ok, -1 if error.
++   At one time this function renamed files, but file permissions are
++   tricky to update given the number of different schemes used by
++   various systems.  So now we just copy.  */
+ 
+ int
+-smart_rename (const char *from, const char *to, int preserve_dates ATTRIBUTE_UNUSED)
++smart_rename (const char *from, const char *to,
++	      struct stat *target_stat)
+ {
+-  bfd_boolean exists;
+-  struct stat s;
+-  int ret = 0;
+-
+-  exists = lstat (to, &s) == 0;
+-
+-#if defined (_WIN32) && !defined (__CYGWIN32__)
+-  /* Win32, unlike unix, will not erase `to' in `rename(from, to)' but
+-     fail instead.  Also, chown is not present.  */
++  int ret;
+ 
+-  if (exists)
+-    remove (to);
+-
+-  ret = rename (from, to);
++  ret = simple_copy (from, to);
+   if (ret != 0)
+-    {
+-      /* We have to clean up here.  */
+-      non_fatal (_("unable to rename '%s'; reason: %s"), to, strerror (errno));
+-      unlink (from);
+-    }
+-#else
+-  /* Use rename only if TO is not a symbolic link and has
+-     only one hard link, and we have permission to write to it.  */
+-  if (! exists
+-      || (!S_ISLNK (s.st_mode)
+-	  && S_ISREG (s.st_mode)
+-	  && (s.st_mode & S_IWUSR)
+-	  && s.st_nlink == 1)
+-      )
+-    {
+-      ret = rename (from, to);
+-      if (ret == 0)
+-	{
+-	  if (exists)
+-	    {
+-	      /* Try to preserve the permission bits and ownership of
+-		 TO.  First get the mode right except for the setuid
+-		 bit.  Then change the ownership.  Then fix the setuid
+-		 bit.  We do the chmod before the chown because if the
+-		 chown succeeds, and we are a normal user, we won't be
+-		 able to do the chmod afterward.  We don't bother to
+-		 fix the setuid bit first because that might introduce
+-		 a fleeting security problem, and because the chown
+-		 will clear the setuid bit anyhow.  We only fix the
+-		 setuid bit if the chown succeeds, because we don't
+-		 want to introduce an unexpected setuid file owned by
+-		 the user running objcopy.  */
+-	      chmod (to, s.st_mode & 0777);
+-	      if (chown (to, s.st_uid, s.st_gid) >= 0)
+-		chmod (to, s.st_mode & 07777);
+-	    }
+-	}
+-      else
+-	{
+-	  /* We have to clean up here.  */
+-	  non_fatal (_("unable to rename '%s'; reason: %s"), to, strerror (errno));
+-	  unlink (from);
+-	}
+-    }
+-  else
+-    {
+-      ret = simple_copy (from, to);
+-      if (ret != 0)
+-	non_fatal (_("unable to copy file '%s'; reason: %s"), to, strerror (errno));
++    non_fatal (_("unable to copy file '%s'; reason: %s"),
++	       to, strerror (errno));
+ 
+-      if (preserve_dates)
+-	set_times (to, &s);
+-      unlink (from);
+-    }
+-#endif /* _WIN32 && !__CYGWIN32__ */
++  if (target_stat != NULL)
++    set_times (to, target_stat);
++  unlink (from);
+ 
+   return ret;
+ }
+-- 
+2.17.1
+
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [OE-core] [hardknott][PATCH] Binutils: Fix CVE-2021-20197
  2021-05-06 12:49     ` Richard Purdie
  2021-05-06 18:19       ` [hardknott][PATCH v2] binutils: " Vinay Kumar
@ 2021-05-06 18:26       ` Vinay Kumar
  1 sibling, 0 replies; 18+ messages in thread
From: Vinay Kumar @ 2021-05-06 18:26 UTC (permalink / raw)
  To: Richard Purdie
  Cc: Mittal, Anuj, openembedded-core, vinay.kumar, rwmacleod, umesh.kalappa0

Hi Richard,

>> The patch was tested and then merged but we can update the patch header with
>> more info if we have it.
Thanks for clarification.

Updated the patch header with additional info,
===========================
This patch is cherry picked from commit
95b91a043aeaeb546d2fea556d84a2de1e917770 on binutils-2_36-branch.

The original patches are discussed at,
https://sourceware.org/bugzilla/show_bug.cgi?id=26945#c14
https://sourceware.org/bugzilla/show_bug.cgi?id=26945#c15
===========================

Sent v2 patch with suggested corrections for review.

Regards,
Vinay

On Thu, May 6, 2021 at 6:19 PM Richard Purdie
<richard.purdie@linuxfoundation.org> wrote:
>
> On Thu, 2021-05-06 at 17:55 +0530, Vinay Kumar wrote:
> > Hi Anuj,
> >
> > Thanks for the feedback.
> >
> > > > This doesn't look like a cherry pick of: https://github.com/bminor/binutils-gdb/commit/d3edaa91d4cf7202ec14342410194841e2f67f12
> > > > What else is in there?
> >
> > cherry picked commit ids in the below order to "SRCREV" of binutils-2.36.inc.
> >
> > 1. 8e03235147a9e774d3ba084e93c2da1aa94d1cec
> > 2. d3edaa91d4cf7202ec14342410194841e2f67f12
> > 3. 8b69e61d4be276bb862698aaafddc3e779d23c8f
> >
> > Direct cherry-pick of "d3edaa91d4cf7202ec14342410194841e2f67f12" into
> > "SRCREV" gives errors due to the changes in arsup.c file. So used
> > 8e03235147a9e774d3ba084e93c2da1aa94d1cec (binutils: Avoid renaming
> > over existing files).
> >
> > The commit id 8b69e61d4be276bb862698aaafddc3e779d23c8f is
> > to fix the binutils build error with Mingw.
> >
> > > > Also this didn't apply on hardknott and I had to rebase it. Please also
> > > > use b in the subject for binutils.
> >
> > Will send a v2 patch with corrections.
>
> The patch was tested and then merged but we can update the patch header with
> more info if we have it.
>
> I'd assumed the differences were due to the smart_rename disabling/renabling
> the patch mentioned.
>
> Cheers,
>
> Richard
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [OE-core] [hardknott][PATCH] Binutils: Fix CVE-2021-20197
  2021-05-06 12:25   ` Vinay Kumar
  2021-05-06 12:49     ` Richard Purdie
@ 2021-05-07  1:00     ` Anuj Mittal
  2021-05-07 14:48       ` Vinay Kumar
  1 sibling, 1 reply; 18+ messages in thread
From: Anuj Mittal @ 2021-05-07  1:00 UTC (permalink / raw)
  To: vinay.m.engg; +Cc: vinay.kumar, openembedded-core, rwmacleod, umesh.kalappa0

On Thu, 2021-05-06 at 17:55 +0530, Vinay Kumar wrote:
> Hi Anuj,
> 
> Thanks for the feedback.
> 
> > > This doesn't look like a cherry pick of: 
> > > https://github.com/bminor/binutils-gdb/commit/d3edaa91d4cf7202ec14342410194841e2f67f12
> > > What else is in there?
> 
> cherry picked commit ids in the below order to "SRCREV" of binutils-
> 2.36.inc.
> 
> 1. 8e03235147a9e774d3ba084e93c2da1aa94d1cec
> 2. d3edaa91d4cf7202ec14342410194841e2f67f12
> 3. 8b69e61d4be276bb862698aaafddc3e779d23c8f
> 
> Direct cherry-pick of "d3edaa91d4cf7202ec14342410194841e2f67f12" into
> "SRCREV" gives errors due to the changes in arsup.c file. So used
> 8e03235147a9e774d3ba084e93c2da1aa94d1cec (binutils: Avoid renaming
> over existing files).
> 
> The commit id 8b69e61d4be276bb862698aaafddc3e779d23c8f is
> to fix the binutils build error with Mingw.

I think it should be 3 separate patches in that case. Right now, the
patch header is wrong as it's not the upstream commit that it is
claiming to be.

Thanks,

Anuj

> 
> > > Also this didn't apply on hardknott and I had to rebase it.
> > > Please also
> > > use b in the subject for binutils.
> 
> Will send a v2 patch with corrections.
> 
> Regards,
> Vinay
> 
> On Thu, May 6, 2021 at 7:27 AM Mittal, Anuj <anuj.mittal@intel.com>
> wrote:
> > 
> > On Thu, 2021-04-29 at 04:30 -0700, Vinay Kumar wrote:
> > > Source: git://sourceware.org/git/binutils-gdb.git
> > > Tracking -- https://sourceware.org/bugzilla/show_bug.cgi?id=26945
> > > 
> > > Backported upstream commit
> > > d3edaa91d4cf7202ec14342410194841e2f67f12 to
> > > binutils-2.36 source along with commit id dependencies
> > > (8e03235147a9e774d3ba084e93c2da1aa94d1cec and
> > > 8b69e61d4be276bb862698aaafddc3e779d23c8f).
> > > 
> > > Upstream-Status: Backport [
> > > https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d3edaa91d4cf7202ec14342410194841e2f67f12
> > > ]
> > > 
> > > Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
> > > ---
> > > This patch is verified by building both Linux and Mingw
> > > toolchains for
> > > ARM target, and executed "binutils" regresssion testing.
> > > 
> > >  .../binutils/binutils-2.36.inc                |   1 +
> > >  .../binutils/binutils/CVE-2021-20197.patch    | 388
> > > ++++++++++++++++++
> > >  2 files changed, 389 insertions(+)
> > 
> > This doesn't look like a cherry pick of:
> > 
> > https://github.com/bminor/binutils-gdb/commit/d3edaa91d4cf7202ec14342410194841e2f67f12
> > 
> > What else is in there?
> > 
> > Also this didn't apply on hardknott and I had to rebase it. Please
> > also
> > use b in the subject for binutils.
> > 
> > Thanks,
> > 
> > Anuj
> > 
> > >  create mode 100644 meta/recipes-devtools/binutils/binutils/CVE-
> > > 2021-
> > > 20197.patch
> > > 
> > > diff --git a/meta/recipes-devtools/binutils/binutils-2.36.inc
> > > b/meta/recipes-devtools/binutils/binutils-2.36.inc
> > > index beddf601fd..66080dfcee 100644
> > > --- a/meta/recipes-devtools/binutils/binutils-2.36.inc
> > > +++ b/meta/recipes-devtools/binutils/binutils-2.36.inc
> > > @@ -36,5 +36,6 @@ SRC_URI = "\
> > >       file://0015-sync-with-OE-libtool-changes.patch \
> > >       
> > > file://0016-Check-for-clang-before-checking-gcc-version.patch \
> > > 
> > > file://0017-Add-support-for-the-DW_FORM_strx-forms-to-the-BFD-li.patch
> > > \
> > > +     file://CVE-2021-20197.patch \
> > >  "
> > >  S  = "${WORKDIR}/git"
> > > diff --git a/meta/recipes-devtools/binutils/binutils/CVE-2021-
> > > 20197.patch b/meta/recipes-devtools/binutils/binutils/CVE-2021-
> > > 20197.patch
> > > new file mode 100644
> > > index 0000000000..d6117b65a4
> > > --- /dev/null
> > > +++ b/meta/recipes-devtools/binutils/binutils/CVE-2021-
> > > 20197.patch
> > > @@ -0,0 +1,388 @@
> > > +From d3edaa91d4cf7202ec14342410194841e2f67f12 Mon Sep 17
> > > 00:00:00 2001
> > > +From: Alan Modra <amodra@gmail.com>
> > > +Date: Fri, 26 Feb 2021 11:30:32 +1030
> > > +Subject: [PATCH] Reinstate various pieces backed out from
> > > smart_rename
> > > changes
> > > +
> > > +In the interests of a stable release various last minute
> > > smart_rename
> > > +patches were backed out of the 2.36 branch.  The main reason to
> > > +reinstate some of those backed out changes here is to make
> > > necessary
> > > +followup fixes to commit 8e03235147a9 simple cherry-picks from
> > > +mainline.  A secondary reason is that ar -M support isn't fixed
> > > for
> > > +pr26945 without this patch.
> > > +
> > > +        PR 26945
> > > +        * ar.c: Don't include libbfd.h.
> > > +        (write_archive): Replace xmalloc+strcpy with xstrdup.
> > > +        * arsup.c (temp_name, real_ofd): New static variables.
> > > +        (ar_open): Use make_tempname and bfd_fdopenw.
> > > +        (ar_save): Adjust to suit ar_open changes.
> > > +        * objcopy.c: Don't include libbfd.h.
> > > +        * rename.c: Rename and reorder variables.
> > > +
> > > +(cherry picked from commit
> > > 95b91a043aeaeb546d2fea556d84a2de1e917770)
> > > +
> > > +Upstream-Status: Backport [
> > > https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d3edaa91d4cf7202ec14342410194841e2f67f12
> > > ]
> > > +CVE: CVE-2021-20197
> > > +Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
> > > +---
> > > + binutils/ar.c      |   6 +-
> > > + binutils/arsup.c   |  37 ++++++++----
> > > + binutils/bucomm.h  |   3 +-
> > > + binutils/objcopy.c |   9 +--
> > > + binutils/rename.c  | 148 +++++++++++---------------------------
> > > ------
> > > -
> > > + 5 files changed, 67 insertions(+), 136 deletions(-)
> > > +
> > > +diff --git a/binutils/ar.c b/binutils/ar.c
> > > +index 45a34e3a6cf..44df48c5c67 100644
> > > +--- a/binutils/ar.c
> > > ++++ b/binutils/ar.c
> > > +@@ -25,7 +25,6 @@
> > > +
> > > + #include "sysdep.h"
> > > + #include "bfd.h"
> > > +-#include "libbfd.h"
> > > + #include "libiberty.h"
> > > + #include "progress.h"
> > > + #include "getopt.h"
> > > +@@ -1255,8 +1254,7 @@ write_archive (bfd *iarch)
> > > +   bfd *contents_head = iarch->archive_next;
> > > +   int ofd = -1;
> > > +
> > > +-  old_name = (char *) xmalloc (strlen (bfd_get_filename
> > > (iarch)) +
> > > 1);
> > > +-  strcpy (old_name, bfd_get_filename (iarch));
> > > ++  old_name = xstrdup (bfd_get_filename (iarch));
> > > +   new_name = make_tempname (old_name, &ofd);
> > > +
> > > +   if (new_name == NULL)
> > > +@@ -1308,7 +1306,7 @@ write_archive (bfd *iarch)
> > > +   /* We don't care if this fails; we might be creating the
> > > archive.
> > > */
> > > +   bfd_close (iarch);
> > > +
> > > +-  if (smart_rename (new_name, old_name, 0) != 0)
> > > ++  if (smart_rename (new_name, old_name, NULL) != 0)
> > > +     xexit (1);
> > > +   free (old_name);
> > > +   free (new_name);
> > > +diff --git a/binutils/arsup.c b/binutils/arsup.c
> > > +index 5403a0c5d74..f7ce8f0bc82 100644
> > > +--- a/binutils/arsup.c
> > > ++++ b/binutils/arsup.c
> > > +@@ -42,6 +42,8 @@ extern int deterministic;
> > > +
> > > + static bfd *obfd;
> > > + static char *real_name;
> > > ++static char *temp_name;
> > > ++static int real_ofd;
> > > + static FILE *outfile;
> > > +
> > > + static void
> > > +@@ -149,27 +151,24 @@ maybequit (void)
> > > + void
> > > + ar_open (char *name, int t)
> > > + {
> > > +-  char *tname;
> > > +-  const char *bname = lbasename (name);
> > > +-  real_name = name;
> > > ++  real_name = xstrdup (name);
> > > ++  temp_name = make_tempname (real_name, &real_ofd);
> > > +
> > > +-  /* Prepend tmp- to the beginning, to avoid file-name clashes
> > > after
> > > +-     truncation on filesystems with limited namespaces (DOS). 
> > > */
> > > +-  if (asprintf (&tname, "%.*stmp-%s", (int) (bname - name),
> > > name,
> > > bname) == -1)
> > > ++  if (temp_name == NULL)
> > > +     {
> > > +-      fprintf (stderr, _("%s: Can't allocate memory for temp
> > > name
> > > (%s)\n"),
> > > ++      fprintf (stderr, _("%s: Can't open temporary file
> > > (%s)\n"),
> > > +              program_name, strerror(errno));
> > > +       maybequit ();
> > > +       return;
> > > +     }
> > > +
> > > +-  obfd = bfd_openw (tname, NULL);
> > > ++  obfd = bfd_fdopenw (temp_name, NULL, real_ofd);
> > > +
> > > +   if (!obfd)
> > > +     {
> > > +       fprintf (stderr,
> > > +              _("%s: Can't open output archive %s\n"),
> > > +-             program_name,  tname);
> > > ++             program_name, temp_name);
> > > +
> > > +       maybequit ();
> > > +     }
> > > +@@ -344,16 +343,30 @@ ar_save (void)
> > > +     }
> > > +   else
> > > +     {
> > > +-      char *ofilename = xstrdup (bfd_get_filename (obfd));
> > > ++      struct stat target_stat;
> > > +
> > > +       if (deterministic > 0)
> > > +         obfd->flags |= BFD_DETERMINISTIC_OUTPUT;
> > > +
> > > +       bfd_close (obfd);
> > > +
> > > +-      smart_rename (ofilename, real_name, 0);
> > > ++      if (stat (real_name, &target_stat) != 0)
> > > ++      {
> > > ++        /* The temp file created in ar_open has mode 0600 as
> > > per
> > > mkstemp.
> > > ++           Create the real empty output file here so
> > > smart_rename
> > > will
> > > ++           update the mode according to the process umask.  */
> > > ++        obfd = bfd_openw (real_name, NULL);
> > > ++        if (obfd != NULL)
> > > ++          {
> > > ++            bfd_set_format (obfd, bfd_archive);
> > > ++            bfd_close (obfd);
> > > ++          }
> > > ++      }
> > > ++
> > > ++      smart_rename (temp_name, real_name, NULL);
> > > +       obfd = 0;
> > > +-      free (ofilename);
> > > ++      free (temp_name);
> > > ++      free (real_name);
> > > +     }
> > > + }
> > > +
> > > +diff --git a/binutils/bucomm.h b/binutils/bucomm.h
> > > +index 91f6a5b228f..aa7e33d8cd1 100644
> > > +--- a/binutils/bucomm.h
> > > ++++ b/binutils/bucomm.h
> > > +@@ -71,7 +71,8 @@ extern void print_version (const char *);
> > > + /* In rename.c.  */
> > > + extern void set_times (const char *, const struct stat *);
> > > +
> > > +-extern int smart_rename (const char *, const char *, int);
> > > ++extern int smart_rename (const char *, const char *, struct
> > > stat *);
> > > ++
> > > +
> > > + /* In libiberty.  */
> > > + void *xmalloc (size_t);
> > > +diff --git a/binutils/objcopy.c b/binutils/objcopy.c
> > > +index eab3b6db585..73aa8bc2514 100644
> > > +--- a/binutils/objcopy.c
> > > ++++ b/binutils/objcopy.c
> > > +@@ -20,7 +20,6 @@
> > > +
> > > + #include "sysdep.h"
> > > + #include "bfd.h"
> > > +-#include "libbfd.h"
> > > + #include "progress.h"
> > > + #include "getopt.h"
> > > + #include "libiberty.h"
> > > +@@ -4861,12 +4860,10 @@ strip_main (int argc, char *argv[])
> > > +                output_target, NULL);
> > > +       if (status == 0)
> > > +       {
> > > +-        if (preserve_dates)
> > > +-          set_times (tmpname, &statbuf);
> > > +         if (output_file != tmpname)
> > > +           status = (smart_rename (tmpname,
> > > +                                   output_file ? output_file :
> > > argv[i],
> > > +-                                  preserve_dates) != 0);
> > > ++                                  preserve_dates ? &statbuf :
> > > NULL)
> > > != 0);
> > > +         if (status == 0)
> > > +           status = hold_status;
> > > +       }
> > > +@@ -5931,11 +5928,9 @@ copy_main (int argc, char *argv[])
> > > +            output_target, input_arch);
> > > +   if (status == 0)
> > > +     {
> > > +-      if (preserve_dates)
> > > +-      set_times (tmpname, &statbuf);
> > > +       if (tmpname != output_filename)
> > > +       status = (smart_rename (tmpname, input_filename,
> > > +-                              preserve_dates) != 0);
> > > ++                              preserve_dates ? &statbuf : NULL)
> > > !=
> > > 0);
> > > +     }
> > > +   else
> > > +     unlink_if_ordinary (tmpname);
> > > +diff --git a/binutils/rename.c b/binutils/rename.c
> > > +index 65ad5bf52c4..72a9323d72c 100644
> > > +--- a/binutils/rename.c
> > > ++++ b/binutils/rename.c
> > > +@@ -24,14 +24,9 @@
> > > +
> > > + #ifdef HAVE_GOOD_UTIME_H
> > > + #include <utime.h>
> > > +-#else /* ! HAVE_GOOD_UTIME_H */
> > > +-#ifdef HAVE_UTIMES
> > > ++#elif defined HAVE_UTIMES
> > > + #include <sys/time.h>
> > > +-#endif /* HAVE_UTIMES */
> > > +-#endif /* ! HAVE_GOOD_UTIME_H */
> > > +-
> > > +-#if ! defined (_WIN32) || defined (__CYGWIN32__)
> > > +-static int simple_copy (const char *, const char *);
> > > ++#endif
> > > +
> > > + /* The number of bytes to copy at once.  */
> > > + #define COPY_BUF 8192
> > > +@@ -82,7 +77,6 @@ simple_copy (const char *from, const char *to)
> > > +     }
> > > +   return 0;
> > > + }
> > > +-#endif /* __CYGWIN32__ or not _WIN32 */
> > > +
> > > + /* Set the times of the file DESTINATION to be the same as
> > > those in
> > > +    STATBUF.  */
> > > +@@ -91,122 +85,52 @@ void
> > > + set_times (const char *destination, const struct stat *statbuf)
> > > + {
> > > +   int result;
> > > +-
> > > +-  {
> > > + #ifdef HAVE_GOOD_UTIME_H
> > > +-    struct utimbuf tb;
> > > +-
> > > +-    tb.actime = statbuf->st_atime;
> > > +-    tb.modtime = statbuf->st_mtime;
> > > +-    result = utime (destination, &tb);
> > > +-#else /* ! HAVE_GOOD_UTIME_H */
> > > +-#ifndef HAVE_UTIMES
> > > +-    long tb[2];
> > > +-
> > > +-    tb[0] = statbuf->st_atime;
> > > +-    tb[1] = statbuf->st_mtime;
> > > +-    result = utime (destination, tb);
> > > +-#else /* HAVE_UTIMES */
> > > +-    struct timeval tv[2];
> > > +-
> > > +-    tv[0].tv_sec = statbuf->st_atime;
> > > +-    tv[0].tv_usec = 0;
> > > +-    tv[1].tv_sec = statbuf->st_mtime;
> > > +-    tv[1].tv_usec = 0;
> > > +-    result = utimes (destination, tv);
> > > +-#endif /* HAVE_UTIMES */
> > > +-#endif /* ! HAVE_GOOD_UTIME_H */
> > > +-  }
> > > ++  struct utimbuf tb;
> > > ++
> > > ++  tb.actime = statbuf->st_atime;
> > > ++  tb.modtime = statbuf->st_mtime;
> > > ++  result = utime (destination, &tb);
> > > ++#elif defined HAVE_UTIMES
> > > ++  struct timeval tv[2];
> > > ++
> > > ++  tv[0].tv_sec = statbuf->st_atime;
> > > ++  tv[0].tv_usec = 0;
> > > ++  tv[1].tv_sec = statbuf->st_mtime;
> > > ++  tv[1].tv_usec = 0;
> > > ++  result = utimes (destination, tv);
> > > ++#else
> > > ++  long tb[2];
> > > ++
> > > ++  tb[0] = statbuf->st_atime;
> > > ++  tb[1] = statbuf->st_mtime;
> > > ++  result = utime (destination, tb);
> > > ++#endif
> > > +
> > > +   if (result != 0)
> > > +     non_fatal (_("%s: cannot set time: %s"), destination,
> > > strerror
> > > (errno));
> > > + }
> > > +
> > > +-#ifndef S_ISLNK
> > > +-#ifdef S_IFLNK
> > > +-#define S_ISLNK(m) (((m) & S_IFMT) == S_IFLNK)
> > > +-#else
> > > +-#define S_ISLNK(m) 0
> > > +-#define lstat stat
> > > +-#endif
> > > +-#endif
> > > +-
> > > +-/* Rename FROM to TO, copying if TO is a link.
> > > +-   Return 0 if ok, -1 if error.  */
> > > ++/* Copy FROM to TO.  TARGET_STAT has the file status that, if
> > > non-
> > > NULL,
> > > ++   is used to fix up timestamps.  Return 0 if ok, -1 if error.
> > > ++   At one time this function renamed files, but file
> > > permissions are
> > > ++   tricky to update given the number of different schemes used
> > > by
> > > ++   various systems.  So now we just copy.  */
> > > +
> > > + int
> > > +-smart_rename (const char *from, const char *to, int
> > > preserve_dates
> > > ATTRIBUTE_UNUSED)
> > > ++smart_rename (const char *from, const char *to,
> > > ++            struct stat *target_stat)
> > > + {
> > > +-  bfd_boolean exists;
> > > +-  struct stat s;
> > > +-  int ret = 0;
> > > +-
> > > +-  exists = lstat (to, &s) == 0;
> > > +-
> > > +-#if defined (_WIN32) && !defined (__CYGWIN32__)
> > > +-  /* Win32, unlike unix, will not erase `to' in `rename(from,
> > > to)'
> > > but
> > > +-     fail instead.  Also, chown is not present.  */
> > > ++  int ret;
> > > +
> > > +-  if (exists)
> > > +-    remove (to);
> > > +-
> > > +-  ret = rename (from, to);
> > > ++  ret = simple_copy (from, to);
> > > +   if (ret != 0)
> > > +-    {
> > > +-      /* We have to clean up here.  */
> > > +-      non_fatal (_("unable to rename '%s'; reason: %s"), to,
> > > strerror
> > > (errno));
> > > +-      unlink (from);
> > > +-    }
> > > +-#else
> > > +-  /* Use rename only if TO is not a symbolic link and has
> > > +-     only one hard link, and we have permission to write to
> > > it.  */
> > > +-  if (! exists
> > > +-      || (!S_ISLNK (s.st_mode)
> > > +-        && S_ISREG (s.st_mode)
> > > +-        && (s.st_mode & S_IWUSR)
> > > +-        && s.st_nlink == 1)
> > > +-      )
> > > +-    {
> > > +-      ret = rename (from, to);
> > > +-      if (ret == 0)
> > > +-      {
> > > +-        if (exists)
> > > +-          {
> > > +-            /* Try to preserve the permission bits and
> > > ownership of
> > > +-               TO.  First get the mode right except for the
> > > setuid
> > > +-               bit.  Then change the ownership.  Then fix the
> > > setuid
> > > +-               bit.  We do the chmod before the chown because
> > > if the
> > > +-               chown succeeds, and we are a normal user, we
> > > won't be
> > > +-               able to do the chmod afterward.  We don't bother
> > > to
> > > +-               fix the setuid bit first because that might
> > > introduce
> > > +-               a fleeting security problem, and because the
> > > chown
> > > +-               will clear the setuid bit anyhow.  We only fix
> > > the
> > > +-               setuid bit if the chown succeeds, because we
> > > don't
> > > +-               want to introduce an unexpected setuid file
> > > owned by
> > > +-               the user running objcopy.  */
> > > +-            chmod (to, s.st_mode & 0777);
> > > +-            if (chown (to, s.st_uid, s.st_gid) >= 0)
> > > +-              chmod (to, s.st_mode & 07777);
> > > +-          }
> > > +-      }
> > > +-      else
> > > +-      {
> > > +-        /* We have to clean up here.  */
> > > +-        non_fatal (_("unable to rename '%s'; reason: %s"), to,
> > > strerror (errno));
> > > +-        unlink (from);
> > > +-      }
> > > +-    }
> > > +-  else
> > > +-    {
> > > +-      ret = simple_copy (from, to);
> > > +-      if (ret != 0)
> > > +-      non_fatal (_("unable to copy file '%s'; reason: %s"), to,
> > > strerror (errno));
> > > ++    non_fatal (_("unable to copy file '%s'; reason: %s"),
> > > ++             to, strerror (errno));
> > > +
> > > +-      if (preserve_dates)
> > > +-      set_times (to, &s);
> > > +-      unlink (from);
> > > +-    }
> > > +-#endif /* _WIN32 && !__CYGWIN32__ */
> > > ++  if (target_stat != NULL)
> > > ++    set_times (to, target_stat);
> > > ++  unlink (from);
> > > +
> > > +   return ret;
> > > + }
> > > +--
> > > +2.17.1
> > > +
> > > 
> > > 
> > > 
> > 


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [OE-core] [hardknott][PATCH] Binutils: Fix CVE-2021-20197
  2021-05-07  1:00     ` Anuj Mittal
@ 2021-05-07 14:48       ` Vinay Kumar
  2021-06-29 13:25         ` Randy MacLeod
  0 siblings, 1 reply; 18+ messages in thread
From: Vinay Kumar @ 2021-05-07 14:48 UTC (permalink / raw)
  To: Mittal, Anuj; +Cc: vinay.kumar, openembedded-core, rwmacleod, umesh.kalappa0

Hi Anuj,

>> Right now, the patch header is wrong as it's not the upstream commit that it is claiming to be.

Created 3 patches.
For the patch of commit "d3edaa91d4cf7202ec14342410194841e2f67f12"
planning to use
below patch header from the commit it is cherry picked.
Also, mentioning that in "Upstream-Status" section as below,
====================================
From 95b91a043aeaeb546d2fea556d84a2de1e917770 Mon Sep 17 00:00:00 2001
From: Alan Modra <amodra@gmail.com>
Date: Mon, 1 Feb 2021 02:04:41 +1030
Subject: [PATCH] pr27270 and pr27284, ar segfaults and wrong file mode

        PR 27270
        PR 27284
        PR 26945
        * ar.c: Don't include libbfd.h.
        (write_archive): Replace xmalloc+strcpy with xstrdup.  Use
        bfd_stat rather than fstat on iostream.  Move stat and fd tests
        outside of _WIN32 ifdef.  Delete skip_stat variable.
        * arsup.c (temp_name, real_ofd): New static variables.
        (ar_open): Use make_tempname and bfd_fdopenw.
        (ar_save): Adjust to suit ar_open changes.  Move stat output
        of _WIN32 ifdef.
        * objcopy.c: Don't include libbfd.h.
        (copy_file): Use bfd_stat.

Upstream-Status:
Backport [https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=95b91a043aeaeb546d2fea556d84a2de1e917770]
Used commit "d3edaa91d4cf7202ec14342410194841e2f67f12" cherry picked from commit
"95b91a043aeaeb546d2fea556d84a2de1e917770"
CVE: CVE-2021-20197
Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
====================================

Other 2 commits ids patch headers are identical to the cherry picked
from commits.

Regards,
Vinay

On Fri, May 7, 2021 at 6:30 AM Mittal, Anuj <anuj.mittal@intel.com> wrote:
>
> On Thu, 2021-05-06 at 17:55 +0530, Vinay Kumar wrote:
> > Hi Anuj,
> >
> > Thanks for the feedback.
> >
> > > > This doesn't look like a cherry pick of:
> > > > https://github.com/bminor/binutils-gdb/commit/d3edaa91d4cf7202ec14342410194841e2f67f12
> > > > What else is in there?
> >
> > cherry picked commit ids in the below order to "SRCREV" of binutils-
> > 2.36.inc.
> >
> > 1. 8e03235147a9e774d3ba084e93c2da1aa94d1cec
> > 2. d3edaa91d4cf7202ec14342410194841e2f67f12
> > 3. 8b69e61d4be276bb862698aaafddc3e779d23c8f
> >
> > Direct cherry-pick of "d3edaa91d4cf7202ec14342410194841e2f67f12" into
> > "SRCREV" gives errors due to the changes in arsup.c file. So used
> > 8e03235147a9e774d3ba084e93c2da1aa94d1cec (binutils: Avoid renaming
> > over existing files).
> >
> > The commit id 8b69e61d4be276bb862698aaafddc3e779d23c8f is
> > to fix the binutils build error with Mingw.
>
> I think it should be 3 separate patches in that case. Right now, the
> patch header is wrong as it's not the upstream commit that it is
> claiming to be.
>
> Thanks,
>
> Anuj
>
> >
> > > > Also this didn't apply on hardknott and I had to rebase it.
> > > > Please also
> > > > use b in the subject for binutils.
> >
> > Will send a v2 patch with corrections.
> >
> > Regards,
> > Vinay
> >
> > On Thu, May 6, 2021 at 7:27 AM Mittal, Anuj <anuj.mittal@intel.com>
> > wrote:
> > >
> > > On Thu, 2021-04-29 at 04:30 -0700, Vinay Kumar wrote:
> > > > Source: git://sourceware.org/git/binutils-gdb.git
> > > > Tracking -- https://sourceware.org/bugzilla/show_bug.cgi?id=26945
> > > >
> > > > Backported upstream commit
> > > > d3edaa91d4cf7202ec14342410194841e2f67f12 to
> > > > binutils-2.36 source along with commit id dependencies
> > > > (8e03235147a9e774d3ba084e93c2da1aa94d1cec and
> > > > 8b69e61d4be276bb862698aaafddc3e779d23c8f).
> > > >
> > > > Upstream-Status: Backport [
> > > > https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d3edaa91d4cf7202ec14342410194841e2f67f12
> > > > ]
> > > >
> > > > Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
> > > > ---
> > > > This patch is verified by building both Linux and Mingw
> > > > toolchains for
> > > > ARM target, and executed "binutils" regresssion testing.
> > > >
> > > >  .../binutils/binutils-2.36.inc                |   1 +
> > > >  .../binutils/binutils/CVE-2021-20197.patch    | 388
> > > > ++++++++++++++++++
> > > >  2 files changed, 389 insertions(+)
> > >
> > > This doesn't look like a cherry pick of:
> > >
> > > https://github.com/bminor/binutils-gdb/commit/d3edaa91d4cf7202ec14342410194841e2f67f12
> > >
> > > What else is in there?
> > >
> > > Also this didn't apply on hardknott and I had to rebase it. Please
> > > also
> > > use b in the subject for binutils.
> > >
> > > Thanks,
> > >
> > > Anuj
> > >
> > > >  create mode 100644 meta/recipes-devtools/binutils/binutils/CVE-
> > > > 2021-
> > > > 20197.patch
> > > >
> > > > diff --git a/meta/recipes-devtools/binutils/binutils-2.36.inc
> > > > b/meta/recipes-devtools/binutils/binutils-2.36.inc
> > > > index beddf601fd..66080dfcee 100644
> > > > --- a/meta/recipes-devtools/binutils/binutils-2.36.inc
> > > > +++ b/meta/recipes-devtools/binutils/binutils-2.36.inc
> > > > @@ -36,5 +36,6 @@ SRC_URI = "\
> > > >       file://0015-sync-with-OE-libtool-changes.patch \
> > > >
> > > > file://0016-Check-for-clang-before-checking-gcc-version.patch \
> > > >
> > > > file://0017-Add-support-for-the-DW_FORM_strx-forms-to-the-BFD-li.patch
> > > > \
> > > > +     file://CVE-2021-20197.patch \
> > > >  "
> > > >  S  = "${WORKDIR}/git"
> > > > diff --git a/meta/recipes-devtools/binutils/binutils/CVE-2021-
> > > > 20197.patch b/meta/recipes-devtools/binutils/binutils/CVE-2021-
> > > > 20197.patch
> > > > new file mode 100644
> > > > index 0000000000..d6117b65a4
> > > > --- /dev/null
> > > > +++ b/meta/recipes-devtools/binutils/binutils/CVE-2021-
> > > > 20197.patch
> > > > @@ -0,0 +1,388 @@
> > > > +From d3edaa91d4cf7202ec14342410194841e2f67f12 Mon Sep 17
> > > > 00:00:00 2001
> > > > +From: Alan Modra <amodra@gmail.com>
> > > > +Date: Fri, 26 Feb 2021 11:30:32 +1030
> > > > +Subject: [PATCH] Reinstate various pieces backed out from
> > > > smart_rename
> > > > changes
> > > > +
> > > > +In the interests of a stable release various last minute
> > > > smart_rename
> > > > +patches were backed out of the 2.36 branch.  The main reason to
> > > > +reinstate some of those backed out changes here is to make
> > > > necessary
> > > > +followup fixes to commit 8e03235147a9 simple cherry-picks from
> > > > +mainline.  A secondary reason is that ar -M support isn't fixed
> > > > for
> > > > +pr26945 without this patch.
> > > > +
> > > > +        PR 26945
> > > > +        * ar.c: Don't include libbfd.h.
> > > > +        (write_archive): Replace xmalloc+strcpy with xstrdup.
> > > > +        * arsup.c (temp_name, real_ofd): New static variables.
> > > > +        (ar_open): Use make_tempname and bfd_fdopenw.
> > > > +        (ar_save): Adjust to suit ar_open changes.
> > > > +        * objcopy.c: Don't include libbfd.h.
> > > > +        * rename.c: Rename and reorder variables.
> > > > +
> > > > +(cherry picked from commit
> > > > 95b91a043aeaeb546d2fea556d84a2de1e917770)
> > > > +
> > > > +Upstream-Status: Backport [
> > > > https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d3edaa91d4cf7202ec14342410194841e2f67f12
> > > > ]
> > > > +CVE: CVE-2021-20197
> > > > +Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
> > > > +---
> > > > + binutils/ar.c      |   6 +-
> > > > + binutils/arsup.c   |  37 ++++++++----
> > > > + binutils/bucomm.h  |   3 +-
> > > > + binutils/objcopy.c |   9 +--
> > > > + binutils/rename.c  | 148 +++++++++++---------------------------
> > > > ------
> > > > -
> > > > + 5 files changed, 67 insertions(+), 136 deletions(-)
> > > > +
> > > > +diff --git a/binutils/ar.c b/binutils/ar.c
> > > > +index 45a34e3a6cf..44df48c5c67 100644
> > > > +--- a/binutils/ar.c
> > > > ++++ b/binutils/ar.c
> > > > +@@ -25,7 +25,6 @@
> > > > +
> > > > + #include "sysdep.h"
> > > > + #include "bfd.h"
> > > > +-#include "libbfd.h"
> > > > + #include "libiberty.h"
> > > > + #include "progress.h"
> > > > + #include "getopt.h"
> > > > +@@ -1255,8 +1254,7 @@ write_archive (bfd *iarch)
> > > > +   bfd *contents_head = iarch->archive_next;
> > > > +   int ofd = -1;
> > > > +
> > > > +-  old_name = (char *) xmalloc (strlen (bfd_get_filename
> > > > (iarch)) +
> > > > 1);
> > > > +-  strcpy (old_name, bfd_get_filename (iarch));
> > > > ++  old_name = xstrdup (bfd_get_filename (iarch));
> > > > +   new_name = make_tempname (old_name, &ofd);
> > > > +
> > > > +   if (new_name == NULL)
> > > > +@@ -1308,7 +1306,7 @@ write_archive (bfd *iarch)
> > > > +   /* We don't care if this fails; we might be creating the
> > > > archive.
> > > > */
> > > > +   bfd_close (iarch);
> > > > +
> > > > +-  if (smart_rename (new_name, old_name, 0) != 0)
> > > > ++  if (smart_rename (new_name, old_name, NULL) != 0)
> > > > +     xexit (1);
> > > > +   free (old_name);
> > > > +   free (new_name);
> > > > +diff --git a/binutils/arsup.c b/binutils/arsup.c
> > > > +index 5403a0c5d74..f7ce8f0bc82 100644
> > > > +--- a/binutils/arsup.c
> > > > ++++ b/binutils/arsup.c
> > > > +@@ -42,6 +42,8 @@ extern int deterministic;
> > > > +
> > > > + static bfd *obfd;
> > > > + static char *real_name;
> > > > ++static char *temp_name;
> > > > ++static int real_ofd;
> > > > + static FILE *outfile;
> > > > +
> > > > + static void
> > > > +@@ -149,27 +151,24 @@ maybequit (void)
> > > > + void
> > > > + ar_open (char *name, int t)
> > > > + {
> > > > +-  char *tname;
> > > > +-  const char *bname = lbasename (name);
> > > > +-  real_name = name;
> > > > ++  real_name = xstrdup (name);
> > > > ++  temp_name = make_tempname (real_name, &real_ofd);
> > > > +
> > > > +-  /* Prepend tmp- to the beginning, to avoid file-name clashes
> > > > after
> > > > +-     truncation on filesystems with limited namespaces (DOS).
> > > > */
> > > > +-  if (asprintf (&tname, "%.*stmp-%s", (int) (bname - name),
> > > > name,
> > > > bname) == -1)
> > > > ++  if (temp_name == NULL)
> > > > +     {
> > > > +-      fprintf (stderr, _("%s: Can't allocate memory for temp
> > > > name
> > > > (%s)\n"),
> > > > ++      fprintf (stderr, _("%s: Can't open temporary file
> > > > (%s)\n"),
> > > > +              program_name, strerror(errno));
> > > > +       maybequit ();
> > > > +       return;
> > > > +     }
> > > > +
> > > > +-  obfd = bfd_openw (tname, NULL);
> > > > ++  obfd = bfd_fdopenw (temp_name, NULL, real_ofd);
> > > > +
> > > > +   if (!obfd)
> > > > +     {
> > > > +       fprintf (stderr,
> > > > +              _("%s: Can't open output archive %s\n"),
> > > > +-             program_name,  tname);
> > > > ++             program_name, temp_name);
> > > > +
> > > > +       maybequit ();
> > > > +     }
> > > > +@@ -344,16 +343,30 @@ ar_save (void)
> > > > +     }
> > > > +   else
> > > > +     {
> > > > +-      char *ofilename = xstrdup (bfd_get_filename (obfd));
> > > > ++      struct stat target_stat;
> > > > +
> > > > +       if (deterministic > 0)
> > > > +         obfd->flags |= BFD_DETERMINISTIC_OUTPUT;
> > > > +
> > > > +       bfd_close (obfd);
> > > > +
> > > > +-      smart_rename (ofilename, real_name, 0);
> > > > ++      if (stat (real_name, &target_stat) != 0)
> > > > ++      {
> > > > ++        /* The temp file created in ar_open has mode 0600 as
> > > > per
> > > > mkstemp.
> > > > ++           Create the real empty output file here so
> > > > smart_rename
> > > > will
> > > > ++           update the mode according to the process umask.  */
> > > > ++        obfd = bfd_openw (real_name, NULL);
> > > > ++        if (obfd != NULL)
> > > > ++          {
> > > > ++            bfd_set_format (obfd, bfd_archive);
> > > > ++            bfd_close (obfd);
> > > > ++          }
> > > > ++      }
> > > > ++
> > > > ++      smart_rename (temp_name, real_name, NULL);
> > > > +       obfd = 0;
> > > > +-      free (ofilename);
> > > > ++      free (temp_name);
> > > > ++      free (real_name);
> > > > +     }
> > > > + }
> > > > +
> > > > +diff --git a/binutils/bucomm.h b/binutils/bucomm.h
> > > > +index 91f6a5b228f..aa7e33d8cd1 100644
> > > > +--- a/binutils/bucomm.h
> > > > ++++ b/binutils/bucomm.h
> > > > +@@ -71,7 +71,8 @@ extern void print_version (const char *);
> > > > + /* In rename.c.  */
> > > > + extern void set_times (const char *, const struct stat *);
> > > > +
> > > > +-extern int smart_rename (const char *, const char *, int);
> > > > ++extern int smart_rename (const char *, const char *, struct
> > > > stat *);
> > > > ++
> > > > +
> > > > + /* In libiberty.  */
> > > > + void *xmalloc (size_t);
> > > > +diff --git a/binutils/objcopy.c b/binutils/objcopy.c
> > > > +index eab3b6db585..73aa8bc2514 100644
> > > > +--- a/binutils/objcopy.c
> > > > ++++ b/binutils/objcopy.c
> > > > +@@ -20,7 +20,6 @@
> > > > +
> > > > + #include "sysdep.h"
> > > > + #include "bfd.h"
> > > > +-#include "libbfd.h"
> > > > + #include "progress.h"
> > > > + #include "getopt.h"
> > > > + #include "libiberty.h"
> > > > +@@ -4861,12 +4860,10 @@ strip_main (int argc, char *argv[])
> > > > +                output_target, NULL);
> > > > +       if (status == 0)
> > > > +       {
> > > > +-        if (preserve_dates)
> > > > +-          set_times (tmpname, &statbuf);
> > > > +         if (output_file != tmpname)
> > > > +           status = (smart_rename (tmpname,
> > > > +                                   output_file ? output_file :
> > > > argv[i],
> > > > +-                                  preserve_dates) != 0);
> > > > ++                                  preserve_dates ? &statbuf :
> > > > NULL)
> > > > != 0);
> > > > +         if (status == 0)
> > > > +           status = hold_status;
> > > > +       }
> > > > +@@ -5931,11 +5928,9 @@ copy_main (int argc, char *argv[])
> > > > +            output_target, input_arch);
> > > > +   if (status == 0)
> > > > +     {
> > > > +-      if (preserve_dates)
> > > > +-      set_times (tmpname, &statbuf);
> > > > +       if (tmpname != output_filename)
> > > > +       status = (smart_rename (tmpname, input_filename,
> > > > +-                              preserve_dates) != 0);
> > > > ++                              preserve_dates ? &statbuf : NULL)
> > > > !=
> > > > 0);
> > > > +     }
> > > > +   else
> > > > +     unlink_if_ordinary (tmpname);
> > > > +diff --git a/binutils/rename.c b/binutils/rename.c
> > > > +index 65ad5bf52c4..72a9323d72c 100644
> > > > +--- a/binutils/rename.c
> > > > ++++ b/binutils/rename.c
> > > > +@@ -24,14 +24,9 @@
> > > > +
> > > > + #ifdef HAVE_GOOD_UTIME_H
> > > > + #include <utime.h>
> > > > +-#else /* ! HAVE_GOOD_UTIME_H */
> > > > +-#ifdef HAVE_UTIMES
> > > > ++#elif defined HAVE_UTIMES
> > > > + #include <sys/time.h>
> > > > +-#endif /* HAVE_UTIMES */
> > > > +-#endif /* ! HAVE_GOOD_UTIME_H */
> > > > +-
> > > > +-#if ! defined (_WIN32) || defined (__CYGWIN32__)
> > > > +-static int simple_copy (const char *, const char *);
> > > > ++#endif
> > > > +
> > > > + /* The number of bytes to copy at once.  */
> > > > + #define COPY_BUF 8192
> > > > +@@ -82,7 +77,6 @@ simple_copy (const char *from, const char *to)
> > > > +     }
> > > > +   return 0;
> > > > + }
> > > > +-#endif /* __CYGWIN32__ or not _WIN32 */
> > > > +
> > > > + /* Set the times of the file DESTINATION to be the same as
> > > > those in
> > > > +    STATBUF.  */
> > > > +@@ -91,122 +85,52 @@ void
> > > > + set_times (const char *destination, const struct stat *statbuf)
> > > > + {
> > > > +   int result;
> > > > +-
> > > > +-  {
> > > > + #ifdef HAVE_GOOD_UTIME_H
> > > > +-    struct utimbuf tb;
> > > > +-
> > > > +-    tb.actime = statbuf->st_atime;
> > > > +-    tb.modtime = statbuf->st_mtime;
> > > > +-    result = utime (destination, &tb);
> > > > +-#else /* ! HAVE_GOOD_UTIME_H */
> > > > +-#ifndef HAVE_UTIMES
> > > > +-    long tb[2];
> > > > +-
> > > > +-    tb[0] = statbuf->st_atime;
> > > > +-    tb[1] = statbuf->st_mtime;
> > > > +-    result = utime (destination, tb);
> > > > +-#else /* HAVE_UTIMES */
> > > > +-    struct timeval tv[2];
> > > > +-
> > > > +-    tv[0].tv_sec = statbuf->st_atime;
> > > > +-    tv[0].tv_usec = 0;
> > > > +-    tv[1].tv_sec = statbuf->st_mtime;
> > > > +-    tv[1].tv_usec = 0;
> > > > +-    result = utimes (destination, tv);
> > > > +-#endif /* HAVE_UTIMES */
> > > > +-#endif /* ! HAVE_GOOD_UTIME_H */
> > > > +-  }
> > > > ++  struct utimbuf tb;
> > > > ++
> > > > ++  tb.actime = statbuf->st_atime;
> > > > ++  tb.modtime = statbuf->st_mtime;
> > > > ++  result = utime (destination, &tb);
> > > > ++#elif defined HAVE_UTIMES
> > > > ++  struct timeval tv[2];
> > > > ++
> > > > ++  tv[0].tv_sec = statbuf->st_atime;
> > > > ++  tv[0].tv_usec = 0;
> > > > ++  tv[1].tv_sec = statbuf->st_mtime;
> > > > ++  tv[1].tv_usec = 0;
> > > > ++  result = utimes (destination, tv);
> > > > ++#else
> > > > ++  long tb[2];
> > > > ++
> > > > ++  tb[0] = statbuf->st_atime;
> > > > ++  tb[1] = statbuf->st_mtime;
> > > > ++  result = utime (destination, tb);
> > > > ++#endif
> > > > +
> > > > +   if (result != 0)
> > > > +     non_fatal (_("%s: cannot set time: %s"), destination,
> > > > strerror
> > > > (errno));
> > > > + }
> > > > +
> > > > +-#ifndef S_ISLNK
> > > > +-#ifdef S_IFLNK
> > > > +-#define S_ISLNK(m) (((m) & S_IFMT) == S_IFLNK)
> > > > +-#else
> > > > +-#define S_ISLNK(m) 0
> > > > +-#define lstat stat
> > > > +-#endif
> > > > +-#endif
> > > > +-
> > > > +-/* Rename FROM to TO, copying if TO is a link.
> > > > +-   Return 0 if ok, -1 if error.  */
> > > > ++/* Copy FROM to TO.  TARGET_STAT has the file status that, if
> > > > non-
> > > > NULL,
> > > > ++   is used to fix up timestamps.  Return 0 if ok, -1 if error.
> > > > ++   At one time this function renamed files, but file
> > > > permissions are
> > > > ++   tricky to update given the number of different schemes used
> > > > by
> > > > ++   various systems.  So now we just copy.  */
> > > > +
> > > > + int
> > > > +-smart_rename (const char *from, const char *to, int
> > > > preserve_dates
> > > > ATTRIBUTE_UNUSED)
> > > > ++smart_rename (const char *from, const char *to,
> > > > ++            struct stat *target_stat)
> > > > + {
> > > > +-  bfd_boolean exists;
> > > > +-  struct stat s;
> > > > +-  int ret = 0;
> > > > +-
> > > > +-  exists = lstat (to, &s) == 0;
> > > > +-
> > > > +-#if defined (_WIN32) && !defined (__CYGWIN32__)
> > > > +-  /* Win32, unlike unix, will not erase `to' in `rename(from,
> > > > to)'
> > > > but
> > > > +-     fail instead.  Also, chown is not present.  */
> > > > ++  int ret;
> > > > +
> > > > +-  if (exists)
> > > > +-    remove (to);
> > > > +-
> > > > +-  ret = rename (from, to);
> > > > ++  ret = simple_copy (from, to);
> > > > +   if (ret != 0)
> > > > +-    {
> > > > +-      /* We have to clean up here.  */
> > > > +-      non_fatal (_("unable to rename '%s'; reason: %s"), to,
> > > > strerror
> > > > (errno));
> > > > +-      unlink (from);
> > > > +-    }
> > > > +-#else
> > > > +-  /* Use rename only if TO is not a symbolic link and has
> > > > +-     only one hard link, and we have permission to write to
> > > > it.  */
> > > > +-  if (! exists
> > > > +-      || (!S_ISLNK (s.st_mode)
> > > > +-        && S_ISREG (s.st_mode)
> > > > +-        && (s.st_mode & S_IWUSR)
> > > > +-        && s.st_nlink == 1)
> > > > +-      )
> > > > +-    {
> > > > +-      ret = rename (from, to);
> > > > +-      if (ret == 0)
> > > > +-      {
> > > > +-        if (exists)
> > > > +-          {
> > > > +-            /* Try to preserve the permission bits and
> > > > ownership of
> > > > +-               TO.  First get the mode right except for the
> > > > setuid
> > > > +-               bit.  Then change the ownership.  Then fix the
> > > > setuid
> > > > +-               bit.  We do the chmod before the chown because
> > > > if the
> > > > +-               chown succeeds, and we are a normal user, we
> > > > won't be
> > > > +-               able to do the chmod afterward.  We don't bother
> > > > to
> > > > +-               fix the setuid bit first because that might
> > > > introduce
> > > > +-               a fleeting security problem, and because the
> > > > chown
> > > > +-               will clear the setuid bit anyhow.  We only fix
> > > > the
> > > > +-               setuid bit if the chown succeeds, because we
> > > > don't
> > > > +-               want to introduce an unexpected setuid file
> > > > owned by
> > > > +-               the user running objcopy.  */
> > > > +-            chmod (to, s.st_mode & 0777);
> > > > +-            if (chown (to, s.st_uid, s.st_gid) >= 0)
> > > > +-              chmod (to, s.st_mode & 07777);
> > > > +-          }
> > > > +-      }
> > > > +-      else
> > > > +-      {
> > > > +-        /* We have to clean up here.  */
> > > > +-        non_fatal (_("unable to rename '%s'; reason: %s"), to,
> > > > strerror (errno));
> > > > +-        unlink (from);
> > > > +-      }
> > > > +-    }
> > > > +-  else
> > > > +-    {
> > > > +-      ret = simple_copy (from, to);
> > > > +-      if (ret != 0)
> > > > +-      non_fatal (_("unable to copy file '%s'; reason: %s"), to,
> > > > strerror (errno));
> > > > ++    non_fatal (_("unable to copy file '%s'; reason: %s"),
> > > > ++             to, strerror (errno));
> > > > +
> > > > +-      if (preserve_dates)
> > > > +-      set_times (to, &s);
> > > > +-      unlink (from);
> > > > +-    }
> > > > +-#endif /* _WIN32 && !__CYGWIN32__ */
> > > > ++  if (target_stat != NULL)
> > > > ++    set_times (to, target_stat);
> > > > ++  unlink (from);
> > > > +
> > > > +   return ret;
> > > > + }
> > > > +--
> > > > +2.17.1
> > > > +
> > > >
> > > > 
> > > >
> > >
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [OE-core] [hardknott][PATCH] Binutils: Fix CVE-2021-20197
  2021-05-07 14:48       ` Vinay Kumar
@ 2021-06-29 13:25         ` Randy MacLeod
  2021-07-02 11:22           ` [hardknott][PATCH v3] binutils: " Vinay Kumar
  2021-07-02 11:39           ` [OE-core] [hardknott][PATCH] Binutils: " Vinay Kumar
  0 siblings, 2 replies; 18+ messages in thread
From: Randy MacLeod @ 2021-06-29 13:25 UTC (permalink / raw)
  To: Vinay Kumar, Mittal, Anuj
  Cc: vinay.kumar, openembedded-core, rwmacleod, umesh.kalappa0

On 2021-05-07 10:48 a.m., Vinay Kumar wrote:
> Hi Anuj,
> 
>>> Right now, the patch header is wrong as it's not the upstream commit that it is claiming to be.
> 
> Created 3 patches.
> For the patch of commit "d3edaa91d4cf7202ec14342410194841e2f67f12"
> planning to use
> below patch header from the commit it is cherry picked.
> Also, mentioning that in "Upstream-Status" section as below,
> ====================================
>>>From 95b91a043aeaeb546d2fea556d84a2de1e917770 Mon Sep 17 00:00:00 2001
> From: Alan Modra <amodra@gmail.com>
> Date: Mon, 1 Feb 2021 02:04:41 +1030
> Subject: [PATCH] pr27270 and pr27284, ar segfaults and wrong file mode
> 
>          PR 27270
>          PR 27284
>          PR 26945
>          * ar.c: Don't include libbfd.h.
>          (write_archive): Replace xmalloc+strcpy with xstrdup.  Use
>          bfd_stat rather than fstat on iostream.  Move stat and fd tests
>          outside of _WIN32 ifdef.  Delete skip_stat variable.
>          * arsup.c (temp_name, real_ofd): New static variables.
>          (ar_open): Use make_tempname and bfd_fdopenw.
>          (ar_save): Adjust to suit ar_open changes.  Move stat output
>          of _WIN32 ifdef.
>          * objcopy.c: Don't include libbfd.h.
>          (copy_file): Use bfd_stat.
> 
> Upstream-Status:
> Backport [https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=95b91a043aeaeb546d2fea556d84a2de1e917770]
> Used commit "d3edaa91d4cf7202ec14342410194841e2f67f12" cherry picked from commit
> "95b91a043aeaeb546d2fea556d84a2de1e917770"
> CVE: CVE-2021-20197
> Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
> ====================================
> 
> Other 2 commits ids patch headers are identical to the cherry picked
> from commits.
> 
> Regards,
> Vinay
> 

Seems that this patch was not merged to hardknott.
I didn't see the version split into 3 patch on the list.

Vinay, can you please re-submit if you agree
that splitting it up into 3 patches makes sense.

-- 
# Randy MacLeod
# Wind River Linux

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [hardknott][PATCH v3] binutils: Fix CVE-2021-20197
  2021-06-29 13:25         ` Randy MacLeod
@ 2021-07-02 11:22           ` Vinay Kumar
  2021-07-13 13:46             ` [hardknott][PATCH v4] " Vinay Kumar
       [not found]             ` <16915DBBAEEB81DE.15464@lists.openembedded.org>
  2021-07-02 11:39           ` [OE-core] [hardknott][PATCH] Binutils: " Vinay Kumar
  1 sibling, 2 replies; 18+ messages in thread
From: Vinay Kumar @ 2021-07-02 11:22 UTC (permalink / raw)
  To: anuj.mittal
  Cc: richard.purdie, openembedded-core, rwmacleod, umesh.kalappa0,
	vinay.kumar, Vinay Kumar

Source: git://sourceware.org/git/binutils-gdb.git
Tracking -- https://sourceware.org/bugzilla/show_bug.cgi?id=26945

Backported upstream commit id d3edaa91d4cf7202ec14342410194841e2f67f12
and its dependent commits 8e03235147a9e774d3ba084e93c2da1aa94d1cec and
8b69e61d4be276bb862698aaafddc3e779d23c8f to binutils-2.36 source.

Upstream-Status: Backport [https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=8e03235147a9e774d3ba084e93c2da1aa94d1cec]
Upstream-Status: Backport [https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d3edaa91d4cf7202ec14342410194841e2f67f12]
Upstream-Status: Backport [https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=8b69e61d4be276bb862698aaafddc3e779d23c8f]

Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
---
This patch is verified by building both Linux and Mingw toolchains for ARM target.

Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
---
 .../binutils/binutils-2.36.inc                |   3 +
 .../binutils/0001-CVE-2021-20197.patch        | 201 ++++++++++++++++++
 .../binutils/0002-CVE-2021-20197.patch        | 170 +++++++++++++++
 .../binutils/0003-CVE-2021-20197.patch        | 171 +++++++++++++++
 4 files changed, 545 insertions(+)
 create mode 100644 meta/recipes-devtools/binutils/binutils/0001-CVE-2021-20197.patch
 create mode 100644 meta/recipes-devtools/binutils/binutils/0002-CVE-2021-20197.patch
 create mode 100644 meta/recipes-devtools/binutils/binutils/0003-CVE-2021-20197.patch

diff --git a/meta/recipes-devtools/binutils/binutils-2.36.inc b/meta/recipes-devtools/binutils/binutils-2.36.inc
index 2968291889..9d770db5a8 100644
--- a/meta/recipes-devtools/binutils/binutils-2.36.inc
+++ b/meta/recipes-devtools/binutils/binutils-2.36.inc
@@ -41,5 +41,8 @@ SRC_URI = "\
      file://0014-Fix-rpath-in-libtool-when-sysroot-is-enabled.patch \
      file://0015-sync-with-OE-libtool-changes.patch \
      file://0016-Check-for-clang-before-checking-gcc-version.patch \
+     file://0001-CVE-2021-20197.patch \
+     file://0002-CVE-2021-20197.patch \
+     file://0003-CVE-2021-20197.patch \
 "
 S  = "${WORKDIR}/git"
diff --git a/meta/recipes-devtools/binutils/binutils/0001-CVE-2021-20197.patch b/meta/recipes-devtools/binutils/binutils/0001-CVE-2021-20197.patch
new file mode 100644
index 0000000000..2b4eaba26d
--- /dev/null
+++ b/meta/recipes-devtools/binutils/binutils/0001-CVE-2021-20197.patch
@@ -0,0 +1,201 @@
+From 8e03235147a9e774d3ba084e93c2da1aa94d1cec Mon Sep 17 00:00:00 2001
+From: Siddhesh Poyarekar <siddhesh@gotplt.org>
+Date: Mon, 22 Feb 2021 20:45:50 +0530
+Subject: [PATCH] binutils: Avoid renaming over existing files
+
+Renaming over existing files needs additional care to restore
+permissions and ownership, which may not always succeed.
+Additionally, other properties of the file such as extended attributes
+may be lost, making the operation flaky.
+
+For predictable results, resort to rename() only if the file does not
+exist, otherwise copy the file contents into the existing file.  This
+ensures that no additional tricks are needed to retain file
+properties.
+
+This also allows dropping of the redundant set_times on the tmpfile in
+objcopy/strip since now we no longer rename over existing files.
+
+binutils/
+
+	* ar.c (write_archive): Adjust call to SMART_RENAME.
+	* arsup.c (ar_save): Likewise.
+	* objcopy (strip_main): Don't set times on temporary file and
+	adjust call to SMART_RENAME.
+	(copy_main): Likewise.
+	* rename.c [!S_ISLNK]: Remove definitions.
+	(try_preserve_permissions): Remove function.
+	(smart_rename): Replace PRESERVE_DATES argument with
+	TARGET_STAT.  Use rename system call only if TO does not exist.
+	* bucomm.h (smart_rename): Adjust declaration.
+
+(cherry picked from commit 3685de750e6a091663a0abe42528cad29e960e35)
+
+Upstream-Status: Backport [https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=8e03235147a9e774d3ba084e93c2da1aa94d1cec]
+CVE: CVE-2021-20197
+Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
+---
+ binutils/ar.c      |  2 +-
+ binutils/arsup.c   |  2 +-
+ binutils/bucomm.h  |  3 ++-
+ binutils/objcopy.c |  8 ++-----
+ binutils/rename.c  | 55 +++++++++-------------------------------------
+ 6 files changed, 29 insertions(+), 54 deletions(-)
+
+diff --git a/binutils/ar.c b/binutils/ar.c
+index 45a34e3a6cf..3a91708b51c 100644
+--- a/binutils/ar.c
++++ b/binutils/ar.c
+@@ -1308,7 +1308,7 @@ write_archive (bfd *iarch)
+   /* We don't care if this fails; we might be creating the archive.  */
+   bfd_close (iarch);
+ 
+-  if (smart_rename (new_name, old_name, 0) != 0)
++  if (smart_rename (new_name, old_name, NULL) != 0)
+     xexit (1);
+   free (old_name);
+   free (new_name);
+diff --git a/binutils/arsup.c b/binutils/arsup.c
+index 5403a0c5d74..0a1f63f6456 100644
+--- a/binutils/arsup.c
++++ b/binutils/arsup.c
+@@ -351,7 +351,7 @@ ar_save (void)
+ 
+       bfd_close (obfd);
+ 
+-      smart_rename (ofilename, real_name, 0);
++      smart_rename (ofilename, real_name, NULL);
+       obfd = 0;
+       free (ofilename);
+     }
+diff --git a/binutils/bucomm.h b/binutils/bucomm.h
+index 91f6a5b228f..aa7e33d8cd1 100644
+--- a/binutils/bucomm.h
++++ b/binutils/bucomm.h
+@@ -71,7 +71,8 @@ extern void print_version (const char *);
+ /* In rename.c.  */
+ extern void set_times (const char *, const struct stat *);
+ 
+-extern int smart_rename (const char *, const char *, int);
++extern int smart_rename (const char *, const char *, struct stat *);
++
+ 
+ /* In libiberty.  */
+ void *xmalloc (size_t);
+diff --git a/binutils/objcopy.c b/binutils/objcopy.c
+index eab3b6db585..07a872b5a80 100644
+--- a/binutils/objcopy.c
++++ b/binutils/objcopy.c
+@@ -4861,12 +4861,10 @@ strip_main (int argc, char *argv[])
+ 		 output_target, NULL);
+       if (status == 0)
+ 	{
+-	  if (preserve_dates)
+-	    set_times (tmpname, &statbuf);
+ 	  if (output_file != tmpname)
+ 	    status = (smart_rename (tmpname,
+ 				    output_file ? output_file : argv[i],
+-				    preserve_dates) != 0);
++				    preserve_dates ? &statbuf : NULL) != 0);
+ 	  if (status == 0)
+ 	    status = hold_status;
+ 	}
+@@ -5931,11 +5929,9 @@ copy_main (int argc, char *argv[])
+ 	     output_target, input_arch);
+   if (status == 0)
+     {
+-      if (preserve_dates)
+-	set_times (tmpname, &statbuf);
+       if (tmpname != output_filename)
+ 	status = (smart_rename (tmpname, input_filename,
+-				preserve_dates) != 0);
++				preserve_dates ? &statbuf : NULL) != 0);
+     }
+   else
+     unlink_if_ordinary (tmpname);
+diff --git a/binutils/rename.c b/binutils/rename.c
+index 65ad5bf52c4..f471b45fd3f 100644
+--- a/binutils/rename.c
++++ b/binutils/rename.c
+@@ -122,20 +122,13 @@ set_times (const char *destination, const struct stat *statbuf)
+     non_fatal (_("%s: cannot set time: %s"), destination, strerror (errno));
+ }
+ 
+-#ifndef S_ISLNK
+-#ifdef S_IFLNK
+-#define S_ISLNK(m) (((m) & S_IFMT) == S_IFLNK)
+-#else
+-#define S_ISLNK(m) 0
+-#define lstat stat
+-#endif
+-#endif
+-
+-/* Rename FROM to TO, copying if TO is a link.
+-   Return 0 if ok, -1 if error.  */
++/* Rename FROM to TO, copying if TO exists.  TARGET_STAT has the file status
++   that, if non-NULL, is used to fix up timestamps after rename.  Return 0 if
++   ok, -1 if error.  */
+ 
+ int
+-smart_rename (const char *from, const char *to, int preserve_dates ATTRIBUTE_UNUSED)
++smart_rename (const char *from, const char *to,
++	      struct stat *target_stat ATTRIBUTE_UNUSED)
+ {
+   bfd_boolean exists;
+   struct stat s;
+@@ -158,38 +151,10 @@ smart_rename (const char *from, const char *to, int preserve_dates ATTRIBUTE_UNU
+       unlink (from);
+     }
+ #else
+-  /* Use rename only if TO is not a symbolic link and has
+-     only one hard link, and we have permission to write to it.  */
+-  if (! exists
+-      || (!S_ISLNK (s.st_mode)
+-	  && S_ISREG (s.st_mode)
+-	  && (s.st_mode & S_IWUSR)
+-	  && s.st_nlink == 1)
+-      )
++  /* Avoid a full copy and use rename if TO does not exist.  */
++  if (!exists)
+     {
+-      ret = rename (from, to);
+-      if (ret == 0)
+-	{
+-	  if (exists)
+-	    {
+-	      /* Try to preserve the permission bits and ownership of
+-		 TO.  First get the mode right except for the setuid
+-		 bit.  Then change the ownership.  Then fix the setuid
+-		 bit.  We do the chmod before the chown because if the
+-		 chown succeeds, and we are a normal user, we won't be
+-		 able to do the chmod afterward.  We don't bother to
+-		 fix the setuid bit first because that might introduce
+-		 a fleeting security problem, and because the chown
+-		 will clear the setuid bit anyhow.  We only fix the
+-		 setuid bit if the chown succeeds, because we don't
+-		 want to introduce an unexpected setuid file owned by
+-		 the user running objcopy.  */
+-	      chmod (to, s.st_mode & 0777);
+-	      if (chown (to, s.st_uid, s.st_gid) >= 0)
+-		chmod (to, s.st_mode & 07777);
+-	    }
+-	}
+-      else
++      if ((ret = rename (from, to)) != 0)
+ 	{
+ 	  /* We have to clean up here.  */
+ 	  non_fatal (_("unable to rename '%s'; reason: %s"), to, strerror (errno));
+@@ -202,8 +167,8 @@ smart_rename (const char *from, const char *to, int preserve_dates ATTRIBUTE_UNU
+       if (ret != 0)
+ 	non_fatal (_("unable to copy file '%s'; reason: %s"), to, strerror (errno));
+ 
+-      if (preserve_dates)
+-	set_times (to, &s);
++      if (target_stat != NULL)
++	set_times (to, target_stat);
+       unlink (from);
+     }
+ #endif /* _WIN32 && !__CYGWIN32__ */
+-- 
+2.31.1
+
diff --git a/meta/recipes-devtools/binutils/binutils/0002-CVE-2021-20197.patch b/meta/recipes-devtools/binutils/binutils/0002-CVE-2021-20197.patch
new file mode 100644
index 0000000000..3771f571eb
--- /dev/null
+++ b/meta/recipes-devtools/binutils/binutils/0002-CVE-2021-20197.patch
@@ -0,0 +1,170 @@
+From d3edaa91d4cf7202ec14342410194841e2f67f12 Mon Sep 17 00:00:00 2001
+From: Alan Modra <amodra@gmail.com>
+Date: Fri, 26 Feb 2021 11:30:32 +1030
+Subject: [PATCH] Reinstate various pieces backed out from smart_rename changes
+
+In the interests of a stable release various last minute smart_rename
+patches were backed out of the 2.36 branch.  The main reason to
+reinstate some of those backed out changes here is to make necessary
+followup fixes to commit 8e03235147a9 simple cherry-picks from
+mainline.  A secondary reason is that ar -M support isn't fixed for
+pr26945 without this patch.
+
+	PR 26945
+	* ar.c: Don't include libbfd.h.
+	(write_archive): Replace xmalloc+strcpy with xstrdup.
+	* arsup.c (temp_name, real_ofd): New static variables.
+	(ar_open): Use make_tempname and bfd_fdopenw.
+	(ar_save): Adjust to suit ar_open changes.
+	* objcopy.c: Don't include libbfd.h.
+	* rename.c: Rename and reorder variables.
+
+(cherry picked from commit 95b91a043aeaeb546d2fea556d84a2de1e917770)
+
+Upstream-Status: Backport [https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d3edaa91d4cf7202ec14342410194841e2f67f12]
+CVE: CVE-2021-20197
+Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
+---
+ binutils/ar.c      |  4 +---
+ binutils/arsup.c   | 37 +++++++++++++++++++++++++------------
+ binutils/objcopy.c |  1 -
+ binutils/rename.c  |  6 +++---
+ 5 files changed, 42 insertions(+), 19 deletions(-)
+
+diff --git a/binutils/ar.c b/binutils/ar.c
+index 3a91708b51c..44df48c5c67 100644
+--- a/binutils/ar.c
++++ b/binutils/ar.c
+@@ -25,7 +25,6 @@
+ 
+ #include "sysdep.h"
+ #include "bfd.h"
+-#include "libbfd.h"
+ #include "libiberty.h"
+ #include "progress.h"
+ #include "getopt.h"
+@@ -1255,8 +1254,7 @@ write_archive (bfd *iarch)
+   bfd *contents_head = iarch->archive_next;
+   int ofd = -1;
+ 
+-  old_name = (char *) xmalloc (strlen (bfd_get_filename (iarch)) + 1);
+-  strcpy (old_name, bfd_get_filename (iarch));
++  old_name = xstrdup (bfd_get_filename (iarch));
+   new_name = make_tempname (old_name, &ofd);
+ 
+   if (new_name == NULL)
+diff --git a/binutils/arsup.c b/binutils/arsup.c
+index 0a1f63f6456..f7ce8f0bc82 100644
+--- a/binutils/arsup.c
++++ b/binutils/arsup.c
+@@ -42,6 +42,8 @@ extern int deterministic;
+ 
+ static bfd *obfd;
+ static char *real_name;
++static char *temp_name;
++static int real_ofd;
+ static FILE *outfile;
+ 
+ static void
+@@ -149,27 +151,24 @@ maybequit (void)
+ void
+ ar_open (char *name, int t)
+ {
+-  char *tname;
+-  const char *bname = lbasename (name);
+-  real_name = name;
++  real_name = xstrdup (name);
++  temp_name = make_tempname (real_name, &real_ofd);
+ 
+-  /* Prepend tmp- to the beginning, to avoid file-name clashes after
+-     truncation on filesystems with limited namespaces (DOS).  */
+-  if (asprintf (&tname, "%.*stmp-%s", (int) (bname - name), name, bname) == -1)
++  if (temp_name == NULL)
+     {
+-      fprintf (stderr, _("%s: Can't allocate memory for temp name (%s)\n"),
++      fprintf (stderr, _("%s: Can't open temporary file (%s)\n"),
+ 	       program_name, strerror(errno));
+       maybequit ();
+       return;
+     }
+ 
+-  obfd = bfd_openw (tname, NULL);
++  obfd = bfd_fdopenw (temp_name, NULL, real_ofd);
+ 
+   if (!obfd)
+     {
+       fprintf (stderr,
+ 	       _("%s: Can't open output archive %s\n"),
+-	       program_name,  tname);
++	       program_name, temp_name);
+ 
+       maybequit ();
+     }
+@@ -344,16 +343,30 @@ ar_save (void)
+     }
+   else
+     {
+-      char *ofilename = xstrdup (bfd_get_filename (obfd));
++      struct stat target_stat;
+ 
+       if (deterministic > 0)
+         obfd->flags |= BFD_DETERMINISTIC_OUTPUT;
+ 
+       bfd_close (obfd);
+ 
+-      smart_rename (ofilename, real_name, NULL);
++      if (stat (real_name, &target_stat) != 0)
++	{
++	  /* The temp file created in ar_open has mode 0600 as per mkstemp.
++	     Create the real empty output file here so smart_rename will
++	     update the mode according to the process umask.  */
++	  obfd = bfd_openw (real_name, NULL);
++	  if (obfd != NULL)
++	    {
++	      bfd_set_format (obfd, bfd_archive);
++	      bfd_close (obfd);
++	    }
++	}
++
++      smart_rename (temp_name, real_name, NULL);
+       obfd = 0;
+-      free (ofilename);
++      free (temp_name);
++      free (real_name);
+     }
+ }
+ 
+diff --git a/binutils/objcopy.c b/binutils/objcopy.c
+index 07a872b5a80..73aa8bc2514 100644
+--- a/binutils/objcopy.c
++++ b/binutils/objcopy.c
+@@ -20,7 +20,6 @@
+ \f
+ #include "sysdep.h"
+ #include "bfd.h"
+-#include "libbfd.h"
+ #include "progress.h"
+ #include "getopt.h"
+ #include "libiberty.h"
+diff --git a/binutils/rename.c b/binutils/rename.c
+index f471b45fd3f..2ff092ee22b 100644
+--- a/binutils/rename.c
++++ b/binutils/rename.c
+@@ -130,11 +130,11 @@ int
+ smart_rename (const char *from, const char *to,
+ 	      struct stat *target_stat ATTRIBUTE_UNUSED)
+ {
+-  bfd_boolean exists;
+-  struct stat s;
+   int ret = 0;
++  struct stat to_stat;
++  bfd_boolean exists;
+ 
+-  exists = lstat (to, &s) == 0;
++  exists = lstat (to, &to_stat) == 0;
+ 
+ #if defined (_WIN32) && !defined (__CYGWIN32__)
+   /* Win32, unlike unix, will not erase `to' in `rename(from, to)' but
+-- 
+2.31.1
+
diff --git a/meta/recipes-devtools/binutils/binutils/0003-CVE-2021-20197.patch b/meta/recipes-devtools/binutils/binutils/0003-CVE-2021-20197.patch
new file mode 100644
index 0000000000..082b28b29c
--- /dev/null
+++ b/meta/recipes-devtools/binutils/binutils/0003-CVE-2021-20197.patch
@@ -0,0 +1,171 @@
+From 8b69e61d4be276bb862698aaafddc3e779d23c8f Mon Sep 17 00:00:00 2001
+From: Alan Modra <amodra@gmail.com>
+Date: Tue, 23 Feb 2021 09:37:39 +1030
+Subject: [PATCH] PR27456, lstat in rename.c on MinGW
+
+	PR 27456
+	* rename.c: Tidy throughout.
+	(smart_rename): Always copy.  Remove windows specific code.
+
+(cherry picked from commit cca8873dd5a6015d5557ea44bc1ea9c252435a29)
+
+Upstream-Status: Backport [https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=8b69e61d4be276bb862698aaafddc3e779d23c8f]
+CVE: CVE-2021-20197
+Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
+---
+ binutils/rename.c  | 111 ++++++++++++++-------------------------------
+ 2 files changed, 40 insertions(+), 76 deletions(-)
+
+diff --git a/binutils/rename.c b/binutils/rename.c
+index 2ff092ee22b..72a9323d72c 100644
+--- a/binutils/rename.c
++++ b/binutils/rename.c
+@@ -24,14 +24,9 @@
+ 
+ #ifdef HAVE_GOOD_UTIME_H
+ #include <utime.h>
+-#else /* ! HAVE_GOOD_UTIME_H */
+-#ifdef HAVE_UTIMES
++#elif defined HAVE_UTIMES
+ #include <sys/time.h>
+-#endif /* HAVE_UTIMES */
+-#endif /* ! HAVE_GOOD_UTIME_H */
+-
+-#if ! defined (_WIN32) || defined (__CYGWIN32__)
+-static int simple_copy (const char *, const char *);
++#endif
+ 
+ /* The number of bytes to copy at once.  */
+ #define COPY_BUF 8192
+@@ -82,7 +77,6 @@ simple_copy (const char *from, const char *to)
+     }
+   return 0;
+ }
+-#endif /* __CYGWIN32__ or not _WIN32 */
+ 
+ /* Set the times of the file DESTINATION to be the same as those in
+    STATBUF.  */
+@@ -91,87 +85,52 @@ void
+ set_times (const char *destination, const struct stat *statbuf)
+ {
+   int result;
+-
+-  {
+ #ifdef HAVE_GOOD_UTIME_H
+-    struct utimbuf tb;
+-
+-    tb.actime = statbuf->st_atime;
+-    tb.modtime = statbuf->st_mtime;
+-    result = utime (destination, &tb);
+-#else /* ! HAVE_GOOD_UTIME_H */
+-#ifndef HAVE_UTIMES
+-    long tb[2];
+-
+-    tb[0] = statbuf->st_atime;
+-    tb[1] = statbuf->st_mtime;
+-    result = utime (destination, tb);
+-#else /* HAVE_UTIMES */
+-    struct timeval tv[2];
+-
+-    tv[0].tv_sec = statbuf->st_atime;
+-    tv[0].tv_usec = 0;
+-    tv[1].tv_sec = statbuf->st_mtime;
+-    tv[1].tv_usec = 0;
+-    result = utimes (destination, tv);
+-#endif /* HAVE_UTIMES */
+-#endif /* ! HAVE_GOOD_UTIME_H */
+-  }
++  struct utimbuf tb;
++
++  tb.actime = statbuf->st_atime;
++  tb.modtime = statbuf->st_mtime;
++  result = utime (destination, &tb);
++#elif defined HAVE_UTIMES
++  struct timeval tv[2];
++
++  tv[0].tv_sec = statbuf->st_atime;
++  tv[0].tv_usec = 0;
++  tv[1].tv_sec = statbuf->st_mtime;
++  tv[1].tv_usec = 0;
++  result = utimes (destination, tv);
++#else
++  long tb[2];
++
++  tb[0] = statbuf->st_atime;
++  tb[1] = statbuf->st_mtime;
++  result = utime (destination, tb);
++#endif
+ 
+   if (result != 0)
+     non_fatal (_("%s: cannot set time: %s"), destination, strerror (errno));
+ }
+ 
+-/* Rename FROM to TO, copying if TO exists.  TARGET_STAT has the file status
+-   that, if non-NULL, is used to fix up timestamps after rename.  Return 0 if
+-   ok, -1 if error.  */
++/* Copy FROM to TO.  TARGET_STAT has the file status that, if non-NULL,
++   is used to fix up timestamps.  Return 0 if ok, -1 if error.
++   At one time this function renamed files, but file permissions are
++   tricky to update given the number of different schemes used by
++   various systems.  So now we just copy.  */
+ 
+ int
+ smart_rename (const char *from, const char *to,
+-	      struct stat *target_stat ATTRIBUTE_UNUSED)
++	      struct stat *target_stat)
+ {
+-  int ret = 0;
+-  struct stat to_stat;
+-  bfd_boolean exists;
+-
+-  exists = lstat (to, &to_stat) == 0;
+-
+-#if defined (_WIN32) && !defined (__CYGWIN32__)
+-  /* Win32, unlike unix, will not erase `to' in `rename(from, to)' but
+-     fail instead.  Also, chown is not present.  */
+-
+-  if (exists)
+-    remove (to);
++  int ret;
+ 
+-  ret = rename (from, to);
++  ret = simple_copy (from, to);
+   if (ret != 0)
+-    {
+-      /* We have to clean up here.  */
+-      non_fatal (_("unable to rename '%s'; reason: %s"), to, strerror (errno));
+-      unlink (from);
+-    }
+-#else
+-  /* Avoid a full copy and use rename if TO does not exist.  */
+-  if (!exists)
+-    {
+-      if ((ret = rename (from, to)) != 0)
+-	{
+-	  /* We have to clean up here.  */
+-	  non_fatal (_("unable to rename '%s'; reason: %s"), to, strerror (errno));
+-	  unlink (from);
+-	}
+-    }
+-  else
+-    {
+-      ret = simple_copy (from, to);
+-      if (ret != 0)
+-	non_fatal (_("unable to copy file '%s'; reason: %s"), to, strerror (errno));
++    non_fatal (_("unable to copy file '%s'; reason: %s"),
++	       to, strerror (errno));
+ 
+-      if (target_stat != NULL)
+-	set_times (to, target_stat);
+-      unlink (from);
+-    }
+-#endif /* _WIN32 && !__CYGWIN32__ */
++  if (target_stat != NULL)
++    set_times (to, target_stat);
++  unlink (from);
+ 
+   return ret;
+ }
+-- 
+2.31.1
+
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [OE-core] [hardknott][PATCH] Binutils: Fix CVE-2021-20197
  2021-06-29 13:25         ` Randy MacLeod
  2021-07-02 11:22           ` [hardknott][PATCH v3] binutils: " Vinay Kumar
@ 2021-07-02 11:39           ` Vinay Kumar
  1 sibling, 0 replies; 18+ messages in thread
From: Vinay Kumar @ 2021-07-02 11:39 UTC (permalink / raw)
  To: Randy MacLeod
  Cc: Mittal, Anuj, vinay.kumar, openembedded-core, rwmacleod, umesh.kalappa0

Hi Randy, Anuj,

Sent for review with 3 separate patches.
https://lists.openembedded.org/g/openembedded-core/message/153462

Regards,
Vinay

On Tue, Jun 29, 2021 at 6:56 PM Randy MacLeod
<randy.macleod@windriver.com> wrote:
>
> On 2021-05-07 10:48 a.m., Vinay Kumar wrote:
> > Hi Anuj,
> >
> >>> Right now, the patch header is wrong as it's not the upstream commit that it is claiming to be.
> >
> > Created 3 patches.
> > For the patch of commit "d3edaa91d4cf7202ec14342410194841e2f67f12"
> > planning to use
> > below patch header from the commit it is cherry picked.
> > Also, mentioning that in "Upstream-Status" section as below,
> > ====================================
> >>From 95b91a043aeaeb546d2fea556d84a2de1e917770 Mon Sep 17 00:00:00 2001
> > From: Alan Modra <amodra@gmail.com>
> > Date: Mon, 1 Feb 2021 02:04:41 +1030
> > Subject: [PATCH] pr27270 and pr27284, ar segfaults and wrong file mode
> >
> >          PR 27270
> >          PR 27284
> >          PR 26945
> >          * ar.c: Don't include libbfd.h.
> >          (write_archive): Replace xmalloc+strcpy with xstrdup.  Use
> >          bfd_stat rather than fstat on iostream.  Move stat and fd tests
> >          outside of _WIN32 ifdef.  Delete skip_stat variable.
> >          * arsup.c (temp_name, real_ofd): New static variables.
> >          (ar_open): Use make_tempname and bfd_fdopenw.
> >          (ar_save): Adjust to suit ar_open changes.  Move stat output
> >          of _WIN32 ifdef.
> >          * objcopy.c: Don't include libbfd.h.
> >          (copy_file): Use bfd_stat.
> >
> > Upstream-Status:
> > Backport [https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=95b91a043aeaeb546d2fea556d84a2de1e917770]
> > Used commit "d3edaa91d4cf7202ec14342410194841e2f67f12" cherry picked from commit
> > "95b91a043aeaeb546d2fea556d84a2de1e917770"
> > CVE: CVE-2021-20197
> > Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
> > ====================================
> >
> > Other 2 commits ids patch headers are identical to the cherry picked
> > from commits.
> >
> > Regards,
> > Vinay
> >
>
> Seems that this patch was not merged to hardknott.
> I didn't see the version split into 3 patch on the list.
>
> Vinay, can you please re-submit if you agree
> that splitting it up into 3 patches makes sense.
>
> --
> # Randy MacLeod
> # Wind River Linux

^ permalink raw reply	[flat|nested] 18+ messages in thread

* [hardknott][PATCH v4] binutils: Fix CVE-2021-20197
  2021-07-02 11:22           ` [hardknott][PATCH v3] binutils: " Vinay Kumar
@ 2021-07-13 13:46             ` Vinay Kumar
       [not found]             ` <16915DBBAEEB81DE.15464@lists.openembedded.org>
  1 sibling, 0 replies; 18+ messages in thread
From: Vinay Kumar @ 2021-07-13 13:46 UTC (permalink / raw)
  To: anuj.mittal
  Cc: richard.purdie, openembedded-core, rwmacleod, umesh.kalappa0,
	vinay.kumar, Vinay Kumar

Source: git://sourceware.org/git/binutils-gdb.git
Tracking -- https://sourceware.org/bugzilla/show_bug.cgi?id=26945

Backported upstream commit d3edaa91d4cf7202ec14342410194841e2f67f12 to
binutils-2.36 source along with commit id dependencies
(8e03235147a9e774d3ba084e93c2da1aa94d1cec and 8b69e61d4be276bb862698aaafddc3e779d23c8f).

Upstream-Status: Backport [https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d3edaa91d4cf7202ec14342410194841e2f67f12]

Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
---
 .../binutils/binutils-2.36.inc                |   1 +
 .../binutils/binutils/CVE-2021-20197.patch    | 388 ++++++++++++++++++
 2 files changed, 389 insertions(+)
 create mode 100644 meta/recipes-devtools/binutils/binutils/CVE-2021-20197.patch

diff --git a/meta/recipes-devtools/binutils/binutils-2.36.inc b/meta/recipes-devtools/binutils/binutils-2.36.inc
index 2968291889..6d0293b4bc 100644
--- a/meta/recipes-devtools/binutils/binutils-2.36.inc
+++ b/meta/recipes-devtools/binutils/binutils-2.36.inc
@@ -41,5 +41,6 @@ SRC_URI = "\
      file://0014-Fix-rpath-in-libtool-when-sysroot-is-enabled.patch \
      file://0015-sync-with-OE-libtool-changes.patch \
      file://0016-Check-for-clang-before-checking-gcc-version.patch \
+     file://CVE-2021-20197.patch \
 "
 S  = "${WORKDIR}/git"
diff --git a/meta/recipes-devtools/binutils/binutils/CVE-2021-20197.patch b/meta/recipes-devtools/binutils/binutils/CVE-2021-20197.patch
new file mode 100644
index 0000000000..d6117b65a4
--- /dev/null
+++ b/meta/recipes-devtools/binutils/binutils/CVE-2021-20197.patch
@@ -0,0 +1,388 @@
+From d3edaa91d4cf7202ec14342410194841e2f67f12 Mon Sep 17 00:00:00 2001
+From: Alan Modra <amodra@gmail.com>
+Date: Fri, 26 Feb 2021 11:30:32 +1030
+Subject: [PATCH] Reinstate various pieces backed out from smart_rename changes
+
+In the interests of a stable release various last minute smart_rename
+patches were backed out of the 2.36 branch.  The main reason to
+reinstate some of those backed out changes here is to make necessary
+followup fixes to commit 8e03235147a9 simple cherry-picks from
+mainline.  A secondary reason is that ar -M support isn't fixed for
+pr26945 without this patch.
+
+        PR 26945
+        * ar.c: Don't include libbfd.h.
+        (write_archive): Replace xmalloc+strcpy with xstrdup.
+        * arsup.c (temp_name, real_ofd): New static variables.
+        (ar_open): Use make_tempname and bfd_fdopenw.
+        (ar_save): Adjust to suit ar_open changes.
+        * objcopy.c: Don't include libbfd.h.
+        * rename.c: Rename and reorder variables.
+
+(cherry picked from commit 95b91a043aeaeb546d2fea556d84a2de1e917770)
+
+Upstream-Status: Backport [https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d3edaa91d4cf7202ec14342410194841e2f67f12]
+CVE: CVE-2021-20197
+Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
+---
+ binutils/ar.c      |   6 +-
+ binutils/arsup.c   |  37 ++++++++----
+ binutils/bucomm.h  |   3 +-
+ binutils/objcopy.c |   9 +--
+ binutils/rename.c  | 148 +++++++++++----------------------------------
+ 5 files changed, 67 insertions(+), 136 deletions(-)
+
+diff --git a/binutils/ar.c b/binutils/ar.c
+index 45a34e3a6cf..44df48c5c67 100644
+--- a/binutils/ar.c
++++ b/binutils/ar.c
+@@ -25,7 +25,6 @@
+ 
+ #include "sysdep.h"
+ #include "bfd.h"
+-#include "libbfd.h"
+ #include "libiberty.h"
+ #include "progress.h"
+ #include "getopt.h"
+@@ -1255,8 +1254,7 @@ write_archive (bfd *iarch)
+   bfd *contents_head = iarch->archive_next;
+   int ofd = -1;
+ 
+-  old_name = (char *) xmalloc (strlen (bfd_get_filename (iarch)) + 1);
+-  strcpy (old_name, bfd_get_filename (iarch));
++  old_name = xstrdup (bfd_get_filename (iarch));
+   new_name = make_tempname (old_name, &ofd);
+ 
+   if (new_name == NULL)
+@@ -1308,7 +1306,7 @@ write_archive (bfd *iarch)
+   /* We don't care if this fails; we might be creating the archive.  */
+   bfd_close (iarch);
+ 
+-  if (smart_rename (new_name, old_name, 0) != 0)
++  if (smart_rename (new_name, old_name, NULL) != 0)
+     xexit (1);
+   free (old_name);
+   free (new_name);
+diff --git a/binutils/arsup.c b/binutils/arsup.c
+index 5403a0c5d74..f7ce8f0bc82 100644
+--- a/binutils/arsup.c
++++ b/binutils/arsup.c
+@@ -42,6 +42,8 @@ extern int deterministic;
+ 
+ static bfd *obfd;
+ static char *real_name;
++static char *temp_name;
++static int real_ofd;
+ static FILE *outfile;
+ 
+ static void
+@@ -149,27 +151,24 @@ maybequit (void)
+ void
+ ar_open (char *name, int t)
+ {
+-  char *tname;
+-  const char *bname = lbasename (name);
+-  real_name = name;
++  real_name = xstrdup (name);
++  temp_name = make_tempname (real_name, &real_ofd);
+ 
+-  /* Prepend tmp- to the beginning, to avoid file-name clashes after
+-     truncation on filesystems with limited namespaces (DOS).  */
+-  if (asprintf (&tname, "%.*stmp-%s", (int) (bname - name), name, bname) == -1)
++  if (temp_name == NULL)
+     {
+-      fprintf (stderr, _("%s: Can't allocate memory for temp name (%s)\n"),
++      fprintf (stderr, _("%s: Can't open temporary file (%s)\n"),
+ 	       program_name, strerror(errno));
+       maybequit ();
+       return;
+     }
+ 
+-  obfd = bfd_openw (tname, NULL);
++  obfd = bfd_fdopenw (temp_name, NULL, real_ofd);
+ 
+   if (!obfd)
+     {
+       fprintf (stderr,
+ 	       _("%s: Can't open output archive %s\n"),
+-	       program_name,  tname);
++	       program_name, temp_name);
+ 
+       maybequit ();
+     }
+@@ -344,16 +343,30 @@ ar_save (void)
+     }
+   else
+     {
+-      char *ofilename = xstrdup (bfd_get_filename (obfd));
++      struct stat target_stat;
+ 
+       if (deterministic > 0)
+         obfd->flags |= BFD_DETERMINISTIC_OUTPUT;
+ 
+       bfd_close (obfd);
+ 
+-      smart_rename (ofilename, real_name, 0);
++      if (stat (real_name, &target_stat) != 0)
++	{
++	  /* The temp file created in ar_open has mode 0600 as per mkstemp.
++	     Create the real empty output file here so smart_rename will
++	     update the mode according to the process umask.  */
++	  obfd = bfd_openw (real_name, NULL);
++	  if (obfd != NULL)
++	    {
++	      bfd_set_format (obfd, bfd_archive);
++	      bfd_close (obfd);
++	    }
++	}
++
++      smart_rename (temp_name, real_name, NULL);
+       obfd = 0;
+-      free (ofilename);
++      free (temp_name);
++      free (real_name);
+     }
+ }
+ 
+diff --git a/binutils/bucomm.h b/binutils/bucomm.h
+index 91f6a5b228f..aa7e33d8cd1 100644
+--- a/binutils/bucomm.h
++++ b/binutils/bucomm.h
+@@ -71,7 +71,8 @@ extern void print_version (const char *);
+ /* In rename.c.  */
+ extern void set_times (const char *, const struct stat *);
+ 
+-extern int smart_rename (const char *, const char *, int);
++extern int smart_rename (const char *, const char *, struct stat *);
++
+ 
+ /* In libiberty.  */
+ void *xmalloc (size_t);
+diff --git a/binutils/objcopy.c b/binutils/objcopy.c
+index eab3b6db585..73aa8bc2514 100644
+--- a/binutils/objcopy.c
++++ b/binutils/objcopy.c
+@@ -20,7 +20,6 @@
+ \f
+ #include "sysdep.h"
+ #include "bfd.h"
+-#include "libbfd.h"
+ #include "progress.h"
+ #include "getopt.h"
+ #include "libiberty.h"
+@@ -4861,12 +4860,10 @@ strip_main (int argc, char *argv[])
+ 		 output_target, NULL);
+       if (status == 0)
+ 	{
+-	  if (preserve_dates)
+-	    set_times (tmpname, &statbuf);
+ 	  if (output_file != tmpname)
+ 	    status = (smart_rename (tmpname,
+ 				    output_file ? output_file : argv[i],
+-				    preserve_dates) != 0);
++				    preserve_dates ? &statbuf : NULL) != 0);
+ 	  if (status == 0)
+ 	    status = hold_status;
+ 	}
+@@ -5931,11 +5928,9 @@ copy_main (int argc, char *argv[])
+ 	     output_target, input_arch);
+   if (status == 0)
+     {
+-      if (preserve_dates)
+-	set_times (tmpname, &statbuf);
+       if (tmpname != output_filename)
+ 	status = (smart_rename (tmpname, input_filename,
+-				preserve_dates) != 0);
++				preserve_dates ? &statbuf : NULL) != 0);
+     }
+   else
+     unlink_if_ordinary (tmpname);
+diff --git a/binutils/rename.c b/binutils/rename.c
+index 65ad5bf52c4..72a9323d72c 100644
+--- a/binutils/rename.c
++++ b/binutils/rename.c
+@@ -24,14 +24,9 @@
+ 
+ #ifdef HAVE_GOOD_UTIME_H
+ #include <utime.h>
+-#else /* ! HAVE_GOOD_UTIME_H */
+-#ifdef HAVE_UTIMES
++#elif defined HAVE_UTIMES
+ #include <sys/time.h>
+-#endif /* HAVE_UTIMES */
+-#endif /* ! HAVE_GOOD_UTIME_H */
+-
+-#if ! defined (_WIN32) || defined (__CYGWIN32__)
+-static int simple_copy (const char *, const char *);
++#endif
+ 
+ /* The number of bytes to copy at once.  */
+ #define COPY_BUF 8192
+@@ -82,7 +77,6 @@ simple_copy (const char *from, const char *to)
+     }
+   return 0;
+ }
+-#endif /* __CYGWIN32__ or not _WIN32 */
+ 
+ /* Set the times of the file DESTINATION to be the same as those in
+    STATBUF.  */
+@@ -91,122 +85,52 @@ void
+ set_times (const char *destination, const struct stat *statbuf)
+ {
+   int result;
+-
+-  {
+ #ifdef HAVE_GOOD_UTIME_H
+-    struct utimbuf tb;
+-
+-    tb.actime = statbuf->st_atime;
+-    tb.modtime = statbuf->st_mtime;
+-    result = utime (destination, &tb);
+-#else /* ! HAVE_GOOD_UTIME_H */
+-#ifndef HAVE_UTIMES
+-    long tb[2];
+-
+-    tb[0] = statbuf->st_atime;
+-    tb[1] = statbuf->st_mtime;
+-    result = utime (destination, tb);
+-#else /* HAVE_UTIMES */
+-    struct timeval tv[2];
+-
+-    tv[0].tv_sec = statbuf->st_atime;
+-    tv[0].tv_usec = 0;
+-    tv[1].tv_sec = statbuf->st_mtime;
+-    tv[1].tv_usec = 0;
+-    result = utimes (destination, tv);
+-#endif /* HAVE_UTIMES */
+-#endif /* ! HAVE_GOOD_UTIME_H */
+-  }
++  struct utimbuf tb;
++
++  tb.actime = statbuf->st_atime;
++  tb.modtime = statbuf->st_mtime;
++  result = utime (destination, &tb);
++#elif defined HAVE_UTIMES
++  struct timeval tv[2];
++
++  tv[0].tv_sec = statbuf->st_atime;
++  tv[0].tv_usec = 0;
++  tv[1].tv_sec = statbuf->st_mtime;
++  tv[1].tv_usec = 0;
++  result = utimes (destination, tv);
++#else
++  long tb[2];
++
++  tb[0] = statbuf->st_atime;
++  tb[1] = statbuf->st_mtime;
++  result = utime (destination, tb);
++#endif
+ 
+   if (result != 0)
+     non_fatal (_("%s: cannot set time: %s"), destination, strerror (errno));
+ }
+ 
+-#ifndef S_ISLNK
+-#ifdef S_IFLNK
+-#define S_ISLNK(m) (((m) & S_IFMT) == S_IFLNK)
+-#else
+-#define S_ISLNK(m) 0
+-#define lstat stat
+-#endif
+-#endif
+-
+-/* Rename FROM to TO, copying if TO is a link.
+-   Return 0 if ok, -1 if error.  */
++/* Copy FROM to TO.  TARGET_STAT has the file status that, if non-NULL,
++   is used to fix up timestamps.  Return 0 if ok, -1 if error.
++   At one time this function renamed files, but file permissions are
++   tricky to update given the number of different schemes used by
++   various systems.  So now we just copy.  */
+ 
+ int
+-smart_rename (const char *from, const char *to, int preserve_dates ATTRIBUTE_UNUSED)
++smart_rename (const char *from, const char *to,
++	      struct stat *target_stat)
+ {
+-  bfd_boolean exists;
+-  struct stat s;
+-  int ret = 0;
+-
+-  exists = lstat (to, &s) == 0;
+-
+-#if defined (_WIN32) && !defined (__CYGWIN32__)
+-  /* Win32, unlike unix, will not erase `to' in `rename(from, to)' but
+-     fail instead.  Also, chown is not present.  */
++  int ret;
+ 
+-  if (exists)
+-    remove (to);
+-
+-  ret = rename (from, to);
++  ret = simple_copy (from, to);
+   if (ret != 0)
+-    {
+-      /* We have to clean up here.  */
+-      non_fatal (_("unable to rename '%s'; reason: %s"), to, strerror (errno));
+-      unlink (from);
+-    }
+-#else
+-  /* Use rename only if TO is not a symbolic link and has
+-     only one hard link, and we have permission to write to it.  */
+-  if (! exists
+-      || (!S_ISLNK (s.st_mode)
+-	  && S_ISREG (s.st_mode)
+-	  && (s.st_mode & S_IWUSR)
+-	  && s.st_nlink == 1)
+-      )
+-    {
+-      ret = rename (from, to);
+-      if (ret == 0)
+-	{
+-	  if (exists)
+-	    {
+-	      /* Try to preserve the permission bits and ownership of
+-		 TO.  First get the mode right except for the setuid
+-		 bit.  Then change the ownership.  Then fix the setuid
+-		 bit.  We do the chmod before the chown because if the
+-		 chown succeeds, and we are a normal user, we won't be
+-		 able to do the chmod afterward.  We don't bother to
+-		 fix the setuid bit first because that might introduce
+-		 a fleeting security problem, and because the chown
+-		 will clear the setuid bit anyhow.  We only fix the
+-		 setuid bit if the chown succeeds, because we don't
+-		 want to introduce an unexpected setuid file owned by
+-		 the user running objcopy.  */
+-	      chmod (to, s.st_mode & 0777);
+-	      if (chown (to, s.st_uid, s.st_gid) >= 0)
+-		chmod (to, s.st_mode & 07777);
+-	    }
+-	}
+-      else
+-	{
+-	  /* We have to clean up here.  */
+-	  non_fatal (_("unable to rename '%s'; reason: %s"), to, strerror (errno));
+-	  unlink (from);
+-	}
+-    }
+-  else
+-    {
+-      ret = simple_copy (from, to);
+-      if (ret != 0)
+-	non_fatal (_("unable to copy file '%s'; reason: %s"), to, strerror (errno));
++    non_fatal (_("unable to copy file '%s'; reason: %s"),
++	       to, strerror (errno));
+ 
+-      if (preserve_dates)
+-	set_times (to, &s);
+-      unlink (from);
+-    }
+-#endif /* _WIN32 && !__CYGWIN32__ */
++  if (target_stat != NULL)
++    set_times (to, target_stat);
++  unlink (from);
+ 
+   return ret;
+ }
+-- 
+2.17.1
+
-- 
2.31.1


^ permalink raw reply related	[flat|nested] 18+ messages in thread

* Re: [OE-core] [hardknott][PATCH v4] binutils: Fix CVE-2021-20197
       [not found]             ` <16915DBBAEEB81DE.15464@lists.openembedded.org>
@ 2021-07-13 13:49               ` Vinay Kumar
       [not found]               ` <16915DE420F8F12E.31724@lists.openembedded.org>
  1 sibling, 0 replies; 18+ messages in thread
From: Vinay Kumar @ 2021-07-13 13:49 UTC (permalink / raw)
  To: Mittal, Anuj
  Cc: Richard Purdie, Patches and discussions about the oe-core layer,
	Randy MacLeod, umesh kalappa0, vinay.kumar

Hi Anuj,

This patch is a cherry-pick of
"f628b052f6c4a957472f2520adfb4b816ef83d95" from master branch.

Regards,
Vinay

On Tue, Jul 13, 2021 at 7:16 PM Vinay Kumar via lists.openembedded.org
<vinay.m.engg=gmail.com@lists.openembedded.org> wrote:
>
> Source: git://sourceware.org/git/binutils-gdb.git
> Tracking -- https://sourceware.org/bugzilla/show_bug.cgi?id=26945
>
> Backported upstream commit d3edaa91d4cf7202ec14342410194841e2f67f12 to
> binutils-2.36 source along with commit id dependencies
> (8e03235147a9e774d3ba084e93c2da1aa94d1cec and 8b69e61d4be276bb862698aaafddc3e779d23c8f).
>
> Upstream-Status: Backport [https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d3edaa91d4cf7202ec14342410194841e2f67f12]
>
> Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
> ---
>  .../binutils/binutils-2.36.inc                |   1 +
>  .../binutils/binutils/CVE-2021-20197.patch    | 388 ++++++++++++++++++
>  2 files changed, 389 insertions(+)
>  create mode 100644 meta/recipes-devtools/binutils/binutils/CVE-2021-20197.patch
>
> diff --git a/meta/recipes-devtools/binutils/binutils-2.36.inc b/meta/recipes-devtools/binutils/binutils-2.36.inc
> index 2968291889..6d0293b4bc 100644
> --- a/meta/recipes-devtools/binutils/binutils-2.36.inc
> +++ b/meta/recipes-devtools/binutils/binutils-2.36.inc
> @@ -41,5 +41,6 @@ SRC_URI = "\
>       file://0014-Fix-rpath-in-libtool-when-sysroot-is-enabled.patch \
>       file://0015-sync-with-OE-libtool-changes.patch \
>       file://0016-Check-for-clang-before-checking-gcc-version.patch \
> +     file://CVE-2021-20197.patch \
>  "
>  S  = "${WORKDIR}/git"
> diff --git a/meta/recipes-devtools/binutils/binutils/CVE-2021-20197.patch b/meta/recipes-devtools/binutils/binutils/CVE-2021-20197.patch
> new file mode 100644
> index 0000000000..d6117b65a4
> --- /dev/null
> +++ b/meta/recipes-devtools/binutils/binutils/CVE-2021-20197.patch
> @@ -0,0 +1,388 @@
> +From d3edaa91d4cf7202ec14342410194841e2f67f12 Mon Sep 17 00:00:00 2001
> +From: Alan Modra <amodra@gmail.com>
> +Date: Fri, 26 Feb 2021 11:30:32 +1030
> +Subject: [PATCH] Reinstate various pieces backed out from smart_rename changes
> +
> +In the interests of a stable release various last minute smart_rename
> +patches were backed out of the 2.36 branch.  The main reason to
> +reinstate some of those backed out changes here is to make necessary
> +followup fixes to commit 8e03235147a9 simple cherry-picks from
> +mainline.  A secondary reason is that ar -M support isn't fixed for
> +pr26945 without this patch.
> +
> +        PR 26945
> +        * ar.c: Don't include libbfd.h.
> +        (write_archive): Replace xmalloc+strcpy with xstrdup.
> +        * arsup.c (temp_name, real_ofd): New static variables.
> +        (ar_open): Use make_tempname and bfd_fdopenw.
> +        (ar_save): Adjust to suit ar_open changes.
> +        * objcopy.c: Don't include libbfd.h.
> +        * rename.c: Rename and reorder variables.
> +
> +(cherry picked from commit 95b91a043aeaeb546d2fea556d84a2de1e917770)
> +
> +Upstream-Status: Backport [https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d3edaa91d4cf7202ec14342410194841e2f67f12]
> +CVE: CVE-2021-20197
> +Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
> +---
> + binutils/ar.c      |   6 +-
> + binutils/arsup.c   |  37 ++++++++----
> + binutils/bucomm.h  |   3 +-
> + binutils/objcopy.c |   9 +--
> + binutils/rename.c  | 148 +++++++++++----------------------------------
> + 5 files changed, 67 insertions(+), 136 deletions(-)
> +
> +diff --git a/binutils/ar.c b/binutils/ar.c
> +index 45a34e3a6cf..44df48c5c67 100644
> +--- a/binutils/ar.c
> ++++ b/binutils/ar.c
> +@@ -25,7 +25,6 @@
> +
> + #include "sysdep.h"
> + #include "bfd.h"
> +-#include "libbfd.h"
> + #include "libiberty.h"
> + #include "progress.h"
> + #include "getopt.h"
> +@@ -1255,8 +1254,7 @@ write_archive (bfd *iarch)
> +   bfd *contents_head = iarch->archive_next;
> +   int ofd = -1;
> +
> +-  old_name = (char *) xmalloc (strlen (bfd_get_filename (iarch)) + 1);
> +-  strcpy (old_name, bfd_get_filename (iarch));
> ++  old_name = xstrdup (bfd_get_filename (iarch));
> +   new_name = make_tempname (old_name, &ofd);
> +
> +   if (new_name == NULL)
> +@@ -1308,7 +1306,7 @@ write_archive (bfd *iarch)
> +   /* We don't care if this fails; we might be creating the archive.  */
> +   bfd_close (iarch);
> +
> +-  if (smart_rename (new_name, old_name, 0) != 0)
> ++  if (smart_rename (new_name, old_name, NULL) != 0)
> +     xexit (1);
> +   free (old_name);
> +   free (new_name);
> +diff --git a/binutils/arsup.c b/binutils/arsup.c
> +index 5403a0c5d74..f7ce8f0bc82 100644
> +--- a/binutils/arsup.c
> ++++ b/binutils/arsup.c
> +@@ -42,6 +42,8 @@ extern int deterministic;
> +
> + static bfd *obfd;
> + static char *real_name;
> ++static char *temp_name;
> ++static int real_ofd;
> + static FILE *outfile;
> +
> + static void
> +@@ -149,27 +151,24 @@ maybequit (void)
> + void
> + ar_open (char *name, int t)
> + {
> +-  char *tname;
> +-  const char *bname = lbasename (name);
> +-  real_name = name;
> ++  real_name = xstrdup (name);
> ++  temp_name = make_tempname (real_name, &real_ofd);
> +
> +-  /* Prepend tmp- to the beginning, to avoid file-name clashes after
> +-     truncation on filesystems with limited namespaces (DOS).  */
> +-  if (asprintf (&tname, "%.*stmp-%s", (int) (bname - name), name, bname) == -1)
> ++  if (temp_name == NULL)
> +     {
> +-      fprintf (stderr, _("%s: Can't allocate memory for temp name (%s)\n"),
> ++      fprintf (stderr, _("%s: Can't open temporary file (%s)\n"),
> +              program_name, strerror(errno));
> +       maybequit ();
> +       return;
> +     }
> +
> +-  obfd = bfd_openw (tname, NULL);
> ++  obfd = bfd_fdopenw (temp_name, NULL, real_ofd);
> +
> +   if (!obfd)
> +     {
> +       fprintf (stderr,
> +              _("%s: Can't open output archive %s\n"),
> +-             program_name,  tname);
> ++             program_name, temp_name);
> +
> +       maybequit ();
> +     }
> +@@ -344,16 +343,30 @@ ar_save (void)
> +     }
> +   else
> +     {
> +-      char *ofilename = xstrdup (bfd_get_filename (obfd));
> ++      struct stat target_stat;
> +
> +       if (deterministic > 0)
> +         obfd->flags |= BFD_DETERMINISTIC_OUTPUT;
> +
> +       bfd_close (obfd);
> +
> +-      smart_rename (ofilename, real_name, 0);
> ++      if (stat (real_name, &target_stat) != 0)
> ++      {
> ++        /* The temp file created in ar_open has mode 0600 as per mkstemp.
> ++           Create the real empty output file here so smart_rename will
> ++           update the mode according to the process umask.  */
> ++        obfd = bfd_openw (real_name, NULL);
> ++        if (obfd != NULL)
> ++          {
> ++            bfd_set_format (obfd, bfd_archive);
> ++            bfd_close (obfd);
> ++          }
> ++      }
> ++
> ++      smart_rename (temp_name, real_name, NULL);
> +       obfd = 0;
> +-      free (ofilename);
> ++      free (temp_name);
> ++      free (real_name);
> +     }
> + }
> +
> +diff --git a/binutils/bucomm.h b/binutils/bucomm.h
> +index 91f6a5b228f..aa7e33d8cd1 100644
> +--- a/binutils/bucomm.h
> ++++ b/binutils/bucomm.h
> +@@ -71,7 +71,8 @@ extern void print_version (const char *);
> + /* In rename.c.  */
> + extern void set_times (const char *, const struct stat *);
> +
> +-extern int smart_rename (const char *, const char *, int);
> ++extern int smart_rename (const char *, const char *, struct stat *);
> ++
> +
> + /* In libiberty.  */
> + void *xmalloc (size_t);
> +diff --git a/binutils/objcopy.c b/binutils/objcopy.c
> +index eab3b6db585..73aa8bc2514 100644
> +--- a/binutils/objcopy.c
> ++++ b/binutils/objcopy.c
> +@@ -20,7 +20,6 @@
> +
> + #include "sysdep.h"
> + #include "bfd.h"
> +-#include "libbfd.h"
> + #include "progress.h"
> + #include "getopt.h"
> + #include "libiberty.h"
> +@@ -4861,12 +4860,10 @@ strip_main (int argc, char *argv[])
> +                output_target, NULL);
> +       if (status == 0)
> +       {
> +-        if (preserve_dates)
> +-          set_times (tmpname, &statbuf);
> +         if (output_file != tmpname)
> +           status = (smart_rename (tmpname,
> +                                   output_file ? output_file : argv[i],
> +-                                  preserve_dates) != 0);
> ++                                  preserve_dates ? &statbuf : NULL) != 0);
> +         if (status == 0)
> +           status = hold_status;
> +       }
> +@@ -5931,11 +5928,9 @@ copy_main (int argc, char *argv[])
> +            output_target, input_arch);
> +   if (status == 0)
> +     {
> +-      if (preserve_dates)
> +-      set_times (tmpname, &statbuf);
> +       if (tmpname != output_filename)
> +       status = (smart_rename (tmpname, input_filename,
> +-                              preserve_dates) != 0);
> ++                              preserve_dates ? &statbuf : NULL) != 0);
> +     }
> +   else
> +     unlink_if_ordinary (tmpname);
> +diff --git a/binutils/rename.c b/binutils/rename.c
> +index 65ad5bf52c4..72a9323d72c 100644
> +--- a/binutils/rename.c
> ++++ b/binutils/rename.c
> +@@ -24,14 +24,9 @@
> +
> + #ifdef HAVE_GOOD_UTIME_H
> + #include <utime.h>
> +-#else /* ! HAVE_GOOD_UTIME_H */
> +-#ifdef HAVE_UTIMES
> ++#elif defined HAVE_UTIMES
> + #include <sys/time.h>
> +-#endif /* HAVE_UTIMES */
> +-#endif /* ! HAVE_GOOD_UTIME_H */
> +-
> +-#if ! defined (_WIN32) || defined (__CYGWIN32__)
> +-static int simple_copy (const char *, const char *);
> ++#endif
> +
> + /* The number of bytes to copy at once.  */
> + #define COPY_BUF 8192
> +@@ -82,7 +77,6 @@ simple_copy (const char *from, const char *to)
> +     }
> +   return 0;
> + }
> +-#endif /* __CYGWIN32__ or not _WIN32 */
> +
> + /* Set the times of the file DESTINATION to be the same as those in
> +    STATBUF.  */
> +@@ -91,122 +85,52 @@ void
> + set_times (const char *destination, const struct stat *statbuf)
> + {
> +   int result;
> +-
> +-  {
> + #ifdef HAVE_GOOD_UTIME_H
> +-    struct utimbuf tb;
> +-
> +-    tb.actime = statbuf->st_atime;
> +-    tb.modtime = statbuf->st_mtime;
> +-    result = utime (destination, &tb);
> +-#else /* ! HAVE_GOOD_UTIME_H */
> +-#ifndef HAVE_UTIMES
> +-    long tb[2];
> +-
> +-    tb[0] = statbuf->st_atime;
> +-    tb[1] = statbuf->st_mtime;
> +-    result = utime (destination, tb);
> +-#else /* HAVE_UTIMES */
> +-    struct timeval tv[2];
> +-
> +-    tv[0].tv_sec = statbuf->st_atime;
> +-    tv[0].tv_usec = 0;
> +-    tv[1].tv_sec = statbuf->st_mtime;
> +-    tv[1].tv_usec = 0;
> +-    result = utimes (destination, tv);
> +-#endif /* HAVE_UTIMES */
> +-#endif /* ! HAVE_GOOD_UTIME_H */
> +-  }
> ++  struct utimbuf tb;
> ++
> ++  tb.actime = statbuf->st_atime;
> ++  tb.modtime = statbuf->st_mtime;
> ++  result = utime (destination, &tb);
> ++#elif defined HAVE_UTIMES
> ++  struct timeval tv[2];
> ++
> ++  tv[0].tv_sec = statbuf->st_atime;
> ++  tv[0].tv_usec = 0;
> ++  tv[1].tv_sec = statbuf->st_mtime;
> ++  tv[1].tv_usec = 0;
> ++  result = utimes (destination, tv);
> ++#else
> ++  long tb[2];
> ++
> ++  tb[0] = statbuf->st_atime;
> ++  tb[1] = statbuf->st_mtime;
> ++  result = utime (destination, tb);
> ++#endif
> +
> +   if (result != 0)
> +     non_fatal (_("%s: cannot set time: %s"), destination, strerror (errno));
> + }
> +
> +-#ifndef S_ISLNK
> +-#ifdef S_IFLNK
> +-#define S_ISLNK(m) (((m) & S_IFMT) == S_IFLNK)
> +-#else
> +-#define S_ISLNK(m) 0
> +-#define lstat stat
> +-#endif
> +-#endif
> +-
> +-/* Rename FROM to TO, copying if TO is a link.
> +-   Return 0 if ok, -1 if error.  */
> ++/* Copy FROM to TO.  TARGET_STAT has the file status that, if non-NULL,
> ++   is used to fix up timestamps.  Return 0 if ok, -1 if error.
> ++   At one time this function renamed files, but file permissions are
> ++   tricky to update given the number of different schemes used by
> ++   various systems.  So now we just copy.  */
> +
> + int
> +-smart_rename (const char *from, const char *to, int preserve_dates ATTRIBUTE_UNUSED)
> ++smart_rename (const char *from, const char *to,
> ++            struct stat *target_stat)
> + {
> +-  bfd_boolean exists;
> +-  struct stat s;
> +-  int ret = 0;
> +-
> +-  exists = lstat (to, &s) == 0;
> +-
> +-#if defined (_WIN32) && !defined (__CYGWIN32__)
> +-  /* Win32, unlike unix, will not erase `to' in `rename(from, to)' but
> +-     fail instead.  Also, chown is not present.  */
> ++  int ret;
> +
> +-  if (exists)
> +-    remove (to);
> +-
> +-  ret = rename (from, to);
> ++  ret = simple_copy (from, to);
> +   if (ret != 0)
> +-    {
> +-      /* We have to clean up here.  */
> +-      non_fatal (_("unable to rename '%s'; reason: %s"), to, strerror (errno));
> +-      unlink (from);
> +-    }
> +-#else
> +-  /* Use rename only if TO is not a symbolic link and has
> +-     only one hard link, and we have permission to write to it.  */
> +-  if (! exists
> +-      || (!S_ISLNK (s.st_mode)
> +-        && S_ISREG (s.st_mode)
> +-        && (s.st_mode & S_IWUSR)
> +-        && s.st_nlink == 1)
> +-      )
> +-    {
> +-      ret = rename (from, to);
> +-      if (ret == 0)
> +-      {
> +-        if (exists)
> +-          {
> +-            /* Try to preserve the permission bits and ownership of
> +-               TO.  First get the mode right except for the setuid
> +-               bit.  Then change the ownership.  Then fix the setuid
> +-               bit.  We do the chmod before the chown because if the
> +-               chown succeeds, and we are a normal user, we won't be
> +-               able to do the chmod afterward.  We don't bother to
> +-               fix the setuid bit first because that might introduce
> +-               a fleeting security problem, and because the chown
> +-               will clear the setuid bit anyhow.  We only fix the
> +-               setuid bit if the chown succeeds, because we don't
> +-               want to introduce an unexpected setuid file owned by
> +-               the user running objcopy.  */
> +-            chmod (to, s.st_mode & 0777);
> +-            if (chown (to, s.st_uid, s.st_gid) >= 0)
> +-              chmod (to, s.st_mode & 07777);
> +-          }
> +-      }
> +-      else
> +-      {
> +-        /* We have to clean up here.  */
> +-        non_fatal (_("unable to rename '%s'; reason: %s"), to, strerror (errno));
> +-        unlink (from);
> +-      }
> +-    }
> +-  else
> +-    {
> +-      ret = simple_copy (from, to);
> +-      if (ret != 0)
> +-      non_fatal (_("unable to copy file '%s'; reason: %s"), to, strerror (errno));
> ++    non_fatal (_("unable to copy file '%s'; reason: %s"),
> ++             to, strerror (errno));
> +
> +-      if (preserve_dates)
> +-      set_times (to, &s);
> +-      unlink (from);
> +-    }
> +-#endif /* _WIN32 && !__CYGWIN32__ */
> ++  if (target_stat != NULL)
> ++    set_times (to, target_stat);
> ++  unlink (from);
> +
> +   return ret;
> + }
> +--
> +2.17.1
> +
> --
> 2.31.1
>
>
> 
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [OE-core] [hardknott][PATCH v4] binutils: Fix CVE-2021-20197
       [not found]               ` <16915DE420F8F12E.31724@lists.openembedded.org>
@ 2021-07-26  4:38                 ` Vinay Kumar
  2021-07-27 14:54                   ` Anuj Mittal
  0 siblings, 1 reply; 18+ messages in thread
From: Vinay Kumar @ 2021-07-26  4:38 UTC (permalink / raw)
  To: Mittal, Anuj
  Cc: Richard Purdie, Patches and discussions about the oe-core layer,
	Randy MacLeod, umesh kalappa0, vinay.kumar

Hi Anuj,

Sent patches for review.

1.    By splitting in to 3 patches.
    [hardknott][PATCH v3] binutils: Fix CVE-2021-20197:
    https://lists.openembedded.org/g/openembedded-core/message/153462

2.    Cherry-pick of master branch
    [hardknott][PATCH v4] binutils: Fix CVE-2021-20197
    https://lists.openembedded.org/g/openembedded-core/message/153809

Please let me know if you feel any more information needed in the patches.

Regards,
Vinay

On Tue, Jul 13, 2021 at 7:19 PM Vinay Kumar via lists.openembedded.org
<vinay.m.engg=gmail.com@lists.openembedded.org> wrote:
>
> Hi Anuj,
>
> This patch is a cherry-pick of
> "f628b052f6c4a957472f2520adfb4b816ef83d95" from master branch.
>
> Regards,
> Vinay
>
> On Tue, Jul 13, 2021 at 7:16 PM Vinay Kumar via lists.openembedded.org
> <vinay.m.engg=gmail.com@lists.openembedded.org> wrote:
> >
> > Source: git://sourceware.org/git/binutils-gdb.git
> > Tracking -- https://sourceware.org/bugzilla/show_bug.cgi?id=26945
> >
> > Backported upstream commit d3edaa91d4cf7202ec14342410194841e2f67f12 to
> > binutils-2.36 source along with commit id dependencies
> > (8e03235147a9e774d3ba084e93c2da1aa94d1cec and 8b69e61d4be276bb862698aaafddc3e779d23c8f).
> >
> > Upstream-Status: Backport [https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d3edaa91d4cf7202ec14342410194841e2f67f12]
> >
> > Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
> > ---
> >  .../binutils/binutils-2.36.inc                |   1 +
> >  .../binutils/binutils/CVE-2021-20197.patch    | 388 ++++++++++++++++++
> >  2 files changed, 389 insertions(+)
> >  create mode 100644 meta/recipes-devtools/binutils/binutils/CVE-2021-20197.patch
> >
> > diff --git a/meta/recipes-devtools/binutils/binutils-2.36.inc b/meta/recipes-devtools/binutils/binutils-2.36.inc
> > index 2968291889..6d0293b4bc 100644
> > --- a/meta/recipes-devtools/binutils/binutils-2.36.inc
> > +++ b/meta/recipes-devtools/binutils/binutils-2.36.inc
> > @@ -41,5 +41,6 @@ SRC_URI = "\
> >       file://0014-Fix-rpath-in-libtool-when-sysroot-is-enabled.patch \
> >       file://0015-sync-with-OE-libtool-changes.patch \
> >       file://0016-Check-for-clang-before-checking-gcc-version.patch \
> > +     file://CVE-2021-20197.patch \
> >  "
> >  S  = "${WORKDIR}/git"
> > diff --git a/meta/recipes-devtools/binutils/binutils/CVE-2021-20197.patch b/meta/recipes-devtools/binutils/binutils/CVE-2021-20197.patch
> > new file mode 100644
> > index 0000000000..d6117b65a4
> > --- /dev/null
> > +++ b/meta/recipes-devtools/binutils/binutils/CVE-2021-20197.patch
> > @@ -0,0 +1,388 @@
> > +From d3edaa91d4cf7202ec14342410194841e2f67f12 Mon Sep 17 00:00:00 2001
> > +From: Alan Modra <amodra@gmail.com>
> > +Date: Fri, 26 Feb 2021 11:30:32 +1030
> > +Subject: [PATCH] Reinstate various pieces backed out from smart_rename changes
> > +
> > +In the interests of a stable release various last minute smart_rename
> > +patches were backed out of the 2.36 branch.  The main reason to
> > +reinstate some of those backed out changes here is to make necessary
> > +followup fixes to commit 8e03235147a9 simple cherry-picks from
> > +mainline.  A secondary reason is that ar -M support isn't fixed for
> > +pr26945 without this patch.
> > +
> > +        PR 26945
> > +        * ar.c: Don't include libbfd.h.
> > +        (write_archive): Replace xmalloc+strcpy with xstrdup.
> > +        * arsup.c (temp_name, real_ofd): New static variables.
> > +        (ar_open): Use make_tempname and bfd_fdopenw.
> > +        (ar_save): Adjust to suit ar_open changes.
> > +        * objcopy.c: Don't include libbfd.h.
> > +        * rename.c: Rename and reorder variables.
> > +
> > +(cherry picked from commit 95b91a043aeaeb546d2fea556d84a2de1e917770)
> > +
> > +Upstream-Status: Backport [https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d3edaa91d4cf7202ec14342410194841e2f67f12]
> > +CVE: CVE-2021-20197
> > +Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
> > +---
> > + binutils/ar.c      |   6 +-
> > + binutils/arsup.c   |  37 ++++++++----
> > + binutils/bucomm.h  |   3 +-
> > + binutils/objcopy.c |   9 +--
> > + binutils/rename.c  | 148 +++++++++++----------------------------------
> > + 5 files changed, 67 insertions(+), 136 deletions(-)
> > +
> > +diff --git a/binutils/ar.c b/binutils/ar.c
> > +index 45a34e3a6cf..44df48c5c67 100644
> > +--- a/binutils/ar.c
> > ++++ b/binutils/ar.c
> > +@@ -25,7 +25,6 @@
> > +
> > + #include "sysdep.h"
> > + #include "bfd.h"
> > +-#include "libbfd.h"
> > + #include "libiberty.h"
> > + #include "progress.h"
> > + #include "getopt.h"
> > +@@ -1255,8 +1254,7 @@ write_archive (bfd *iarch)
> > +   bfd *contents_head = iarch->archive_next;
> > +   int ofd = -1;
> > +
> > +-  old_name = (char *) xmalloc (strlen (bfd_get_filename (iarch)) + 1);
> > +-  strcpy (old_name, bfd_get_filename (iarch));
> > ++  old_name = xstrdup (bfd_get_filename (iarch));
> > +   new_name = make_tempname (old_name, &ofd);
> > +
> > +   if (new_name == NULL)
> > +@@ -1308,7 +1306,7 @@ write_archive (bfd *iarch)
> > +   /* We don't care if this fails; we might be creating the archive.  */
> > +   bfd_close (iarch);
> > +
> > +-  if (smart_rename (new_name, old_name, 0) != 0)
> > ++  if (smart_rename (new_name, old_name, NULL) != 0)
> > +     xexit (1);
> > +   free (old_name);
> > +   free (new_name);
> > +diff --git a/binutils/arsup.c b/binutils/arsup.c
> > +index 5403a0c5d74..f7ce8f0bc82 100644
> > +--- a/binutils/arsup.c
> > ++++ b/binutils/arsup.c
> > +@@ -42,6 +42,8 @@ extern int deterministic;
> > +
> > + static bfd *obfd;
> > + static char *real_name;
> > ++static char *temp_name;
> > ++static int real_ofd;
> > + static FILE *outfile;
> > +
> > + static void
> > +@@ -149,27 +151,24 @@ maybequit (void)
> > + void
> > + ar_open (char *name, int t)
> > + {
> > +-  char *tname;
> > +-  const char *bname = lbasename (name);
> > +-  real_name = name;
> > ++  real_name = xstrdup (name);
> > ++  temp_name = make_tempname (real_name, &real_ofd);
> > +
> > +-  /* Prepend tmp- to the beginning, to avoid file-name clashes after
> > +-     truncation on filesystems with limited namespaces (DOS).  */
> > +-  if (asprintf (&tname, "%.*stmp-%s", (int) (bname - name), name, bname) == -1)
> > ++  if (temp_name == NULL)
> > +     {
> > +-      fprintf (stderr, _("%s: Can't allocate memory for temp name (%s)\n"),
> > ++      fprintf (stderr, _("%s: Can't open temporary file (%s)\n"),
> > +              program_name, strerror(errno));
> > +       maybequit ();
> > +       return;
> > +     }
> > +
> > +-  obfd = bfd_openw (tname, NULL);
> > ++  obfd = bfd_fdopenw (temp_name, NULL, real_ofd);
> > +
> > +   if (!obfd)
> > +     {
> > +       fprintf (stderr,
> > +              _("%s: Can't open output archive %s\n"),
> > +-             program_name,  tname);
> > ++             program_name, temp_name);
> > +
> > +       maybequit ();
> > +     }
> > +@@ -344,16 +343,30 @@ ar_save (void)
> > +     }
> > +   else
> > +     {
> > +-      char *ofilename = xstrdup (bfd_get_filename (obfd));
> > ++      struct stat target_stat;
> > +
> > +       if (deterministic > 0)
> > +         obfd->flags |= BFD_DETERMINISTIC_OUTPUT;
> > +
> > +       bfd_close (obfd);
> > +
> > +-      smart_rename (ofilename, real_name, 0);
> > ++      if (stat (real_name, &target_stat) != 0)
> > ++      {
> > ++        /* The temp file created in ar_open has mode 0600 as per mkstemp.
> > ++           Create the real empty output file here so smart_rename will
> > ++           update the mode according to the process umask.  */
> > ++        obfd = bfd_openw (real_name, NULL);
> > ++        if (obfd != NULL)
> > ++          {
> > ++            bfd_set_format (obfd, bfd_archive);
> > ++            bfd_close (obfd);
> > ++          }
> > ++      }
> > ++
> > ++      smart_rename (temp_name, real_name, NULL);
> > +       obfd = 0;
> > +-      free (ofilename);
> > ++      free (temp_name);
> > ++      free (real_name);
> > +     }
> > + }
> > +
> > +diff --git a/binutils/bucomm.h b/binutils/bucomm.h
> > +index 91f6a5b228f..aa7e33d8cd1 100644
> > +--- a/binutils/bucomm.h
> > ++++ b/binutils/bucomm.h
> > +@@ -71,7 +71,8 @@ extern void print_version (const char *);
> > + /* In rename.c.  */
> > + extern void set_times (const char *, const struct stat *);
> > +
> > +-extern int smart_rename (const char *, const char *, int);
> > ++extern int smart_rename (const char *, const char *, struct stat *);
> > ++
> > +
> > + /* In libiberty.  */
> > + void *xmalloc (size_t);
> > +diff --git a/binutils/objcopy.c b/binutils/objcopy.c
> > +index eab3b6db585..73aa8bc2514 100644
> > +--- a/binutils/objcopy.c
> > ++++ b/binutils/objcopy.c
> > +@@ -20,7 +20,6 @@
> > +
> > + #include "sysdep.h"
> > + #include "bfd.h"
> > +-#include "libbfd.h"
> > + #include "progress.h"
> > + #include "getopt.h"
> > + #include "libiberty.h"
> > +@@ -4861,12 +4860,10 @@ strip_main (int argc, char *argv[])
> > +                output_target, NULL);
> > +       if (status == 0)
> > +       {
> > +-        if (preserve_dates)
> > +-          set_times (tmpname, &statbuf);
> > +         if (output_file != tmpname)
> > +           status = (smart_rename (tmpname,
> > +                                   output_file ? output_file : argv[i],
> > +-                                  preserve_dates) != 0);
> > ++                                  preserve_dates ? &statbuf : NULL) != 0);
> > +         if (status == 0)
> > +           status = hold_status;
> > +       }
> > +@@ -5931,11 +5928,9 @@ copy_main (int argc, char *argv[])
> > +            output_target, input_arch);
> > +   if (status == 0)
> > +     {
> > +-      if (preserve_dates)
> > +-      set_times (tmpname, &statbuf);
> > +       if (tmpname != output_filename)
> > +       status = (smart_rename (tmpname, input_filename,
> > +-                              preserve_dates) != 0);
> > ++                              preserve_dates ? &statbuf : NULL) != 0);
> > +     }
> > +   else
> > +     unlink_if_ordinary (tmpname);
> > +diff --git a/binutils/rename.c b/binutils/rename.c
> > +index 65ad5bf52c4..72a9323d72c 100644
> > +--- a/binutils/rename.c
> > ++++ b/binutils/rename.c
> > +@@ -24,14 +24,9 @@
> > +
> > + #ifdef HAVE_GOOD_UTIME_H
> > + #include <utime.h>
> > +-#else /* ! HAVE_GOOD_UTIME_H */
> > +-#ifdef HAVE_UTIMES
> > ++#elif defined HAVE_UTIMES
> > + #include <sys/time.h>
> > +-#endif /* HAVE_UTIMES */
> > +-#endif /* ! HAVE_GOOD_UTIME_H */
> > +-
> > +-#if ! defined (_WIN32) || defined (__CYGWIN32__)
> > +-static int simple_copy (const char *, const char *);
> > ++#endif
> > +
> > + /* The number of bytes to copy at once.  */
> > + #define COPY_BUF 8192
> > +@@ -82,7 +77,6 @@ simple_copy (const char *from, const char *to)
> > +     }
> > +   return 0;
> > + }
> > +-#endif /* __CYGWIN32__ or not _WIN32 */
> > +
> > + /* Set the times of the file DESTINATION to be the same as those in
> > +    STATBUF.  */
> > +@@ -91,122 +85,52 @@ void
> > + set_times (const char *destination, const struct stat *statbuf)
> > + {
> > +   int result;
> > +-
> > +-  {
> > + #ifdef HAVE_GOOD_UTIME_H
> > +-    struct utimbuf tb;
> > +-
> > +-    tb.actime = statbuf->st_atime;
> > +-    tb.modtime = statbuf->st_mtime;
> > +-    result = utime (destination, &tb);
> > +-#else /* ! HAVE_GOOD_UTIME_H */
> > +-#ifndef HAVE_UTIMES
> > +-    long tb[2];
> > +-
> > +-    tb[0] = statbuf->st_atime;
> > +-    tb[1] = statbuf->st_mtime;
> > +-    result = utime (destination, tb);
> > +-#else /* HAVE_UTIMES */
> > +-    struct timeval tv[2];
> > +-
> > +-    tv[0].tv_sec = statbuf->st_atime;
> > +-    tv[0].tv_usec = 0;
> > +-    tv[1].tv_sec = statbuf->st_mtime;
> > +-    tv[1].tv_usec = 0;
> > +-    result = utimes (destination, tv);
> > +-#endif /* HAVE_UTIMES */
> > +-#endif /* ! HAVE_GOOD_UTIME_H */
> > +-  }
> > ++  struct utimbuf tb;
> > ++
> > ++  tb.actime = statbuf->st_atime;
> > ++  tb.modtime = statbuf->st_mtime;
> > ++  result = utime (destination, &tb);
> > ++#elif defined HAVE_UTIMES
> > ++  struct timeval tv[2];
> > ++
> > ++  tv[0].tv_sec = statbuf->st_atime;
> > ++  tv[0].tv_usec = 0;
> > ++  tv[1].tv_sec = statbuf->st_mtime;
> > ++  tv[1].tv_usec = 0;
> > ++  result = utimes (destination, tv);
> > ++#else
> > ++  long tb[2];
> > ++
> > ++  tb[0] = statbuf->st_atime;
> > ++  tb[1] = statbuf->st_mtime;
> > ++  result = utime (destination, tb);
> > ++#endif
> > +
> > +   if (result != 0)
> > +     non_fatal (_("%s: cannot set time: %s"), destination, strerror (errno));
> > + }
> > +
> > +-#ifndef S_ISLNK
> > +-#ifdef S_IFLNK
> > +-#define S_ISLNK(m) (((m) & S_IFMT) == S_IFLNK)
> > +-#else
> > +-#define S_ISLNK(m) 0
> > +-#define lstat stat
> > +-#endif
> > +-#endif
> > +-
> > +-/* Rename FROM to TO, copying if TO is a link.
> > +-   Return 0 if ok, -1 if error.  */
> > ++/* Copy FROM to TO.  TARGET_STAT has the file status that, if non-NULL,
> > ++   is used to fix up timestamps.  Return 0 if ok, -1 if error.
> > ++   At one time this function renamed files, but file permissions are
> > ++   tricky to update given the number of different schemes used by
> > ++   various systems.  So now we just copy.  */
> > +
> > + int
> > +-smart_rename (const char *from, const char *to, int preserve_dates ATTRIBUTE_UNUSED)
> > ++smart_rename (const char *from, const char *to,
> > ++            struct stat *target_stat)
> > + {
> > +-  bfd_boolean exists;
> > +-  struct stat s;
> > +-  int ret = 0;
> > +-
> > +-  exists = lstat (to, &s) == 0;
> > +-
> > +-#if defined (_WIN32) && !defined (__CYGWIN32__)
> > +-  /* Win32, unlike unix, will not erase `to' in `rename(from, to)' but
> > +-     fail instead.  Also, chown is not present.  */
> > ++  int ret;
> > +
> > +-  if (exists)
> > +-    remove (to);
> > +-
> > +-  ret = rename (from, to);
> > ++  ret = simple_copy (from, to);
> > +   if (ret != 0)
> > +-    {
> > +-      /* We have to clean up here.  */
> > +-      non_fatal (_("unable to rename '%s'; reason: %s"), to, strerror (errno));
> > +-      unlink (from);
> > +-    }
> > +-#else
> > +-  /* Use rename only if TO is not a symbolic link and has
> > +-     only one hard link, and we have permission to write to it.  */
> > +-  if (! exists
> > +-      || (!S_ISLNK (s.st_mode)
> > +-        && S_ISREG (s.st_mode)
> > +-        && (s.st_mode & S_IWUSR)
> > +-        && s.st_nlink == 1)
> > +-      )
> > +-    {
> > +-      ret = rename (from, to);
> > +-      if (ret == 0)
> > +-      {
> > +-        if (exists)
> > +-          {
> > +-            /* Try to preserve the permission bits and ownership of
> > +-               TO.  First get the mode right except for the setuid
> > +-               bit.  Then change the ownership.  Then fix the setuid
> > +-               bit.  We do the chmod before the chown because if the
> > +-               chown succeeds, and we are a normal user, we won't be
> > +-               able to do the chmod afterward.  We don't bother to
> > +-               fix the setuid bit first because that might introduce
> > +-               a fleeting security problem, and because the chown
> > +-               will clear the setuid bit anyhow.  We only fix the
> > +-               setuid bit if the chown succeeds, because we don't
> > +-               want to introduce an unexpected setuid file owned by
> > +-               the user running objcopy.  */
> > +-            chmod (to, s.st_mode & 0777);
> > +-            if (chown (to, s.st_uid, s.st_gid) >= 0)
> > +-              chmod (to, s.st_mode & 07777);
> > +-          }
> > +-      }
> > +-      else
> > +-      {
> > +-        /* We have to clean up here.  */
> > +-        non_fatal (_("unable to rename '%s'; reason: %s"), to, strerror (errno));
> > +-        unlink (from);
> > +-      }
> > +-    }
> > +-  else
> > +-    {
> > +-      ret = simple_copy (from, to);
> > +-      if (ret != 0)
> > +-      non_fatal (_("unable to copy file '%s'; reason: %s"), to, strerror (errno));
> > ++    non_fatal (_("unable to copy file '%s'; reason: %s"),
> > ++             to, strerror (errno));
> > +
> > +-      if (preserve_dates)
> > +-      set_times (to, &s);
> > +-      unlink (from);
> > +-    }
> > +-#endif /* _WIN32 && !__CYGWIN32__ */
> > ++  if (target_stat != NULL)
> > ++    set_times (to, target_stat);
> > ++  unlink (from);
> > +
> > +   return ret;
> > + }
> > +--
> > +2.17.1
> > +
> > --
> > 2.31.1
> >
> >
> >
> >
>
> 
>

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [OE-core] [hardknott][PATCH v4] binutils: Fix CVE-2021-20197
  2021-07-26  4:38                 ` Vinay Kumar
@ 2021-07-27 14:54                   ` Anuj Mittal
  0 siblings, 0 replies; 18+ messages in thread
From: Anuj Mittal @ 2021-07-27 14:54 UTC (permalink / raw)
  To: vinay.m.engg; +Cc: openembedded-core

Hello,

On Mon, 2021-07-26 at 10:08 +0530, Vinay Kumar wrote:
> Hi Anuj,
> 
> Sent patches for review.
> 
> 1.    By splitting in to 3 patches.
>     [hardknott][PATCH v3] binutils: Fix CVE-2021-20197:
>     https://lists.openembedded.org/g/openembedded-core/message/153462

This has already been merged:

https://git.yoctoproject.org/cgit/cgit.cgi/poky/commit/?h=hardknott&id=db00b0a059013a19b333fd291bde8cee71fc6a11

Thanks,

Anuj


> 
> 2.    Cherry-pick of master branch
>     [hardknott][PATCH v4] binutils: Fix CVE-2021-20197
>     https://lists.openembedded.org/g/openembedded-core/message/153809
> 
> Please let me know if you feel any more information needed in the
> patches.
> 
> Regards,
> Vinay
> 
> On Tue, Jul 13, 2021 at 7:19 PM Vinay Kumar via lists.openembedded.org
> <vinay.m.engg=gmail.com@lists.openembedded.org> wrote:
> > 
> > Hi Anuj,
> > 
> > This patch is a cherry-pick of
> > "f628b052f6c4a957472f2520adfb4b816ef83d95" from master branch.
> > 
> > Regards,
> > Vinay
> > 
> > On Tue, Jul 13, 2021 at 7:16 PM Vinay Kumar via
> > lists.openembedded.org
> > <vinay.m.engg=gmail.com@lists.openembedded.org> wrote:
> > > 
> > > Source: git://sourceware.org/git/binutils-gdb.git
> > > Tracking -- https://sourceware.org/bugzilla/show_bug.cgi?id=26945
> > > 
> > > Backported upstream commit d3edaa91d4cf7202ec14342410194841e2f67f12
> > > to
> > > binutils-2.36 source along with commit id dependencies
> > > (8e03235147a9e774d3ba084e93c2da1aa94d1cec and
> > > 8b69e61d4be276bb862698aaafddc3e779d23c8f).
> > > 
> > > Upstream-Status: Backport
> > > [https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d3edaa91d4cf7202ec14342410194841e2f67f12
> > > ]
> > > 
> > > Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
> > > ---
> > >  .../binutils/binutils-2.36.inc                |   1 +
> > >  .../binutils/binutils/CVE-2021-20197.patch    | 388
> > > ++++++++++++++++++
> > >  2 files changed, 389 insertions(+)
> > >  create mode 100644 meta/recipes-devtools/binutils/binutils/CVE-
> > > 2021-20197.patch
> > > 
> > > diff --git a/meta/recipes-devtools/binutils/binutils-2.36.inc
> > > b/meta/recipes-devtools/binutils/binutils-2.36.inc
> > > index 2968291889..6d0293b4bc 100644
> > > --- a/meta/recipes-devtools/binutils/binutils-2.36.inc
> > > +++ b/meta/recipes-devtools/binutils/binutils-2.36.inc
> > > @@ -41,5 +41,6 @@ SRC_URI = "\
> > >      
> > > file://0014-Fix-rpath-in-libtool-when-sysroot-is-enabled.patch \
> > >       file://0015-sync-with-OE-libtool-changes.patch \
> > >      
> > > file://0016-Check-for-clang-before-checking-gcc-version.patch \
> > > +     file://CVE-2021-20197.patch \
> > >  "
> > >  S  = "${WORKDIR}/git"
> > > diff --git a/meta/recipes-devtools/binutils/binutils/CVE-2021-
> > > 20197.patch b/meta/recipes-devtools/binutils/binutils/CVE-2021-
> > > 20197.patch
> > > new file mode 100644
> > > index 0000000000..d6117b65a4
> > > --- /dev/null
> > > +++ b/meta/recipes-devtools/binutils/binutils/CVE-2021-20197.patch
> > > @@ -0,0 +1,388 @@
> > > +From d3edaa91d4cf7202ec14342410194841e2f67f12 Mon Sep 17 00:00:00
> > > 2001
> > > +From: Alan Modra <amodra@gmail.com>
> > > +Date: Fri, 26 Feb 2021 11:30:32 +1030
> > > +Subject: [PATCH] Reinstate various pieces backed out from
> > > smart_rename changes
> > > +
> > > +In the interests of a stable release various last minute
> > > smart_rename
> > > +patches were backed out of the 2.36 branch.  The main reason to
> > > +reinstate some of those backed out changes here is to make
> > > necessary
> > > +followup fixes to commit 8e03235147a9 simple cherry-picks from
> > > +mainline.  A secondary reason is that ar -M support isn't fixed
> > > for
> > > +pr26945 without this patch.
> > > +
> > > +        PR 26945
> > > +        * ar.c: Don't include libbfd.h.
> > > +        (write_archive): Replace xmalloc+strcpy with xstrdup.
> > > +        * arsup.c (temp_name, real_ofd): New static variables.
> > > +        (ar_open): Use make_tempname and bfd_fdopenw.
> > > +        (ar_save): Adjust to suit ar_open changes.
> > > +        * objcopy.c: Don't include libbfd.h.
> > > +        * rename.c: Rename and reorder variables.
> > > +
> > > +(cherry picked from commit
> > > 95b91a043aeaeb546d2fea556d84a2de1e917770)
> > > +
> > > +Upstream-Status: Backport
> > > [https://sourceware.org/git/gitweb.cgi?p=binutils-gdb.git;h=d3edaa91d4cf7202ec14342410194841e2f67f12
> > > ]
> > > +CVE: CVE-2021-20197
> > > +Signed-off-by: Vinay Kumar <vinay.m.engg@gmail.com>
> > > +---
> > > + binutils/ar.c      |   6 +-
> > > + binutils/arsup.c   |  37 ++++++++----
> > > + binutils/bucomm.h  |   3 +-
> > > + binutils/objcopy.c |   9 +--
> > > + binutils/rename.c  | 148 +++++++++++-----------------------------
> > > -----
> > > + 5 files changed, 67 insertions(+), 136 deletions(-)
> > > +
> > > +diff --git a/binutils/ar.c b/binutils/ar.c
> > > +index 45a34e3a6cf..44df48c5c67 100644
> > > +--- a/binutils/ar.c
> > > ++++ b/binutils/ar.c
> > > +@@ -25,7 +25,6 @@
> > > +
> > > + #include "sysdep.h"
> > > + #include "bfd.h"
> > > +-#include "libbfd.h"
> > > + #include "libiberty.h"
> > > + #include "progress.h"
> > > + #include "getopt.h"
> > > +@@ -1255,8 +1254,7 @@ write_archive (bfd *iarch)
> > > +   bfd *contents_head = iarch->archive_next;
> > > +   int ofd = -1;
> > > +
> > > +-  old_name = (char *) xmalloc (strlen (bfd_get_filename (iarch))
> > > + 1);
> > > +-  strcpy (old_name, bfd_get_filename (iarch));
> > > ++  old_name = xstrdup (bfd_get_filename (iarch));
> > > +   new_name = make_tempname (old_name, &ofd);
> > > +
> > > +   if (new_name == NULL)
> > > +@@ -1308,7 +1306,7 @@ write_archive (bfd *iarch)
> > > +   /* We don't care if this fails; we might be creating the
> > > archive.  */
> > > +   bfd_close (iarch);
> > > +
> > > +-  if (smart_rename (new_name, old_name, 0) != 0)
> > > ++  if (smart_rename (new_name, old_name, NULL) != 0)
> > > +     xexit (1);
> > > +   free (old_name);
> > > +   free (new_name);
> > > +diff --git a/binutils/arsup.c b/binutils/arsup.c
> > > +index 5403a0c5d74..f7ce8f0bc82 100644
> > > +--- a/binutils/arsup.c
> > > ++++ b/binutils/arsup.c
> > > +@@ -42,6 +42,8 @@ extern int deterministic;
> > > +
> > > + static bfd *obfd;
> > > + static char *real_name;
> > > ++static char *temp_name;
> > > ++static int real_ofd;
> > > + static FILE *outfile;
> > > +
> > > + static void
> > > +@@ -149,27 +151,24 @@ maybequit (void)
> > > + void
> > > + ar_open (char *name, int t)
> > > + {
> > > +-  char *tname;
> > > +-  const char *bname = lbasename (name);
> > > +-  real_name = name;
> > > ++  real_name = xstrdup (name);
> > > ++  temp_name = make_tempname (real_name, &real_ofd);
> > > +
> > > +-  /* Prepend tmp- to the beginning, to avoid file-name clashes
> > > after
> > > +-     truncation on filesystems with limited namespaces (DOS).  */
> > > +-  if (asprintf (&tname, "%.*stmp-%s", (int) (bname - name), name,
> > > bname) == -1)
> > > ++  if (temp_name == NULL)
> > > +     {
> > > +-      fprintf (stderr, _("%s: Can't allocate memory for temp name
> > > (%s)\n"),
> > > ++      fprintf (stderr, _("%s: Can't open temporary file (%s)\n"),
> > > +              program_name, strerror(errno));
> > > +       maybequit ();
> > > +       return;
> > > +     }
> > > +
> > > +-  obfd = bfd_openw (tname, NULL);
> > > ++  obfd = bfd_fdopenw (temp_name, NULL, real_ofd);
> > > +
> > > +   if (!obfd)
> > > +     {
> > > +       fprintf (stderr,
> > > +              _("%s: Can't open output archive %s\n"),
> > > +-             program_name,  tname);
> > > ++             program_name, temp_name);
> > > +
> > > +       maybequit ();
> > > +     }
> > > +@@ -344,16 +343,30 @@ ar_save (void)
> > > +     }
> > > +   else
> > > +     {
> > > +-      char *ofilename = xstrdup (bfd_get_filename (obfd));
> > > ++      struct stat target_stat;
> > > +
> > > +       if (deterministic > 0)
> > > +         obfd->flags |= BFD_DETERMINISTIC_OUTPUT;
> > > +
> > > +       bfd_close (obfd);
> > > +
> > > +-      smart_rename (ofilename, real_name, 0);
> > > ++      if (stat (real_name, &target_stat) != 0)
> > > ++      {
> > > ++        /* The temp file created in ar_open has mode 0600 as per
> > > mkstemp.
> > > ++           Create the real empty output file here so smart_rename
> > > will
> > > ++           update the mode according to the process umask.  */
> > > ++        obfd = bfd_openw (real_name, NULL);
> > > ++        if (obfd != NULL)
> > > ++          {
> > > ++            bfd_set_format (obfd, bfd_archive);
> > > ++            bfd_close (obfd);
> > > ++          }
> > > ++      }
> > > ++
> > > ++      smart_rename (temp_name, real_name, NULL);
> > > +       obfd = 0;
> > > +-      free (ofilename);
> > > ++      free (temp_name);
> > > ++      free (real_name);
> > > +     }
> > > + }
> > > +
> > > +diff --git a/binutils/bucomm.h b/binutils/bucomm.h
> > > +index 91f6a5b228f..aa7e33d8cd1 100644
> > > +--- a/binutils/bucomm.h
> > > ++++ b/binutils/bucomm.h
> > > +@@ -71,7 +71,8 @@ extern void print_version (const char *);
> > > + /* In rename.c.  */
> > > + extern void set_times (const char *, const struct stat *);
> > > +
> > > +-extern int smart_rename (const char *, const char *, int);
> > > ++extern int smart_rename (const char *, const char *, struct stat
> > > *);
> > > ++
> > > +
> > > + /* In libiberty.  */
> > > + void *xmalloc (size_t);
> > > +diff --git a/binutils/objcopy.c b/binutils/objcopy.c
> > > +index eab3b6db585..73aa8bc2514 100644
> > > +--- a/binutils/objcopy.c
> > > ++++ b/binutils/objcopy.c
> > > +@@ -20,7 +20,6 @@
> > > +
> > > + #include "sysdep.h"
> > > + #include "bfd.h"
> > > +-#include "libbfd.h"
> > > + #include "progress.h"
> > > + #include "getopt.h"
> > > + #include "libiberty.h"
> > > +@@ -4861,12 +4860,10 @@ strip_main (int argc, char *argv[])
> > > +                output_target, NULL);
> > > +       if (status == 0)
> > > +       {
> > > +-        if (preserve_dates)
> > > +-          set_times (tmpname, &statbuf);
> > > +         if (output_file != tmpname)
> > > +           status = (smart_rename (tmpname,
> > > +                                   output_file ? output_file :
> > > argv[i],
> > > +-                                  preserve_dates) != 0);
> > > ++                                  preserve_dates ? &statbuf :
> > > NULL) != 0);
> > > +         if (status == 0)
> > > +           status = hold_status;
> > > +       }
> > > +@@ -5931,11 +5928,9 @@ copy_main (int argc, char *argv[])
> > > +            output_target, input_arch);
> > > +   if (status == 0)
> > > +     {
> > > +-      if (preserve_dates)
> > > +-      set_times (tmpname, &statbuf);
> > > +       if (tmpname != output_filename)
> > > +       status = (smart_rename (tmpname, input_filename,
> > > +-                              preserve_dates) != 0);
> > > ++                              preserve_dates ? &statbuf : NULL)
> > > != 0);
> > > +     }
> > > +   else
> > > +     unlink_if_ordinary (tmpname);
> > > +diff --git a/binutils/rename.c b/binutils/rename.c
> > > +index 65ad5bf52c4..72a9323d72c 100644
> > > +--- a/binutils/rename.c
> > > ++++ b/binutils/rename.c
> > > +@@ -24,14 +24,9 @@
> > > +
> > > + #ifdef HAVE_GOOD_UTIME_H
> > > + #include <utime.h>
> > > +-#else /* ! HAVE_GOOD_UTIME_H */
> > > +-#ifdef HAVE_UTIMES
> > > ++#elif defined HAVE_UTIMES
> > > + #include <sys/time.h>
> > > +-#endif /* HAVE_UTIMES */
> > > +-#endif /* ! HAVE_GOOD_UTIME_H */
> > > +-
> > > +-#if ! defined (_WIN32) || defined (__CYGWIN32__)
> > > +-static int simple_copy (const char *, const char *);
> > > ++#endif
> > > +
> > > + /* The number of bytes to copy at once.  */
> > > + #define COPY_BUF 8192
> > > +@@ -82,7 +77,6 @@ simple_copy (const char *from, const char *to)
> > > +     }
> > > +   return 0;
> > > + }
> > > +-#endif /* __CYGWIN32__ or not _WIN32 */
> > > +
> > > + /* Set the times of the file DESTINATION to be the same as those
> > > in
> > > +    STATBUF.  */
> > > +@@ -91,122 +85,52 @@ void
> > > + set_times (const char *destination, const struct stat *statbuf)
> > > + {
> > > +   int result;
> > > +-
> > > +-  {
> > > + #ifdef HAVE_GOOD_UTIME_H
> > > +-    struct utimbuf tb;
> > > +-
> > > +-    tb.actime = statbuf->st_atime;
> > > +-    tb.modtime = statbuf->st_mtime;
> > > +-    result = utime (destination, &tb);
> > > +-#else /* ! HAVE_GOOD_UTIME_H */
> > > +-#ifndef HAVE_UTIMES
> > > +-    long tb[2];
> > > +-
> > > +-    tb[0] = statbuf->st_atime;
> > > +-    tb[1] = statbuf->st_mtime;
> > > +-    result = utime (destination, tb);
> > > +-#else /* HAVE_UTIMES */
> > > +-    struct timeval tv[2];
> > > +-
> > > +-    tv[0].tv_sec = statbuf->st_atime;
> > > +-    tv[0].tv_usec = 0;
> > > +-    tv[1].tv_sec = statbuf->st_mtime;
> > > +-    tv[1].tv_usec = 0;
> > > +-    result = utimes (destination, tv);
> > > +-#endif /* HAVE_UTIMES */
> > > +-#endif /* ! HAVE_GOOD_UTIME_H */
> > > +-  }
> > > ++  struct utimbuf tb;
> > > ++
> > > ++  tb.actime = statbuf->st_atime;
> > > ++  tb.modtime = statbuf->st_mtime;
> > > ++  result = utime (destination, &tb);
> > > ++#elif defined HAVE_UTIMES
> > > ++  struct timeval tv[2];
> > > ++
> > > ++  tv[0].tv_sec = statbuf->st_atime;
> > > ++  tv[0].tv_usec = 0;
> > > ++  tv[1].tv_sec = statbuf->st_mtime;
> > > ++  tv[1].tv_usec = 0;
> > > ++  result = utimes (destination, tv);
> > > ++#else
> > > ++  long tb[2];
> > > ++
> > > ++  tb[0] = statbuf->st_atime;
> > > ++  tb[1] = statbuf->st_mtime;
> > > ++  result = utime (destination, tb);
> > > ++#endif
> > > +
> > > +   if (result != 0)
> > > +     non_fatal (_("%s: cannot set time: %s"), destination,
> > > strerror (errno));
> > > + }
> > > +
> > > +-#ifndef S_ISLNK
> > > +-#ifdef S_IFLNK
> > > +-#define S_ISLNK(m) (((m) & S_IFMT) == S_IFLNK)
> > > +-#else
> > > +-#define S_ISLNK(m) 0
> > > +-#define lstat stat
> > > +-#endif
> > > +-#endif
> > > +-
> > > +-/* Rename FROM to TO, copying if TO is a link.
> > > +-   Return 0 if ok, -1 if error.  */
> > > ++/* Copy FROM to TO.  TARGET_STAT has the file status that, if
> > > non-NULL,
> > > ++   is used to fix up timestamps.  Return 0 if ok, -1 if error.
> > > ++   At one time this function renamed files, but file permissions
> > > are
> > > ++   tricky to update given the number of different schemes used by
> > > ++   various systems.  So now we just copy.  */
> > > +
> > > + int
> > > +-smart_rename (const char *from, const char *to, int
> > > preserve_dates ATTRIBUTE_UNUSED)
> > > ++smart_rename (const char *from, const char *to,
> > > ++            struct stat *target_stat)
> > > + {
> > > +-  bfd_boolean exists;
> > > +-  struct stat s;
> > > +-  int ret = 0;
> > > +-
> > > +-  exists = lstat (to, &s) == 0;
> > > +-
> > > +-#if defined (_WIN32) && !defined (__CYGWIN32__)
> > > +-  /* Win32, unlike unix, will not erase `to' in `rename(from,
> > > to)' but
> > > +-     fail instead.  Also, chown is not present.  */
> > > ++  int ret;
> > > +
> > > +-  if (exists)
> > > +-    remove (to);
> > > +-
> > > +-  ret = rename (from, to);
> > > ++  ret = simple_copy (from, to);
> > > +   if (ret != 0)
> > > +-    {
> > > +-      /* We have to clean up here.  */
> > > +-      non_fatal (_("unable to rename '%s'; reason: %s"), to,
> > > strerror (errno));
> > > +-      unlink (from);
> > > +-    }
> > > +-#else
> > > +-  /* Use rename only if TO is not a symbolic link and has
> > > +-     only one hard link, and we have permission to write to it. 
> > > */
> > > +-  if (! exists
> > > +-      || (!S_ISLNK (s.st_mode)
> > > +-        && S_ISREG (s.st_mode)
> > > +-        && (s.st_mode & S_IWUSR)
> > > +-        && s.st_nlink == 1)
> > > +-      )
> > > +-    {
> > > +-      ret = rename (from, to);
> > > +-      if (ret == 0)
> > > +-      {
> > > +-        if (exists)
> > > +-          {
> > > +-            /* Try to preserve the permission bits and ownership
> > > of
> > > +-               TO.  First get the mode right except for the
> > > setuid
> > > +-               bit.  Then change the ownership.  Then fix the
> > > setuid
> > > +-               bit.  We do the chmod before the chown because if
> > > the
> > > +-               chown succeeds, and we are a normal user, we won't
> > > be
> > > +-               able to do the chmod afterward.  We don't bother
> > > to
> > > +-               fix the setuid bit first because that might
> > > introduce
> > > +-               a fleeting security problem, and because the chown
> > > +-               will clear the setuid bit anyhow.  We only fix the
> > > +-               setuid bit if the chown succeeds, because we don't
> > > +-               want to introduce an unexpected setuid file owned
> > > by
> > > +-               the user running objcopy.  */
> > > +-            chmod (to, s.st_mode & 0777);
> > > +-            if (chown (to, s.st_uid, s.st_gid) >= 0)
> > > +-              chmod (to, s.st_mode & 07777);
> > > +-          }
> > > +-      }
> > > +-      else
> > > +-      {
> > > +-        /* We have to clean up here.  */
> > > +-        non_fatal (_("unable to rename '%s'; reason: %s"), to,
> > > strerror (errno));
> > > +-        unlink (from);
> > > +-      }
> > > +-    }
> > > +-  else
> > > +-    {
> > > +-      ret = simple_copy (from, to);
> > > +-      if (ret != 0)
> > > +-      non_fatal (_("unable to copy file '%s'; reason: %s"), to,
> > > strerror (errno));
> > > ++    non_fatal (_("unable to copy file '%s'; reason: %s"),
> > > ++             to, strerror (errno));
> > > +
> > > +-      if (preserve_dates)
> > > +-      set_times (to, &s);
> > > +-      unlink (from);
> > > +-    }
> > > +-#endif /* _WIN32 && !__CYGWIN32__ */
> > > ++  if (target_stat != NULL)
> > > ++    set_times (to, target_stat);
> > > ++  unlink (from);
> > > +
> > > +   return ret;
> > > + }
> > > +--
> > > +2.17.1
> > > +
> > > --
> > > 2.31.1
> > > 
> > > 
> > > 
> > > 
> > 
> > 
> > 
> 
> 
> 


^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, other threads:[~2021-07-27 14:55 UTC | newest]

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-04-29 11:30 [hardknott][PATCH] Binutils: Fix CVE-2021-20197 Vinay Kumar
2021-05-05 11:32 ` Vinay Kumar
2021-05-05 13:21   ` [OE-core] " Richard Purdie
2021-05-05 14:03     ` Vinay Kumar
2021-05-06  1:57 ` Anuj Mittal
2021-05-06 12:25   ` Vinay Kumar
2021-05-06 12:49     ` Richard Purdie
2021-05-06 18:19       ` [hardknott][PATCH v2] binutils: " Vinay Kumar
2021-05-06 18:26       ` [OE-core] [hardknott][PATCH] Binutils: " Vinay Kumar
2021-05-07  1:00     ` Anuj Mittal
2021-05-07 14:48       ` Vinay Kumar
2021-06-29 13:25         ` Randy MacLeod
2021-07-02 11:22           ` [hardknott][PATCH v3] binutils: " Vinay Kumar
2021-07-13 13:46             ` [hardknott][PATCH v4] " Vinay Kumar
     [not found]             ` <16915DBBAEEB81DE.15464@lists.openembedded.org>
2021-07-13 13:49               ` [OE-core] " Vinay Kumar
     [not found]               ` <16915DE420F8F12E.31724@lists.openembedded.org>
2021-07-26  4:38                 ` Vinay Kumar
2021-07-27 14:54                   ` Anuj Mittal
2021-07-02 11:39           ` [OE-core] [hardknott][PATCH] Binutils: " Vinay Kumar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.