diff --git a/cmd/storage-rest-server.go b/cmd/storage-rest-server.go index f20e4e99f..17d274bca 100644 --- a/cmd/storage-rest-server.go +++ b/cmd/storage-rest-server.go @@ -290,6 +290,9 @@ func (s *storageRESTServer) NSScannerHandler(ctx context.Context, params *nsScan for update := range updates { resp := storageNSScannerRPC.NewResponse() resp.Update = &update + // Pooled responses may carry fields from a previous + // scan, always clear the fields not being set. + resp.Final = nil out <- resp } }() @@ -301,6 +304,7 @@ func (s *storageRESTServer) NSScannerHandler(ctx context.Context, params *nsScan // Send final response. resp := storageNSScannerRPC.NewResponse() resp.Final = &ui + resp.Update = nil out <- resp return nil } diff --git a/cmd/storage-rest-server_test.go b/cmd/storage-rest-server_test.go new file mode 100644 index 000000000..93ce35803 --- /dev/null +++ b/cmd/storage-rest-server_test.go @@ -0,0 +1,94 @@ +// Copyright (c) 2026 Julien Laurenceau and Feng Ruohang +// +// This file is part of Silo Object Storage. +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// This program is distributed in the hope that it will be useful +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +package cmd + +import ( + "context" + "testing" + + "github.com/minio/madmin-go/v3" + "github.com/minio/minio/internal/grid" +) + +type scannerResponseTestDisk struct { + StorageAPI +} + +func (scannerResponseTestDisk) NSScanner(_ context.Context, cache dataUsageCache, updates chan<- dataUsageEntry, _ madmin.HealScanMode, _ func() bool) (dataUsageCache, error) { + defer close(updates) + updates <- dataUsageEntry{Size: int64(cache.Info.NextCycle)} + cache.Info.NextCycle++ + return cache, nil +} + +func TestNSScannerHandlerClearsStaleResponses(t *testing.T) { + endpoint, err := NewEndpoint(t.TempDir()) + if err != nil { + t.Fatal(err) + } + globalLocalDrivesMu.Lock() + oldDrives, oldSets := globalLocalDrivesMap, globalLocalSetDrives + globalLocalDrivesMap = map[string]StorageAPI{endpoint.String(): scannerResponseTestDisk{}} + globalLocalSetDrives = nil + globalLocalDrivesMu.Unlock() + t.Cleanup(func() { + globalLocalDrivesMu.Lock() + globalLocalDrivesMap, globalLocalSetDrives = oldDrives, oldSets + globalLocalDrivesMu.Unlock() + }) + + oldRPC := storageNSScannerRPC + t.Cleanup(func() { storageNSScannerRPC = oldRPC }) + // Every new allocation is dirty too: the check must not depend on + // sync.Pool returning a previously used object. + storageNSScannerRPC = grid.NewStream[*nsScannerOptions, grid.NoPayload, *nsScannerResp](grid.HandlerNSScanner, + func() *nsScannerOptions { return &nsScannerOptions{} }, nil, + func() *nsScannerResp { + return &nsScannerResp{Update: &dataUsageEntry{Size: -1}, Final: &dataUsageCache{}} + }) + server := storageRESTServer{endpoint: endpoint} + for cycle := uint32(1); cycle <= 3; cycle++ { + out := make(chan *nsScannerResp, 2) + params := &nsScannerOptions{Cache: &dataUsageCache{Info: dataUsageCacheInfo{Name: "bucket", NextCycle: cycle}}} + if err := server.NSScannerHandler(t.Context(), params, out); err != nil { + t.Fatal(err) + } + if len(out) != 2 { + t.Fatalf("cycle %d: got %d responses, want an update and a final cache", cycle, len(out)) + } + for i := 0; i < 2; i++ { + resp := <-out + wire, err := resp.MarshalMsg(nil) + storageNSScannerRPC.PutResponse(resp) + if err != nil { + t.Fatal(err) + } + var decoded nsScannerResp + if _, err := decoded.UnmarshalMsg(wire); err != nil { + t.Fatal(err) + } + if i == 0 { + if decoded.Update == nil || decoded.Final != nil || decoded.Update.Size != int64(cycle) { + t.Errorf("cycle %d: invalid update response: %+v", cycle, decoded) + } + } else if decoded.Update != nil || decoded.Final == nil || decoded.Final.Info.NextCycle != cycle+1 { + t.Errorf("cycle %d: invalid final response: %+v", cycle, decoded) + } + } + } +} diff --git a/cmd/xl-storage-format-v2-legacy.go b/cmd/xl-storage-format-v2-legacy.go index 9d682487b..06201693b 100644 --- a/cmd/xl-storage-format-v2-legacy.go +++ b/cmd/xl-storage-format-v2-legacy.go @@ -40,6 +40,10 @@ func (x *xlMetaV2VersionHeader) unmarshalV(v uint8, bts []byte) (o []byte, err e // unmarshalV1 decodes version 1, never released. func (x *xlMetaV2VersionHeader) unmarshalV1(bts []byte) (o []byte, err error) { + // Fields absent from the V1 wire format must be reset since the + // header may be reused across decodes. + x.Signature = [4]byte{} + x.EcN, x.EcM = 0, 0 var zb0001 uint32 zb0001, bts, err = msgp.ReadArrayHeaderBytes(bts) if err != nil { @@ -134,7 +138,7 @@ type xlMetaV2VersionHeaderV2 struct { // UnmarshalMsg implements msgp.Unmarshaler func (z *xlMetaV2VersionHeaderV2) UnmarshalMsg(bts []byte) (o []byte, err error) { - z.EcN, z.EcN = 0, 0 + z.EcN, z.EcM = 0, 0 var zb0001 uint32 zb0001, bts, err = msgp.ReadArrayHeaderBytes(bts) if err != nil { @@ -184,7 +188,7 @@ func (z *xlMetaV2VersionHeaderV2) UnmarshalMsg(bts []byte) (o []byte, err error) // DecodeMsg implements msgp.Decodable func (z *xlMetaV2VersionHeaderV2) DecodeMsg(dc *msgp.Reader) (err error) { - z.EcN, z.EcN = 0, 0 + z.EcN, z.EcM = 0, 0 var zb0001 uint32 zb0001, err = dc.ReadArrayHeader() if err != nil { diff --git a/cmd/xl-storage-format-v2_test.go b/cmd/xl-storage-format-v2_test.go index 6a9b15a3f..f5f682500 100644 --- a/cmd/xl-storage-format-v2_test.go +++ b/cmd/xl-storage-format-v2_test.go @@ -38,6 +38,7 @@ import ( "github.com/klauspost/compress/zstd" "github.com/minio/minio/internal/bucket/lifecycle" xhttp "github.com/minio/minio/internal/http" + "github.com/tinylib/msgp/msgp" ) func TestReadXLMetaNoData(t *testing.T) { @@ -1190,3 +1191,92 @@ func BenchmarkToFileInfoNoParts(b *testing.B) { func BenchmarkToFileInfoWithParts(b *testing.B) { benchmarkManyPartsOptionally(b, true) } + +// TestXLMetaV2HeaderLegacyUnmarshalResetsEc checks that decoding legacy +// version headers, which carry no erasure metadata fields, cannot leak +// stale EcN/EcM values from a previously decoded object into the same +// structure. +func TestXLMetaV2HeaderLegacyUnmarshalResetsEc(t *testing.T) { + base := xlMetaV2VersionHeader{ + VersionID: [16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}, + ModTime: time.Now().Unix(), + Signature: [4]byte{1, 2, 3, 4}, + Type: ObjectType, + Flags: xlFlagInlineData, + } + + // Legacy V2 header wire format: 5 fields, no EcN/EcM. + var legacy []byte + legacy = msgp.AppendArrayHeader(legacy, 5) + legacy = msgp.AppendBytes(legacy, base.VersionID[:]) + legacy = msgp.AppendInt64(legacy, base.ModTime) + legacy = msgp.AppendBytes(legacy, base.Signature[:]) + legacy = msgp.AppendUint8(legacy, uint8(base.Type)) + legacy = msgp.AppendUint8(legacy, uint8(base.Flags)) + + for _, decoder := range []string{"UnmarshalMsg", "DecodeMsg", "loadIndexed"} { + t.Run(decoder, func(t *testing.T) { + header := base + header.EcN, header.EcM = 4, 8 + x2 := xlMetaV2VersionHeaderV2{xlMetaV2VersionHeader: &header} + var err error + switch decoder { + case "UnmarshalMsg": + _, err = x2.UnmarshalMsg(legacy) + case "DecodeMsg": + err = x2.DecodeMsg(msgp.NewReader(bytes.NewReader(legacy))) + case "loadIndexed": + indexed := msgp.AppendUint(nil, 2) + indexed = msgp.AppendUint(indexed, xlMetaVersion) + indexed = msgp.AppendInt(indexed, 1) + indexed = msgp.AppendBytes(indexed, legacy) + indexed = msgp.AppendBytes(indexed, nil) + xl := xlMetaV2{versions: []xlMetaV2ShallowVersion{{header: header}}} + err = xl.loadIndexed(indexed, nil) + header = xl.versions[0].header + } + if err != nil { + t.Fatal(err) + } + if header != base { + t.Fatalf("legacy header: got %+v, want %+v", header, base) + } + }) + } + + // V1 headers (4 fields, never released) must zero the fields too. + var v1 []byte + v1 = msgp.AppendArrayHeader(v1, 4) + v1 = msgp.AppendBytes(v1, base.VersionID[:]) + v1 = msgp.AppendInt64(v1, base.ModTime) + v1 = msgp.AppendUint8(v1, uint8(base.Type)) + v1 = msgp.AppendUint8(v1, uint8(base.Flags)) + + header := base + header.EcN, header.EcM = 4, 8 + header.Signature = [4]byte{9, 9, 9, 9} + if _, err := header.unmarshalV1(v1); err != nil { + t.Fatalf("legacy V1 header unmarshal failed: %v", err) + } + if header.EcN != 0 || header.EcM != 0 { + t.Fatalf("stale erasure metadata leaked on V1 decode: EcN %d, EcM %d", header.EcN, header.EcM) + } + if header.Signature != ([4]byte{}) { + t.Fatalf("stale signature leaked on V1 decode: %v", header.Signature) + } + + // Current headers must keep their erasure metadata. + current := base + current.EcN, current.EcM = 4, 8 + wire, err := current.MarshalMsg(nil) + if err != nil { + t.Fatalf("header marshal failed: %v", err) + } + decoded := base + if _, err := decoded.UnmarshalMsg(wire); err != nil { + t.Fatalf("header unmarshal failed: %v", err) + } + if decoded.EcN != 4 || decoded.EcM != 8 { + t.Fatalf("erasure metadata lost on current decode: EcN %d, EcM %d", decoded.EcN, decoded.EcM) + } +}