* David Ahern <dsah...@gmail.com> wrote:
> If the filesystem where a file is written using mmap fills perf record
> gets a SIGBUS and terminated. Handle the SIGBUS by using longjmp to
> bounce out of the memcpy and fail the write.
>
> Signed-off-by: David Ahern <dsah...@gmail.com>
> Cc: Ingo Molnar <mi...@kernel.org>
> Cc: Frederic Weisbecker <fweis...@gmail.com>
> Cc: Peter Zijlstra <pet...@infradead.org>
> Cc: Jiri Olsa <jo...@redhat.com>
> Cc: Namhyung Kim <namhy...@kernel.org>
> Cc: Mike Galbraith <efa...@gmx.de>
> Cc: Stephane Eranian <eran...@google.com>
> ---
> tools/perf/builtin-record.c | 22 ++++++++++++++++------
> 1 file changed, 16 insertions(+), 6 deletions(-)
>
> diff --git a/tools/perf/builtin-record.c b/tools/perf/builtin-record.c
> index 1a4fa5df215b..48d6535d144f 100644
> --- a/tools/perf/builtin-record.c
> +++ b/tools/perf/builtin-record.c
> @@ -29,9 +29,11 @@
> #include <unistd.h>
> #include <sched.h>
> #include <sys/mman.h>
> +#include <setjmp.h>
>
> /* output file mmap'ed N chunks at a time */
> #define MMAP_OUTPUT_SIZE (64*1024*1024)
> +sigjmp_buf mmap_jmp;
>
> #ifndef HAVE_ON_EXIT_SUPPORT
> #ifndef ATEXIT_MAX
> @@ -141,6 +143,7 @@ static int do_mmap_output(struct perf_record *rec, void
> *buf, size_t size)
> {
> u64 remaining;
> off_t offset;
> + volatile size_t total_len = 0;
>
> if (rec->mmap.addr == NULL) {
> next_segment:
> @@ -157,20 +160,23 @@ next_segment:
> * space write what we can then go back and create the
> * next segment
> */
> - if (size > remaining) {
> - memcpy(rec->mmap.addr + rec->mmap.offset, buf, remaining);
> + if (setjmp(mmap_jmp) != 0) {
> + pr_err("mmap copy failed.\n");
> + return -1;
> + }
> + if (size-total_len > remaining) {
> + memcpy(rec->mmap.addr + rec->mmap.offset, buf+total_len,
> remaining);
> rec->bytes_written += remaining;
>
> - size -= remaining;
> - buf += remaining;
> + total_len += remaining;
>
> munmap(rec->mmap.addr, rec->mmap.out_size);
> goto next_segment;
> }
>
> /* more data to copy and it fits in the current segment */
> - if (size) {
> - memcpy(rec->mmap.addr + rec->mmap.offset, buf, size);
> + if (size - total_len) {
> + memcpy(rec->mmap.addr + rec->mmap.offset, buf+total_len,
> size-total_len);
> rec->bytes_written += size;
> rec->mmap.offset += size;
> }
> @@ -272,6 +278,9 @@ static void sig_handler(int sig)
> if (sig == SIGCHLD)
> child_finished = 1;
>
> + if (sig == SIGBUS)
> + longjmp(mmap_jmp, 1);
So this isn't very robust, because it assumes that all sources of SIGBUS
are due to that memcpy() hitting -ENOSPC...
There are several failure modes:
- If mmap_jmp is not set yet and we get a SIGBUS is some other place,
then the longjmp() result will be undefined.
- If mmap_jmp environment is set, but we've returned from
do_mmap_output() already, then the result will be undefined - likely a
non-obvious crash.
So at minimum we need a flag that tells us whether the jump environment is
valid or not - i.e. whether we are executing inside the protected region
or not - and only do the longjmp() if that flag is set.
Is there really no other way to handle the -ENOSPC case robustly? I guess
not because the memcpy() really needs memory to write to, but I thought
I'd ask ...
Thanks,
Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
