From 3cb7734eac78b562f9ef1a572f586e5e5198c45c Mon Sep 17 00:00:00 2001
From: Nick Craig-Wood <nick@craig-wood.com>
Date: Sat, 13 Aug 2022 11:59:42 +0100
Subject: [PATCH] config: move locking to fix fatal error: concurrent map read
 and map write

Before this change we assumed that github.com/Unknwon/goconfig was
threadsafe as documented.

However it turns out it is not threadsafe and looking at the code it
appears that making it threadsafe might be quite hard.

So this change increases the lock coverage in configfile to cover the
goconfig uses also.

Fixes #6378
---
 fs/config/configfile/configfile.go | 49 ++++++++++++++++++++++--------
 1 file changed, 36 insertions(+), 13 deletions(-)

diff --git a/fs/config/configfile/configfile.go b/fs/config/configfile/configfile.go
index 572ac15e1..c0bef0389 100644
--- a/fs/config/configfile/configfile.go
+++ b/fs/config/configfile/configfile.go
@@ -24,16 +24,15 @@ func Install() {
 // Storage implements config.Storage for saving and loading config
 // data in a simple INI based file.
 type Storage struct {
-	gc *goconfig.ConfigFile // config file loaded - thread safe
 	mu sync.Mutex           // to protect the following variables
+	gc *goconfig.ConfigFile // config file loaded - not thread safe
 	fi os.FileInfo          // stat of the file when last loaded
 }
 
 // Check to see if we need to reload the config
-func (s *Storage) check() {
-	s.mu.Lock()
-	defer s.mu.Unlock()
-
+//
+// mu must be held when calling this
+func (s *Storage) _check() {
 	if configPath := config.GetConfigPath(); configPath != "" {
 		// Check to see if config file has changed since it was last loaded
 		fi, err := os.Stat(configPath)
@@ -174,7 +173,10 @@ func (s *Storage) Save() error {
 
 // Serialize the config into a string
 func (s *Storage) Serialize() (string, error) {
-	s.check()
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	s._check()
 	var buf bytes.Buffer
 	if err := goconfig.SaveConfigData(s.gc, &buf); err != nil {
 		return "", fmt.Errorf("failed to save config file: %w", err)
@@ -185,7 +187,10 @@ func (s *Storage) Serialize() (string, error) {
 
 // HasSection returns true if section exists in the config file
 func (s *Storage) HasSection(section string) bool {
-	s.check()
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	s._check()
 	_, err := s.gc.GetSection(section)
 	return err == nil
 }
@@ -193,26 +198,38 @@ func (s *Storage) HasSection(section string) bool {
 // DeleteSection removes the named section and all config from the
 // config file
 func (s *Storage) DeleteSection(section string) {
-	s.check()
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	s._check()
 	s.gc.DeleteSection(section)
 }
 
 // GetSectionList returns a slice of strings with names for all the
 // sections
 func (s *Storage) GetSectionList() []string {
-	s.check()
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	s._check()
 	return s.gc.GetSectionList()
 }
 
 // GetKeyList returns the keys in this section
 func (s *Storage) GetKeyList(section string) []string {
-	s.check()
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	s._check()
 	return s.gc.GetKeyList(section)
 }
 
 // GetValue returns the key in section with a found flag
 func (s *Storage) GetValue(section string, key string) (value string, found bool) {
-	s.check()
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	s._check()
 	value, err := s.gc.GetValue(section, key)
 	if err != nil {
 		return "", false
@@ -222,7 +239,10 @@ func (s *Storage) GetValue(section string, key string) (value string, found bool
 
 // SetValue sets the value under key in section
 func (s *Storage) SetValue(section string, key string, value string) {
-	s.check()
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	s._check()
 	if strings.HasPrefix(section, ":") {
 		fs.Logf(nil, "Can't save config %q for on the fly backend %q", key, section)
 		return
@@ -232,7 +252,10 @@ func (s *Storage) SetValue(section string, key string, value string) {
 
 // DeleteKey removes the key under section
 func (s *Storage) DeleteKey(section string, key string) bool {
-	s.check()
+	s.mu.Lock()
+	defer s.mu.Unlock()
+
+	s._check()
 	return s.gc.DeleteKey(section, key)
 }