DAOS-10042 control: PATH is too long

[wangzhaorong-cestc]

DAOS-10042 control: PATH is too long

Before adding the usr/sbin environment variable in the execreq interface, check whether it already exists, and add it only if it does not exist

[github-actions[bot]]

Bug-tracker data: Ticket title is 'dmg storage query usage fails if arg list too long' Status is 'Open' Labels: 'triaged,usability' Job should run at elevated priority (3) https://daosio.atlassian.net/browse/DAOS-10042

[mjmac]

Sorry I missed the review request on this, must have gotten caught by filters.

The approach proposed in the PR may be OK, but I am not convinced that it's the correct solution. In the ticket (DAOS-10042), I don't see any evidence that the problem is that $PATH is really too long, or that the PR fixes the problem.

The PR seems to suggest that /usr/sbin is already in the $PATH, and checking for it will avoid making $PATH too long if it's already present. That may be true, but what about the case where $PATH is right at the maximum length and doesn't have /usr/sbin in it? The new code will correctly detect that it's missing and append it, and then the exec will fail again.

All that having been said, I'm not sure that the problem is really due to the size of the $PATH environment variable. According to the execve man page, the default limit for modern kernels is quite large. Do you have examples of extremely large $PATH variable values being used in your environment?

I created a simple test file and verified that on my system (el8.6), an env variable value can be up to 131063 bytes long:

package main

import (
        "fmt"
        "os"
        "os/exec"
        "strings"
)

func main() {
        val := os.Getenv("BIG_ENV")
        if val != "" {
                fmt.Printf("BIG_ENV len: %d\n", len(val))
                return
        }

        if err := os.Setenv("BIG_ENV", strings.Repeat("x", 131063)); err != nil {
                panic(err)
        }

        child := exec.Command(os.Args[0])
        child.Env = os.Environ()
        child.Stdout = os.Stdout

        if err := child.Run(); err != nil {
                panic(err)
        }
}

$ go run ./test.go
BIG_ENV len: 131063

[zp841126]

Hi [mjmac]

We will call "dmg storage scan query usage" periodicity(every 30s) in our system, it will encounter this issu after running one day.

I used strace to confirm the issue caused by append "/usr/sbin" multi times.

[zp841126]

BTW, I used centos 7.

[mjmac]

Oh, I understand the problem now. We shouldn't be appending to the $PATH environment in the parent process; we should only append to the child's environment. This is effectively a slow memory leak.

[mjmac]

Thank you for the proposed solution. I have implemented a simpler fix in #11146, please take a look.