https://bugs.kde.org/show_bug.cgi?id=359871

Steven Smith <so...@archy.org.uk> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #97682|0                           |1
        is obsolete|                            |

--- Comment #17 from Steven Smith <so...@archy.org.uk> ---
Created attachment 97688
  --> https://bugs.kde.org/attachment.cgi?id=97688&action=edit
Forth attempt at a Linux fix. I think this one is actually correct.

My main concern was that, because we drop the global lock while making the
syscall, we might be racing with some other guest thread calling mmap/munmap,
so the system call might happen even if ML_(safe_to_deref) says no, and if
we've skipped the sanitisation the bug I was trying to fix will come back.
Similarly, if POST calls ML_(safe_to_deref) to figure out whether to VG_(free)
things then it might either free something it shouldn't or allow the memory to
leak. Force-failing guest calls where we can't do the sanitisation seemed like
the easiest way of avoiding the issues, except that I still need some way for
the PRE call to tell the POST one what/whether it needs to VG_(free), which
brought me to the assertion failures in my last attempt.

But I suppose it's possible to make syscalls fail without using
SET_STATUS_Failure, so that's what this variant does. I think it covers all the
relevant cases, or at least all of the ones I've thought of, even if it is a
bit more dependent on kernel implementation details than I would have liked.

Perhaps I was just overthinking things and the right answer would have been to
just ignore the races? The guest would have to be doing something pretty crazy
for them to matter, after all. On the other hand, I think this one'll work, and
it'd be nice to get the edge cases right, even if it is perhaps a little
pedantic.

This patch also includes some more bits in scalar.c, but I'm not convinced they
represent a sensible way of testing this:

1) I can reproduce the issue with --tool=none, so it clearly isn't a problem
with memcheck, and it seems a bit odd to put the test case there.
2) The scalar.c regtest fails on my machine even without any of my changes,
seemingly because it only works in 32 bit more and I have a 64 bit system, and
it doesn't seem to like the debug symbols on the libc in my cross-compile
setup. That makes it tricky for me to do any sensible testing of the test case.

I defer to your judgement on whether this ends up being a net positive for the
codebase.

-- 
You are receiving this mail because:
You are watching all bug changes.

Reply via email to