Nil pointer dereference on FreeBSD if pf is not enabled

[dfr]

Is this a BUG REPORT or FEATURE REQUEST? (leave only one on its own line)

/kind bug

Description

Attempting to run a container on a FreeBSD host where the pf kernel module is not loaded causes a nil pointer reference. The CNI setup fails with an appropriate error but later podman segfaults in (*Container).addNetworkNamespace. Looks like some error handling is missing, either in networking_freebsd.go or container_internal_freebsd.go

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0x1be85c9]

goroutine 1 [running]:
github.com/containers/podman/v4/libpod.(*Container).addNetworkNamespace(...)
	/wrkdirs/usr/ports/sysutils/podman/work/podman-a77ac5be8/libpod/container_internal_freebsd.go:175
github.com/containers/podman/v4/libpod.(*Container).generateSpec(0xc0003e4680, {0x76fe08, 0xc000326000})
	/wrkdirs/usr/ports/sysutils/podman/work/podman-a77ac5be8/libpod/container_internal_common.go:122 +0x1c9
...

Steps to reproduce the issue:

1. service pf stop && kldunload pf

2. podman run quay.io/dougrabson/hello

Describe the results you received:

Segfault as decribed above

Describe the results you expected:

Podman should report the error

Additional information you deem important (e.g. issue happens only occasionally):

Only affects FreeBSD

[github-actions]

A friendly reminder that this issue had no activity for 30 days.

[flouthoc]

Hi @dfr ,

I think following patch should fix this, but I don't have a freebsd setup handy. Could you try the patch and let me know :)

diff --git a/libpod/container_internal_freebsd.go b/libpod/container_internal_freebsd.go
index 32e1f1a8d..1c5a90b19 100644
--- a/libpod/container_internal_freebsd.go
+++ b/libpod/container_internal_freebsd.go
@@ -164,10 +164,10 @@ func (c *Container) reloadNetwork() error {
 // Add an existing container's network jail
 func (c *Container) addNetworkContainer(g *generate.Generator, ctr string) error {
        nsCtr, err := c.runtime.state.Container(ctr)
-       c.runtime.state.UpdateContainer(nsCtr)
        if err != nil {
                return fmt.Errorf("retrieving dependency %s of container %s from state: %w", ctr, c.ID(), err)
        }
+       c.runtime.state.UpdateContainer(nsCtr)
        g.AddAnnotation("org.freebsd.parentJail", nsCtr.state.NetNS.Name)
        return nil
 }

[dfr]

I had forgotten I created this issue - I think I orginally wanted to assign it to myself as a reminder but couldn't figure out how, possible because I'm not a member of containers. I think this was fixed in #16554 - testing against a recent podman build gives this:

Trying to pull quay.io/dougrabson/hello:latest...
Getting image source signatures
Copying blob b13a5ec7f3d2 done
Copying config f81c971736 done
Writing manifest to image destination
Storing signatures
WARN[0003] Failed to load cached network config: network podman not found in CNI cache, falling back to loading network podman from disk
Error: plugin type="bridge" failed (add): cni plugin bridge failed: The pf kernel module must be loaded to support ipMasq networks

Testing with a build just prior to the merge of #16554 reproduces the original issue but unfortunately your change doesn't help - the nil pointer dereference is in the statement calling g.AddAnnotation. I like your suggestion though - it makes sense to propagate the error immediately. I'll make a PR with that and add a check for nsCtr.state.NetNS being non-nil.