Summary: | app-antivirus/clamav-0.99.2-r1: check1_clamscan.sh fails with SIGBUS | ||
---|---|---|---|
Product: | Gentoo Linux | Reporter: | Rolf Eike Beer <eike> |
Component: | Current packages | Assignee: | Antivirus Team <antivirus> |
Status: | RESOLVED FIXED | ||
Severity: | normal | CC: | matoro_gentoo, mjo, sam, sparc |
Priority: | Normal | Keywords: | PATCH, PullRequest, TESTFAILURE |
Version: | unspecified | ||
Hardware: | Sparc64 | ||
OS: | Linux | ||
URL: | https://bugzilla.clamav.net/show_bug.cgi?id=12383 | ||
See Also: | https://github.com/gentoo/gentoo/pull/26754 | ||
Whiteboard: | |||
Package list: | Runtime testing required: | --- | |
Bug Depends on: | |||
Bug Blocks: | 371525, 619302 | ||
Attachments: |
build.log
test-suite.log build.log for 0.99.4 build.log for 0.100.0 patch |
Description
Rolf Eike Beer
![]() Created attachment 506716 [details]
build.log
Created attachment 506718 [details]
test-suite.log
please test this with current versions (preferrably 0.99.4 or 0.100.0) Created attachment 528416 [details]
build.log for 0.99.4
Error is still there.
Created attachment 528418 [details]
build.log for 0.100.0
Error not in log. Given the many test errors I doubt it's really "fixed".
I finally managed to get a backtrace. The problem the the unaligned cast to YR_RULE* as seen in the example and in the neighbor case. #0 0xf7ced8ac in yr_execute_code (aclsig=0xf6d389b8, acdata=0xffffa934, context=0xffffa6c8, timeout=0, start_time=0) at yara_exec.c:434 #1 0xf7ae482c in yara_eval (ctx=0xffffaec4, root=0xf7218f44, acdata=0xffffa934, target_info=0xffffa880, hash=0xffffac70 "+\366\310@;[\nl\315\317\307Ǥ4P|", lsid=0) at matcher.c:840 #2 0xf7ae49b0 in cli_exp_eval (ctx=0xffffaec4, root=0xf7218f44, acdata=0xffffa934, target_info=0xffffa880, hash=0xffffac70 "+\366\310@;[\nl\315\317\307Ǥ4P|") at matcher.c:864 #3 0xf7ae5fbc in cli_fmap_scandesc (ctx=0xffffaec4, ftype=CL_TYPE_MSEXE, ftonly=0 '\000', ftoffset=0xffffab14, acmode=3, acres=0x0, refhash=0xffffac70 "+\366\310@;[\nl\315\317\307Ǥ4P|") at matcher.c:1244 #4 0xf7b1701c in cli_scanraw (ctx=0xffffaec4, type=CL_TYPE_MSEXE, typercg=1 '\001', dettype=0xffffac30, refhash=0xffffac70 "+\366\310@;[\nl\315\317\307Ǥ4P|") at scanners.c:2598 #5 0xf7b1b3f0 in magic_scandesc (ctx=0xffffaec4, type=CL_TYPE_MSEXE) at scanners.c:3674 #6 0xf7b1be40 in cli_base_scandesc (desc=4, filepath=0x700c6480 "/var/tmp/portage/app-antivirus/clamav-0.101.0/work/clamav-0.101.0/unit_tests/../test/clam-aspack.exe", ctx=0xffffaec4, type=CL_TYPE_ANY) at scanners.c:3824 #7 0xf7b1bed4 in cli_magic_scandesc (desc=4, filepath=0x700c6480 "/var/tmp/portage/app-antivirus/clamav-0.101.0/work/clamav-0.101.0/unit_tests/../test/clam-aspack.exe", ctx=0xffffaec4) at scanners.c:3836 #8 0xf7b1cf4c in scan_common (desc=4, map=0x0, filepath=0x700c5f00 "/var/tmp/portage/app-antivirus/clamav-0.101.0/work/clamav-0.101.0/unit_tests/../test/clam-aspack.exe", virname=0xffffb058, scanned=0x70038ad8 <info+20>, engine=0x70052048, scanoptions=0xffffb244, context=0xffffb074) at scanners.c:4112 #9 0xf7b1d0b0 in cl_scandesc_callback (desc=4, filename=0x700c5f00 "/var/tmp/portage/app-antivirus/clamav-0.101.0/work/clamav-0.101.0/unit_tests/../test/clam-aspack.exe", virname=0xffffb058, scanned=0x70038ad8 <info+20>, engine=0x70052048, scanoptions=0xffffb244, context=0xffffb074) at scanners.c:4261 #10 0x70010678 in scanfile (filename=0x700c5f00 "/var/tmp/portage/app-antivirus/clamav-0.101.0/work/clamav-0.101.0/unit_tests/../test/clam-aspack.exe", engine=0x70052048, opts=0x7004a650, options=0xffffb244) at manager.c:392 #11 0x70014530 in scanmanager (opts=0x7004a650) at manager.c:1206 #12 0x7000e7bc in main (argc=52, argv=0xffffb8e4) at clamscan.c:161 (gdb) l 429 #endif 430 break; 431 432 case OP_MATCH_RULE: 433 pop(r1); 434 rule = *(YR_RULE**)(ip + 1); 435 ip += sizeof(uint64_t); 436 437 if (!IS_UNDEFINED(r1) && r1) 438 #if REAL_YARA This version is not in the tree anymore, so closing it. if the problem still persists for you please reopen it. (In reply to Thomas Raschbacher from comment #7) > This version is not in the tree anymore, so closing it. if the problem still > persists for you please reopen it. The absolute minimum you should do before closing a bug as OBSOLETE/UPSTREAM is at least a cursory glance at the source code to see if it's possible the problem has been fixed. Rolf gave you a back trace and a line number of the exact spot the problem occurs. There's a pretty high likelihood that the problem is there, since yara_exec.c:434 still contains the exact same line. Perhaps as the Gentoo maintainer you would be interested in reporting a bug upstream, or even attempting to fix the problem yourself? The surrounding code shows plenty of examples of how to do it. Actually, upstream commit c0525b368b4e48e547f3969967adf0b01e939aa6 already fixed most of the issues, but for whatever reason did not fix this one (or the one above in the OP_PUSH_RULE case) But seriously though: Bug opened nearly two years ago. Nothing from your side except a request for more testing (which the reporter did!) and then closing the bug, even when the reporter showed you exactly what the problem was. That's really bad. So we're back to just radio silence from the nominal maintainer? Reported upstream at $URL. I also took a wild-ass guess at a patch, based on commit c0525b368b4 (thanks Matt). It doesn't crash my machine, so that's nice. (In reply to Matt Turner from comment #9) > So we're back to just radio silence from the nominal maintainer? sorry i seem to have missed some bug updates (yet again busy RL) I closed the bug because that version was long out of the tree and I didn't have the time to look at the source code. -- and that'S also why I added to please re-open if it persists with newer versions, since I couldn't reproduce it on my machine @mjo: seems that bug you linked is not publically accessible (i think clamav marks all bugs reported "private" until someone confirms it is not a security issue. Yeah, I can't mark the bug as "public." There hasn't been any activity on it yet. The patch I suggested is below, although I should reiterate that I'm totally guessing. index 94a477259..25ae7f135 100644 --- a/libclamav/yara_exec.c +++ b/libclamav/yara_exec.c @@ -420,7 +420,7 @@ int yr_execute_code( break; case OP_PUSH_RULE: - rule = *(YR_RULE**)(ip + 1); + memcpy(&rule, ip + 1, sizeof(YR_RULE*)); ip += sizeof(uint64_t); #if REAL_YARA push(rule->t_flags[tidx] & RULE_TFLAGS_MATCH ? 1 : 0); @@ -431,7 +431,7 @@ int yr_execute_code( case OP_MATCH_RULE: pop(r1); - rule = *(YR_RULE**)(ip + 1); + memcpy(&rule, ip + 1, sizeof(YR_RULE*)); ip += sizeof(uint64_t); if (!IS_UNDEFINED(r1) && r1) Did anything happen with this? bug is still private When testing with 0.104.1 I at least do not see any SIGBUS anymore, only test errors. mjo@: can you ping the upstream bug for us? I've just tested the latest in-tree versions, and I don't see test errors on 0.104.2 or 0.104.3, at least on 64-bit. Can I get a retest with the most recent in-tree? All the tests pass for me on 64-bit. I'll test 32-bit userland after I update my chroot. I still see the same code in v0.104.3: case OP_PUSH_RULE: rule = *(YR_RULE**)(ip + 1); ... case OP_MATCH_RULE: pop(r1); rule = *(YR_RULE**)(ip + 1); Maybe Cc'ing mjo@ directly will garner a response. Tried 32-bit just now and tests do indeed fail there. But SIGBUS seems gone, so if we want to solely keep this targeting 32-bit test failures we should drop the block on 371525 and instead block https://bugs.gentoo.org/843998 There hasn't been any activity on bugzilla, but the upstream development model is based on CADT. They've decided to rewrite in rust so this will be the least of your problems on sparc. I dropped myself as a maintainer from anything newer than the 0.103.x series that uses autotools and C. I have a functioning rust toolchain on sparc, and I'm working with gyakolev on getting it published in https://bugs.gentoo.org/769467 (this is pending https://bugs.gentoo.org/842246 atm). So we can worry about that when the time comes. For the matter of this particular ticket, given that the issue directly addressed in the subject is gone, do you want to (a) close it as no longer present in in-tree versions or (b) change the ticket into tracking the test failures on 32-bit, and not consider it resolved until then? (In reply to matoro from comment #20) > > For the matter of this particular ticket, given that the issue directly > addressed in the subject is gone, do you want to (a) close it as no longer > present in in-tree versions or (b) change the ticket into tracking the test > failures on 32-bit, and not consider it resolved until then? Does this particular test still fail, but with something other than SIGBUS? Or are there other unrelated 32-bit sparc test failures? In the first case I would say we should tweak the description of this bug, but in the second, this should be closed and a new bug can be used to track the failures and block bug 843998. The code has undefined behavior. It's just luck that it doesn't cause a problem on 64-bit SPARC. I'll just write a patch since this is clearly not going to be fixed any other way. Created attachment 786614 [details, diff]
patch
You're right, the test no longer SIGBUSes, even without patching. Here is a patch to fix potential unaligned accesses, though it doesn't solve the test failures.
(In reply to Matt Turner from comment #23) > Created attachment 786614 [details, diff] [details, diff] > patch > > You're right, the test no longer SIGBUSes, even without patching. Here is a > patch to fix potential unaligned accesses, though it doesn't solve the test > failures. IMO you maximize your chances of getting this merged by submitting a PR to https://github.com/Cisco-Talos/clamav It'll live to be a hundred if I post it to bugzilla. (In reply to Matt Turner from comment #23) > Created attachment 786614 [details, diff] [details, diff] > patch > > You're right, the test no longer SIGBUSes, even without patching. Here is a > patch to fix potential unaligned accesses, though it doesn't solve the test > failures. Matt, what do you want to keep as the plan of action for this bug? I see four options: * Close as obsolete since the original alignment-related test failures are gone. * Backport your patch to correct the UB, even though it's no longer causing visible problems, then close as obsolete. * Mask as broken on 32-bit profiles only. Leave unmasked on 64-bit since all tests pass there. * Keep open until 32-bit test failures are resolved. This would probably be indefinitely. The bug has been closed via the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=440f860fb3b880c3c2a90f46297a4b72ff2c69e4 commit 440f860fb3b880c3c2a90f46297a4b72ff2c69e4 Author: matoro <matoro@users.noreply.github.com> AuthorDate: 2022-08-06 02:42:39 +0000 Commit: Sam James <sam@gentoo.org> CommitDate: 2022-08-15 14:53:40 +0000 profiles/arch/sparc/32ul: mask app-antivirus/clamav Test failures on 32-bit, passes on 64-bit. Closes: https://bugs.gentoo.org/638888 Closes: https://github.com/gentoo/gentoo/pull/26754 Signed-off-by: Sam James <sam@gentoo.org> profiles/arch/sparc/32ul/package.mask | 7 +++++++ profiles/arch/sparc/32ul/package.use.mask | 9 +++++++++ 2 files changed, 16 insertions(+) |