diff mbox series

b4 patch corruption

Message ID c9643b70-8751-a2a2-185e-174ec2560328@citrix.com (mailing list archive)
State Not Applicable, archived
Headers show
Series b4 patch corruption | expand

Commit Message

Andrew Cooper Aug. 17, 2023, 12:23 p.m. UTC
Hi,

I'm using b4 v0.12.3, and getting some kind of patch corruption issue:

xen.git$ b4 shazam a67c2fa3-ba1c-3783-c3ee-250aff6903d5@suse.com
Grabbing thread from
lore.kernel.org/all/a67c2fa3-ba1c-3783-c3ee-250aff6903d5@suse.com/t.mbox.gz
Checking for newer revisions
Grabbing search results from lore.kernel.org
Analyzing 1 messages in the thread
Checking attestation on all messages, may take a moment...
---
  ✓ [PATCH] x86emul: rework wrapping of libc functions in test and
fuzzing harnesses
  ---
  ✓ Signed: DKIM/suse.com
---
Total patches: 1
---
Applying: x86emul: rework wrapping of libc functions in test and fuzzing
harnesses
Patch failed at 0001 x86emul: rework wrapping of libc functions in test
and fuzzing harnesses
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".
error: corrupt patch at line 11
hint: Use 'git am --show-current-patch=diff' to see the failed patch

Forwarding this to patch does produce a more specific complaint:

xen.git$ git am --show-current-patch=diff | patch
patching file Makefile
patch: **** malformed patch at line 11: @@ -51,10 +53,10 @@
x86-insn-fuzzer.a: $(OBJS) cpuid.o

but it's not clear what exactly is wrong here.

If I copy the patch out of the email and feed it to `git am` directly,
it's happy:

xen.git$ git am rework-wrap.patch
Applying: x86emul: rework wrapping of libc functions in test and fuzzing
harnesses

I can't spot any interesting differences between what failed and what
`git format-patch` reproduced, but I'm out of my depth here.

My gut feeling is that something has been mangled while b4 was
processing the change, but it's only a guess.

I've attached the copied-off-list patch, and the base commit is
d0eabe3eaf0db5b78843095a2918d50961e99e96

Thanks,

~Andrew

Comments

Konstantin Ryabitsev Aug. 31, 2023, 8:26 p.m. UTC | #1
On Thu, Aug 17, 2023 at 01:23:06PM +0100, Andrew Cooper wrote:
> Hi,
> 
> I'm using b4 v0.12.3, and getting some kind of patch corruption issue:

Indeed, I can confirm the problem.

bugbot assign to me

> I can't spot any interesting differences between what failed and what
> `git format-patch` reproduced, but I'm out of my depth here.
> 
> My gut feeling is that something has been mangled while b4 was
> processing the change, but it's only a guess.

I'll have to dig deeper, but the main reason is that it's not a patch
generated with git (you can tell by the lack of diff --git and index lines).
However, b4 shouldn't corrupt such patches either, so I will have to figure
out what is throwing it off.

Thanks for the report.

-K
Kernel.org Bugbot Aug. 31, 2023, 8:32 p.m. UTC | #2
Hello:

This conversation is now tracked by Kernel.org Bugzilla:
https://bugzilla.kernel.org/show_bug.cgi?id=217852

There is no need to do anything else, just keep talking.
Kernel.org Bugbot Sept. 6, 2023, 9:09 p.m. UTC | #3
Konstantin Ryabitsev writes in commit 0438c6a107d3475895281146e51d68069a41dc3d:

am: properly handle non-git diffs

We were not properly parsing patches generated by something other than
"git diff" or "git format-patch". Notably, these are pretty rare, but we
still shouldn't fail when doing that.

This additionally adds a few tests and fixes a silly mistake that I had
all along in the sample data.

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=217852
Signed-off-by: Konstantin Ryabitsev <konstantin@linuxfoundation.org>

