Просмотр исходного кода

daemon/logger: fix data race in LogFile

The log message's timestamp was being read after it was returned to the
pool. By coincidence the timestamp field happened to not be zeroed on
reset so much of the time things would work as expected. But if the
message value was to be taken back out of the pool before WriteLogEntry
returned, the timestamp recorded in the gzip header of compressed
rotated log files would be incorrect.

Make future use-after-put bugs fail fast by zeroing all fields of the
Message value, including the timestamp, when it is put into the pool.

Signed-off-by: Cory Snider <csnider@mirantis.com>
Cory Snider 3 лет назад
Родитель
Сommit
90c54320c8
3 измененных файлов с 4 добавлено и 12 удалено
  1. 0 2
      api/types/backend/backend.go
  2. 1 9
      daemon/logger/logger.go
  3. 3 1
      daemon/logger/loggerutils/logfile.go

+ 0 - 2
api/types/backend/backend.go

@@ -38,8 +38,6 @@ type PartialLogMetaData struct {
 // LogMessage is datastructure that represents piece of output produced by some
 // container.  The Line member is a slice of an array whose contents can be
 // changed after a log driver's Log() method returns.
-// changes to this struct need to be reflect in the reset method in
-// daemon/logger/logger.go
 type LogMessage struct {
 	Line         []byte
 	Source       string

+ 1 - 9
daemon/logger/logger.go

@@ -49,20 +49,12 @@ func PutMessage(msg *Message) {
 // Message is subtyped from backend.LogMessage because there is a lot of
 // internal complexity around the Message type that should not be exposed
 // to any package not explicitly importing the logger type.
-//
-// Any changes made to this struct must also be updated in the `reset` function
 type Message backend.LogMessage
 
 // reset sets the message back to default values
 // This is used when putting a message back into the message pool.
-// Any changes to the `Message` struct should be reflected here.
 func (m *Message) reset() {
-	m.Line = m.Line[:0]
-	m.Source = ""
-	m.Attrs = nil
-	m.PLogMetaData = nil
-
-	m.Err = nil
+	*m = Message{Line: m.Line[:0]}
 }
 
 // AsLogMessage returns a pointer to the message as a pointer to

+ 3 - 1
daemon/logger/loggerutils/logfile.go

@@ -156,7 +156,9 @@ func (w *LogFile) WriteLogEntry(msg *logger.Message) error {
 		return errors.Wrap(err, "error marshalling log message")
 	}
 
+	ts := msg.Timestamp
 	logger.PutMessage(msg)
+	msg = nil // Turn use-after-put bugs into panics.
 
 	w.mu.Lock()
 	if w.closed {
@@ -172,7 +174,7 @@ func (w *LogFile) WriteLogEntry(msg *logger.Message) error {
 	n, err := w.f.Write(b)
 	if err == nil {
 		w.currentSize += int64(n)
-		w.lastTimestamp = msg.Timestamp
+		w.lastTimestamp = ts
 	}
 
 	w.mu.Unlock()