moby / buildkit

concurrent, cache-efficient, and Dockerfile-agnostic builder toolkit
https://github.com/moby/moby/issues/34227
Apache License 2.0
8.27k stars 1.17k forks source link

Panic when building multi-platform `FROM scratch` image (nil pointer dereference in `buildkit/frontend/gateway/grpcclient.convertRef`) #5379

Closed apostasie closed 1 month ago

apostasie commented 2 months ago

First seen as part of nerdctl https://github.com/containerd/nerdctl/issues/3481

Reproducer:

buildctl --addr=unix:///run/user/501/buildkit/buildkitd.sock build --frontend=dockerfile.v0 --local=context=. --output=type=oci --local=dockerfile=/tmp --opt=filename=Dockerfile --opt platform=linux/amd64,linux/arm64 > foo.out

Dockerfile:

FROM scratch

Results in:

[+] Building 0.0s (0/1)
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x10 pc=0x6c75e8]

goroutine 9 [running]:
github.com/moby/buildkit/frontend/gateway/grpcclient.convertRef({0xc096a0?, 0x0})
    /src/frontend/gateway/grpcclient/client.go:99 +0x48
github.com/moby/buildkit/frontend/gateway/grpcclient.(*grpcClient).Run.func1()
    /src/frontend/gateway/grpcclient/client.go:131 +0x824
github.com/moby/buildkit/frontend/gateway/grpcclient.(*grpcClient).Run(0x40003c0900, {0xc07990, 0x40003f86e0}, 0x40004860e0)
    /src/frontend/gateway/grpcclient/client.go:249 +0x3f8
github.com/moby/buildkit/client.(*Client).Build.func2({0x4000362dc1, 0x19}, 0x400041e230, 0x0?)
    /src/client/build.go:58 +0x30c
github.com/moby/buildkit/client.(*Client).solve.func3()
    /src/client/solve.go:295 +0x54
golang.org/x/sync/errgroup.(*Group).Go.func1()
    /src/vendor/golang.org/x/sync/errgroup/errgroup.go:78 +0x58
created by golang.org/x/sync/errgroup.(*Group).Go in goroutine 30
    /src/vendor/golang.org/x/sync/errgroup/errgroup.go:75 +0x98

System:

buildctl github.com/moby/buildkit v0.16.0 0865fcc9b78559e856e81dc52b3613701e7be28d
buildkitd github.com/moby/buildkit v0.16.0 0865fcc9b78559e856e81dc52b3613701e7be28d

Linux lima-v2 6.8.0-45-generic #45-Ubuntu SMP PREEMPT_DYNAMIC Fri Aug 30 12:26:41 UTC 2024 aarch64 aarch64 aarch64 GNU/Linux

Running rootless or rootfull does not seem to change the outcome.

This is also true for 1.15.3

apostasie commented 2 months ago

cc @AkihiroSuda

AkihiroSuda commented 2 months ago

lima

Is this issue really specific to Lima?

apostasie commented 2 months ago

lima

Is this issue really specific to Lima?

I do not think so.

jsternberg commented 1 month ago

The issue seems to be related to Go's nil keyword working very weirdly with interfaces. One method passes a (*reference)(nil) to a method that takes client.Reference and client.Reference(nil) != (*reference)(nil) so the nil check fails.

The linked PR above should fix it. Thanks for finding this. I think this likely affected multi-platform because the following code path was wrong:

for k, v := range pbRes.Refs.Refs {
    var ref *reference
    if v.Id != "" {
        ref, err = newReference(c, v)
        if err != nil {
            return nil, err
        }
    }
    res.AddRef(k, ref) // incorrect usage of ref when v.Id is blank
}

I also cleaned it up a bit while I was in there since newReference never returned an error and the function signature didn't benefit from returning an error due to function chaining anywhere.

apostasie commented 1 month ago

@jsternberg really appreciate the fast fix. Thanks a lot!