Compare commits

...

8 Commits

Author SHA1 Message Date
Feng Ruohang 2ca0f9961a build: gate releases on Object Lock regressions
Signed-off-by: Feng Ruohang <rh@vonng.com>
2026-10-15 13:04:18 +08:00
Feng Ruohang d94b3b3c92 Merge pull request #233 from julienlau/julienlau/fix/stale-metadata-leaks
Fix stale metadata in legacy xl.meta headers and scanner responses.

Keep scanner response resets local and cover reused legacy headers and both scanner response variants.

Signed-off-by: Feng Ruohang <rh@vonng.com>
2026-10-15 10:01:39 +08:00
Feng Ruohang 86c5a92eef fix: keep scanner response resets local and strengthen regression tests
Keep the legacy header resets and clear the unused NSScanner response
field at each construction site. Remove the generic stream Recycler call:
Recycler transfers resource ownership and cannot be treated as a reset
hook followed by another insertion into the handler's pool.

Exercise the actual NSScanner handler across repeated scans with dirty
response allocations, so the regression test does not depend on sync.Pool
reusing an object. Check both response variants after wire serialization.
Cover legacy DecodeMsg and reused indexed metadata headers as well.

Retain the original contributor's commits and DCO sign-offs.

Signed-off-by: Feng Ruohang <rh@vonng.com>
2026-10-15 09:48:16 +08:00
Feng Ruohang cdc3b8cb10 Merge remote-tracking branch 'origin/main' into codex/pr-233-maintainer-fix
Signed-off-by: Feng Ruohang <rh@vonng.com>
2026-10-15 09:42:46 +08:00
Feng Ruohang 0d3ba5d4cb Merge pull request #235 from pgsty/codex/retention-preflight-hardening
Retain the verified Object Lock write-lock defenses and the coordinated bilingual repair documentation.

Signed-off-by: Feng Ruohang <rh@vonng.com>
2026-10-14 23:42:33 +08:00
Julien Laurenceau ba6e8c8d69 fix: reset Signature when unmarshaling legacy V1 xl.meta headers
The V1 header wire format has no Signature field, so decoding into a
reused header leaked the previous object's signature. Reset it along
with EcN/EcM so a decode fully replaces the header contents.

Signed-off-by: Julien Laurenceau <julien.laurenceau@pepitedata.com>
2026-10-12 17:12:49 +02:00
Julien Laurenceau a54d0c4354 fix: reset pooled NSScanner RPC responses to avoid stale usage data
NSScanner responses stream either a dataUsageEntry update or the
final dataUsageCache. The grid framework recycles stream responses
into a pool without resetting them, and the handler set only one
field per response - so a recycled response still carried the Final
(or Update) payload of an earlier scan. A remote node reading such a
response accepted the stale final cache, corrupting bucket usage
data across scanner cycles.

Honor Recycler types when pooling stream responses, implement it for
nsScannerResp, and clear the unset field at the response
construction sites for defense in depth.

Signed-off-by: Julien Laurenceau <julien.laurenceau@pepitedata.com>
2026-10-12 17:02:02 +02:00
Julien Laurenceau 00d4a206f7 fix: zero EcM when unmarshaling legacy xl.meta V2 headers
The legacy header decode paths zeroed EcN twice (z.EcN, z.EcN = 0, 0)
instead of EcN and EcM, so EcM retained its previous value when the
same structure was reused across decodes. Decoding an xl.meta written
by an older release would then leak the erasure metadata (data/parity
block counts) of a previously decoded object into it, potentially
corrupting quorum and healing decisions.

Also zero the fields on the V1 header path, which had no reset at
all.