(via https://git.kernel.org/pub/scm/utils/b4/b4.git/commit/?id=0438c6a107d3)
diff mbox series

Patch

From: Jan Beulich <jbeulich@suse.com>
Subject: [PATCH] x86emul: rework wrapping of libc functions in test and fuzzing harnesses

Our present approach is working fully behind the compiler's back. This
was found to not work with LTO. Employ ld's --wrap= option instead. Note
that while this makes the build work at least with new enough gcc (it
doesn't with gcc7, for example, due to tool chain side issues afaict),
according to my testing things still won't work when building the
fuzzing harness with afl-cc: While with the gcc7 tool chain I see afl-as
getting invoked, this does not happen with gcc13. Yet without using that
assembler wrapper the resulting binary will look uninstrumented to
afl-fuzz.

While checking the resulting binaries I noticed that we've gained uses
of snprintf() and strstr(), which only just so happen to not cause any
problems. Add a wrappers for them as well.

Since we don't have any actual uses of v{,sn}printf(), no definitions of
their wrappers appear (just yet). But I think we want
__wrap_{,sn}printf() to properly use __real_v{,sn}printf() right away,
which means we need delarations of the latter.

Reported-by: Andrew Cooper <andrew.cooper3@citrix.com>
Suggested-by: Andrew Cooper <andrew.cooper3@citrix.com>
Signed-off-by: Jan Beulich <jbeulich@suse.com>

--- a/tools/fuzz/x86_instruction_emulator/Makefile
+++ b/tools/fuzz/x86_instruction_emulator/Makefile
@@ -35,6 +35,8 @@  OBJS := fuzz-emul.o x86-emulate.o
 OBJS += x86_emulate/0f01.o x86_emulate/0fae.o x86_emulate/0fc7.o
 OBJS += x86_emulate/decode.o x86_emulate/fpu.o
 
+WRAPPED = $(shell sed -n 's,^ *WRAP(\([[:alnum:]_]*\));,\1,p' x86-emulate.h)
+
 private.h := x86-emulate.h x86_emulate/x86_emulate.h x86_emulate/private.h
 
 x86-emulate.h: x86_emulate/x86_emulate.h
@@ -51,10 +53,10 @@  x86-insn-fuzzer.a: $(OBJS) cpuid.o
 	$(AR) rc $@ $^
 
 afl-harness: afl-harness.o $(OBJS) cpuid.o wrappers.o
-	$(CC) $(CFLAGS) $^ -o $@
+	$(CC) $(CFLAGS) $(addprefix -Wl$(comma)--wrap=,$(WRAPPED)) $^ -o $@
 
 afl-harness-cov: afl-harness-cov.o $(patsubst %.o,%-cov.o,$(OBJS)) cpuid.o wrappers.o
-	$(CC) $(CFLAGS) $(GCOV_FLAGS) $^ -o $@
+	$(CC) $(CFLAGS) $(GCOV_FLAGS) $(addprefix -Wl$(comma)--wrap=,$(WRAPPED)) $^ -o $@
 
 # Common targets
 .PHONY: all
--- a/tools/tests/x86_emulator/Makefile
+++ b/tools/tests/x86_emulator/Makefile
@@ -259,8 +259,10 @@  OBJS := x86-emulate.o cpuid.o test_x86_e
 OBJS += x86_emulate/0f01.o x86_emulate/0fae.o x86_emulate/0fc7.o
 OBJS += x86_emulate/blk.o x86_emulate/decode.o x86_emulate/fpu.o x86_emulate/util.o
 
+WRAPPED := $(shell sed -n 's,^ *WRAP(\([[:alnum:]_]*\));,\1,p' x86-emulate.h)
+
 $(TARGET): $(OBJS)
-	$(HOSTCC) $(HOSTCFLAGS) -o $@ $^
+	$(HOSTCC) $(HOSTCFLAGS) $(addprefix -Wl$(comma)--wrap=,$(WRAPPED)) -o $@ $^
 
 .PHONY: clean
 clean:
--- a/tools/tests/x86_emulator/wrappers.c
+++ b/tools/tests/x86_emulator/wrappers.c
@@ -1,78 +1,103 @@ 
 #include <stdarg.h>
 
-#define WRAP(x) typeof(x) emul_##x
+#define WRAP(x) typeof(x) __wrap_ ## x, __real_ ## x
 #include "x86-emulate.h"
 
-size_t emul_fwrite(const void *src, size_t sz, size_t n, FILE *f)
+size_t __wrap_fwrite(const void *src, size_t sz, size_t n, FILE *f)
 {
     emul_save_fpu_state();
-    sz = fwrite(src, sz, n, f);
+    sz = __real_fwrite(src, sz, n, f);
     emul_restore_fpu_state();
 
     return sz;
 }
 
-int emul_memcmp(const void *p1, const void *p2, size_t sz)
+int __wrap_memcmp(const void *p1, const void *p2, size_t sz)
 {
     int rc;
 
     emul_save_fpu_state();
-    rc = memcmp(p1, p2, sz);
+    rc = __real_memcmp(p1, p2, sz);
     emul_restore_fpu_state();
 
     return rc;
 }
 
-void *emul_memcpy(void *dst, const void *src, size_t sz)
+void *__wrap_memcpy(void *dst, const void *src, size_t sz)
 {
     emul_save_fpu_state();
-    memcpy(dst, src, sz);
+    __real_memcpy(dst, src, sz);
     emul_restore_fpu_state();
 
     return dst;
 }
 
-void *emul_memset(void *dst, int c, size_t sz)
+void *__wrap_memset(void *dst, int c, size_t sz)
 {
     emul_save_fpu_state();
-    memset(dst, c, sz);
+    __real_memset(dst, c, sz);
     emul_restore_fpu_state();
 
     return dst;
 }
 
-int emul_printf(const char *fmt, ...)
+int __wrap_printf(const char *fmt, ...)
 {
     va_list varg;
     int rc;
 
     emul_save_fpu_state();
     va_start(varg, fmt);
-    rc = vprintf(fmt, varg);
+    rc = __real_vprintf(fmt, varg);
     va_end(varg);
     emul_restore_fpu_state();
 
     return rc;
 }
 
-int emul_putchar(int c)
+int __wrap_putchar(int c)
 {
     int rc;
 
     emul_save_fpu_state();
-    rc = putchar(c);
+    rc = __real_putchar(c);
     emul_restore_fpu_state();
 
     return rc;
 }
 
-int emul_puts(const char *str)
+int __wrap_puts(const char *str)
 {
     int rc;
 
     emul_save_fpu_state();
-    rc = puts(str);
+    rc = __real_puts(str);
     emul_restore_fpu_state();
 
     return rc;
 }
+
+int __wrap_snprintf(char *buf, size_t n, const char *fmt, ...)
+{
+    va_list varg;
+    int rc;
+
+    emul_save_fpu_state();
+    va_start(varg, fmt);
+    rc = __real_vsnprintf(buf, n, fmt, varg);
+    va_end(varg);
+    emul_restore_fpu_state();
+
+    return rc;
+}
+
+char *__wrap_strstr(const char *s1, const char *s2)
+{
+    char *s;
+
+    emul_save_fpu_state();
+    s = __real_strstr(s1, s2);
+    emul_restore_fpu_state();
+
+    return s;
+}
--- a/tools/tests/x86_emulator/x86-emulate.h
+++ b/tools/tests/x86_emulator/x86-emulate.h
@@ -32,9 +32,7 @@ 
 #ifdef EOF
 # error "Must not include <stdio.h> before x86-emulate.h"
 #endif
-#ifdef WRAP
-# include <stdio.h>
-#endif
+#include <stdio.h>
 
 #include <xen/xen.h>
 
@@ -88,11 +86,7 @@  struct x86_fxsr *get_fpu_save_area(void)
  * around the actual function.
  */
 #ifndef WRAP
-# if 0 /* This only works for explicit calls, not for compiler generated ones. */
-#  define WRAP(x) typeof(x) x asm("emul_" #x)
-# else
-# define WRAP(x) asm(".equ " #x ", emul_" #x)
-# endif
+# define WRAP(x) typeof(x) __wrap_ ## x
 #endif
 
 WRAP(fwrite);
@@ -102,6 +96,10 @@  WRAP(memset);
 WRAP(printf);
 WRAP(putchar);
 WRAP(puts);
+WRAP(snprintf);
+WRAP(strstr);
+WRAP(vprintf);
+WRAP(vsnprintf);
 
 #undef WRAP