From 196049a040fa75d8bea80191f7fb5fa4e473d081 Mon Sep 17 00:00:00 2001 From: Daniel Date: Tue, 26 Jan 2021 14:15:07 +0100 Subject: [PATCH 1/2] Check binary path of PID lock --- cmds/portmaster-start/lock.go | 38 ++++++++++++++++++++++++++++++----- cmds/portmaster-start/run.go | 5 ++++- 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/cmds/portmaster-start/lock.go b/cmds/portmaster-start/lock.go index 7d7af1bc..14182069 100644 --- a/cmds/portmaster-start/lock.go +++ b/cmds/portmaster-start/lock.go @@ -32,14 +32,42 @@ func checkAndCreateInstanceLock(name string) (pid int32, err error) { return 0, createInstanceLock(lockFilePath) } - // check if process exists + // Check if process exists. p, err := processInfo.NewProcess(int32(parsedPid)) - if err == nil { - return p.Pid, nil + if err != nil { + // A process with the locked PID does not exist. + // This is expected, so we can continue normally. + return 0, createInstanceLock(lockFilePath) } - // else create new lock - return 0, createInstanceLock(lockFilePath) + // Get the process paths and evaluate and clean them. + executingBinaryPath, err := p.Exe() + if err != nil { + return 0, fmt.Errorf("failed to get path of existing process: %w", err) + } + cleanedExecutingBinaryPath, err := filepath.EvalSymlinks(executingBinaryPath) + if err != nil { + return 0, fmt.Errorf("failed to evaluate path of existing process: %w", err) + } + ownBinaryPath, err := os.Executable() + if err != nil { + return 0, fmt.Errorf("failed to get path of own process: %w", err) + } + cleanedOwnBinaryPath, err := filepath.EvalSymlinks(ownBinaryPath) + if err != nil { + return 0, fmt.Errorf("failed to evaluate path of own process: %w", err) + } + + // Check if the binary path matches. + if cleanedExecutingBinaryPath != cleanedOwnBinaryPath { + // The process with the locked PID belongs to another binary. + // As the Portmaster usually starts very early, it will have a low PID, + // which could be assigned to another process on next boot. + return 0, createInstanceLock(lockFilePath) + } + + // Return PID of already running instance. + return p.Pid, nil } func createInstanceLock(lockFilePath string) error { diff --git a/cmds/portmaster-start/run.go b/cmds/portmaster-start/run.go index 98b0f766..81cd2fe5 100644 --- a/cmds/portmaster-start/run.go +++ b/cmds/portmaster-start/run.go @@ -130,7 +130,10 @@ func run(opts *Options, cmdArgs []string) (err error) { // check for duplicate instances if opts.ShortIdentifier == "core" || opts.ShortIdentifier == "hub" { - pid, _ := checkAndCreateInstanceLock(opts.ShortIdentifier) + pid, err := checkAndCreateInstanceLock(opts.ShortIdentifier) + if err != nil { + return fmt.Errorf("failed to exec lock: %w", err) + } if pid != 0 { return fmt.Errorf("another instance of %s is already running: PID %d", opts.Name, pid) } From 6a1c28249197b8468655dd2bfa8a512b2a41c8f2 Mon Sep 17 00:00:00 2001 From: Daniel Date: Thu, 28 Jan 2021 16:18:42 +0100 Subject: [PATCH 2/2] Implement review suggestion --- cmds/portmaster-start/lock.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/cmds/portmaster-start/lock.go b/cmds/portmaster-start/lock.go index 14182069..e2dd56b6 100644 --- a/cmds/portmaster-start/lock.go +++ b/cmds/portmaster-start/lock.go @@ -1,6 +1,7 @@ package main import ( + "errors" "fmt" "io/ioutil" "log" @@ -34,10 +35,16 @@ func checkAndCreateInstanceLock(name string) (pid int32, err error) { // Check if process exists. p, err := processInfo.NewProcess(int32(parsedPid)) - if err != nil { + switch { + case err == nil: + // Process exists, continue. + case errors.Is(err, processInfo.ErrorProcessNotRunning): // A process with the locked PID does not exist. // This is expected, so we can continue normally. return 0, createInstanceLock(lockFilePath) + default: + // There was an internal error getting the process. + return 0, err } // Get the process paths and evaluate and clean them.