From ce8901890298bc1ddd482b789193ff1ec8eaa30a Mon Sep 17 00:00:00 2001 From: Michael Eischer Date: Sun, 3 Jul 2022 14:47:53 +0200 Subject: [PATCH] Fix data race in blob_saver After the `BlobSaver` job is submitted, the buffer can be released and reused by another `FileSaver` even before `BlobSaver.Save` returns. That FileSaver will the change `buf.Data` leading to wrong backup statistics. Found by `go test -race ./...`: WARNING: DATA RACE Write at 0x00c0000784a0 by goroutine 41: github.com/restic/restic/internal/archiver.(*FileSaver).saveFile() /home/michael/Projekte/restic/restic/internal/archiver/file_saver.go:176 +0x789 github.com/restic/restic/internal/archiver.(*FileSaver).worker() /home/michael/Projekte/restic/restic/internal/archiver/file_saver.go:242 +0x2af github.com/restic/restic/internal/archiver.NewFileSaver.func2() /home/michael/Projekte/restic/restic/internal/archiver/file_saver.go:88 +0x5d golang.org/x/sync/errgroup.(*Group).Go.func1() /home/michael/go/pkg/mod/golang.org/x/sync@v0.0.0-20210220032951-036812b2e83c/errgroup/errgroup.go:57 +0x91 Previous read at 0x00c0000784a0 by goroutine 29: github.com/restic/restic/internal/archiver.(*BlobSaver).Save() /home/michael/Projekte/restic/restic/internal/archiver/blob_saver.go:57 +0x1dd github.com/restic/restic/internal/archiver.(*BlobSaver).Save-fm() :1 +0xac github.com/restic/restic/internal/archiver.(*FileSaver).saveFile() /home/michael/Projekte/restic/restic/internal/archiver/file_saver.go:191 +0x855 github.com/restic/restic/internal/archiver.(*FileSaver).worker() /home/michael/Projekte/restic/restic/internal/archiver/file_saver.go:242 +0x2af github.com/restic/restic/internal/archiver.NewFileSaver.func2() /home/michael/Projekte/restic/restic/internal/archiver/file_saver.go:88 +0x5d golang.org/x/sync/errgroup.(*Group).Go.func1() /home/michael/go/pkg/mod/golang.org/x/sync@v0.0.0-20210220032951-036812b2e83c/errgroup/errgroup.go:57 +0x91 --- internal/archiver/blob_saver.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/archiver/blob_saver.go b/internal/archiver/blob_saver.go index eca158c20..0e6959524 100644 --- a/internal/archiver/blob_saver.go +++ b/internal/archiver/blob_saver.go @@ -45,6 +45,8 @@ func (s *BlobSaver) TriggerShutdown() { // Save stores a blob in the repo. It checks the index and the known blobs // before saving anything. It takes ownership of the buffer passed in. func (s *BlobSaver) Save(ctx context.Context, t restic.BlobType, buf *Buffer) FutureBlob { + // buf might be freed once the job was submitted, thus calculate the length now + length := len(buf.Data) ch := make(chan saveBlobResponse, 1) select { case s.ch <- saveBlobJob{BlobType: t, buf: buf, ch: ch}: @@ -54,7 +56,7 @@ func (s *BlobSaver) Save(ctx context.Context, t restic.BlobType, buf *Buffer) Fu return FutureBlob{ch: ch} } - return FutureBlob{ch: ch, length: len(buf.Data)} + return FutureBlob{ch: ch, length: length} } // FutureBlob is returned by SaveBlob and will return the data once it has been processed.