Signed-off-by: Julien Laurenceau <julien.laurenceau@pepitedata.com>
2026-10-12 17:02:02 +02:00
5 changed files with 200 additions and 2 deletions
+6
View File
@@ -70,6 +70,12 @@ jobs:
buildscripts/verify-rebrand.sh buildscripts/verify-rebrand.sh
dockerscripts/docker-entrypoint_test.sh dockerscripts/docker-entrypoint_test.sh
# Keep this gate aligned with the Object Lock race regression in go.yml.
- name: Gate release on Object Lock authorization and overwrite tests under race detector
env:
MINIO_API_REQUESTS_MAX: "10000"
run: go test -race -tags kqueue,dev ./cmd -run '^Test(APIPutObjectRetention|APIVersionAddressedWrite|PutObjectRetention|DirectoryObjectLock|ObjectLockOverwrite|APICopyObjectPreservesVersionAfterSuspension|MultipartNullObjectLock|PoolsObjectLockOverwrite|MultipartVersioningTransitions|PlanReview|RecoveredMultipartUpload)' -count=1 -timeout=5m
- name: Compute release variables - name: Compute release variables
env: env:
# Passed through the environment, never interpolated into the script # Passed through the environment, never interpolated into the script
+4
View File
@@ -290,6 +290,9 @@ func (s *storageRESTServer) NSScannerHandler(ctx context.Context, params *nsScan
for update := range updates { for update := range updates {
resp := storageNSScannerRPC.NewResponse() resp := storageNSScannerRPC.NewResponse()
resp.Update = &update resp.Update = &update
// Pooled responses may carry fields from a previous
// scan, always clear the fields not being set.
resp.Final = nil
out <- resp out <- resp
} }
}() }()
@@ -301,6 +304,7 @@ func (s *storageRESTServer) NSScannerHandler(ctx context.Context, params *nsScan
// Send final response. // Send final response.
resp := storageNSScannerRPC.NewResponse() resp := storageNSScannerRPC.NewResponse()
resp.Final = &ui resp.Final = &ui
resp.Update = nil
out <- resp out <- resp
return nil return nil
} }
+94
View File
@@ -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 <http://www.gnu.org/licenses/>.
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)
}
}
}
}
+6 -2
View File
@@ -40,6 +40,10 @@ func (x *xlMetaV2VersionHeader) unmarshalV(v uint8, bts []byte) (o []byte, err e
// unmarshalV1 decodes version 1, never released. // unmarshalV1 decodes version 1, never released.
func (x *xlMetaV2VersionHeader) unmarshalV1(bts []byte) (o []byte, err error) { 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 var zb0001 uint32
zb0001, bts, err = msgp.ReadArrayHeaderBytes(bts) zb0001, bts, err = msgp.ReadArrayHeaderBytes(bts)
if err != nil { if err != nil {
@@ -134,7 +138,7 @@ type xlMetaV2VersionHeaderV2 struct {
// UnmarshalMsg implements msgp.Unmarshaler // UnmarshalMsg implements msgp.Unmarshaler
func (z *xlMetaV2VersionHeaderV2) UnmarshalMsg(bts []byte) (o []byte, err error) { 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 var zb0001 uint32
zb0001, bts, err = msgp.ReadArrayHeaderBytes(bts) zb0001, bts, err = msgp.ReadArrayHeaderBytes(bts)
if err != nil { if err != nil {
@@ -184,7 +188,7 @@ func (z *xlMetaV2VersionHeaderV2) UnmarshalMsg(bts []byte) (o []byte, err error)
// DecodeMsg implements msgp.Decodable // DecodeMsg implements msgp.Decodable
func (z *xlMetaV2VersionHeaderV2) DecodeMsg(dc *msgp.Reader) (err error) { func (z *xlMetaV2VersionHeaderV2) DecodeMsg(dc *msgp.Reader) (err error) {
z.EcN, z.EcN = 0, 0 z.EcN, z.EcM = 0, 0
var zb0001 uint32 var zb0001 uint32
zb0001, err = dc.ReadArrayHeader() zb0001, err = dc.ReadArrayHeader()
if err != nil { if err != nil {
+90
View File
@@ -38,6 +38,7 @@ import (
"github.com/klauspost/compress/zstd" "github.com/klauspost/compress/zstd"
"github.com/minio/minio/internal/bucket/lifecycle" "github.com/minio/minio/internal/bucket/lifecycle"
xhttp "github.com/minio/minio/internal/http" xhttp "github.com/minio/minio/internal/http"
"github.com/tinylib/msgp/msgp"
) )
func TestReadXLMetaNoData(t *testing.T) { func TestReadXLMetaNoData(t *testing.T) {
@@ -1190,3 +1191,92 @@ func BenchmarkToFileInfoNoParts(b *testing.B) {
func BenchmarkToFileInfoWithParts(b *testing.B) { func BenchmarkToFileInfoWithParts(b *testing.B) {
benchmarkManyPartsOptionally(b, true) 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)
}
}