pkg/internal/filesystem/lock.go: potential nil pointer panic from file.Close() in deferred function

[fedebram]

Hi everyone, I was reading the nerdctl codebase to understand how networking is handled with containerd. Inside pkg/internal/filesystem/lock.go there is a function called commonLock, and within it there is a deferred function that calls file.Close(). The problem is that in every error path, the file is either already closed or still nil, so if an error occurs, the deferred function will call Close() on a nil pointer, resulting in a panic.

This is an edge case, but it can be resolved by simply removing the file.Close() call inside the deferred function.

I can open a PR if that would be helpful.

Here is a snippet of the function I am referring to:

func commonlock(path string, mode lockType) (file *os.File, err error) {
	defer func() {
		if err != nil {
			err = errors.Join(ErrLockFail, err, file.Close())
		}
	}()
	if runtime.GOOS == "windows" {
		if sl, err := os.Stat(path); err == nil && sl.IsDir() {
			path = path + ".nerdctl.lock"
		}
	}
	file, err = os.Open(path)
	if errors.Is(err, os.ErrNotExist) {
		file, err = os.OpenFile(path, os.O_RDONLY|os.O_CREATE, privateFilePermission)
	}
	if err != nil {
		return nil, err
	}
	if err = platformSpecificLock(file, mode); err != nil {
		return nil, errors.Join(err, file.Close())
	}
	return file, nil
}

[AkihiroSuda]

I can open a PR if that would be helpful.

Thanks for reporting, could you open a PR?

[fedebram]

Ok, I'm going to open a pr. Thanks for the response