Обновить patches/0001-sparc_attrs.c-Prevent-buffer-overflow-in-sparc_check.patch

Message ID 20241105224803.20847-1-ant.v.moryakov@gmail.com
State Rejected
Headers
Series Обновить patches/0001-sparc_attrs.c-Prevent-buffer-overflow-in-sparc_check.patch |

Commit Message

Anton Moryakov Nov. 5, 2024, 10:48 p.m. UTC
  From: Моряков, Антон <avmoryakov@noreply.localhost>

---
 elfutils/backends/sparc_attrs.c                           | 5 ++++-
 ...c_attrs.c-Prevent-buffer-overflow-in-sparc_check.patch | 8 ++++----
 2 files changed, 8 insertions(+), 5 deletions(-)
  

Comments

Mark Wielaard Nov. 6, 2024, 7:32 p.m. UTC | #1
Hi Anton,

> +#define NAME_MAX_SIZE (32 * 17 + 32 + 1)
> +static_assert(NAME_MAX_SIZE == (32 * 17 + 32 + 1), "Buffer size for 'name' is insufficient");

This is indeed a static_assert. But doesn't really add any value. It
just checks the calculation one line above. But doesn't explain or
check why that calculation is "correct".

Is there a way to better work together.  We have seen various RASU JSC
"fixes" but it isn't really clear what those are or why you want to
fix these issues.

What exactly is RASU JSC?
Why do you believe the issues it finds should be "fixed"?

Reviewing these patches costs real time and energy.
So it would be good to understand what the goal of these patches is.
It feels a lot of the suggested fixes introduce new (other) issues.

Thanks,

Mark
  

Patch

diff --git a/elfutils/backends/sparc_attrs.c b/elfutils/backends/sparc_attrs.c
index 974e8fb..a50b4f7 100644
--- a/elfutils/backends/sparc_attrs.c
+++ b/elfutils/backends/sparc_attrs.c
@@ -36,6 +36,9 @@ 
 #define BACKEND sparc_
 #include "libebl_CPU.h"
 
+#define NAME_MAX_SIZE (32 * 17 + 32 + 1)
+static_assert(NAME_MAX_SIZE == (32 * 17 + 32 + 1), "Buffer size for 'name' is insufficient");
+
 bool
 sparc_check_object_attribute (Ebl *ebl __attribute__ ((unused)),
 			      const char *vendor, int tag, uint64_t value,
@@ -63,7 +66,7 @@  sparc_check_object_attribute (Ebl *ebl __attribute__ ((unused)),
   /* NAME should be big enough to hold any possible comma-separated
      list (no repetitions allowed) of attribute names from one of the
      arrays above.  */
-  static char name[32*17+32+1];
+  static char name[NAME_MAX_SIZE];
   name[0] = '\0';
 
   if (!strcmp (vendor, "gnu"))
diff --git a/patches/0001-sparc_attrs.c-Prevent-buffer-overflow-in-sparc_check.patch b/patches/0001-sparc_attrs.c-Prevent-buffer-overflow-in-sparc_check.patch
index cb6a0c6..894ab4f 100644
--- a/patches/0001-sparc_attrs.c-Prevent-buffer-overflow-in-sparc_check.patch
+++ b/patches/0001-sparc_attrs.c-Prevent-buffer-overflow-in-sparc_check.patch
@@ -14,13 +14,13 @@  Found by RASU JSC.
 
 Signed-off-by: Anton Moryakov <ant.v.moryakov@gmail.com>
 ---
- elfutils/backends/sparc_attrs.c | 11 ++++++++---
+backends/sparc_attrs.c | 11 ++++++++---
  1 file changed, 8 insertions(+), 3 deletions(-)
 
-diff --git a/elfutils/backends/sparc_attrs.c b/elfutils/backends/sparc_attrs.c
+diff --git a/backends/sparc_attrs.c b/backends/sparc_attrs.c
 index 974e8fb..0ba17c8 100644
---- a/elfutils/backends/sparc_attrs.c
-+++ b/elfutils/backends/sparc_attrs.c
+--- a/backends/sparc_attrs.c
++++ b/backends/sparc_attrs.c
 @@ -87,12 +87,17 @@ sparc_check_object_attribute (Ebl *ebl __attribute__ ((unused)),
              }