From a54d0c43545c5707e3dd09d9b10c4614511a423a Mon Sep 17 00:00:00 2001 From: Julien Laurenceau Date: Mon, 12 Oct 2026 17:02:02 +0200 Subject: [PATCH] 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 --- cmd/storage-rest-common.go | 10 +++++++++ cmd/storage-rest-server.go | 4 ++++ cmd/storage-rest-server_test.go | 36 +++++++++++++++++++++++++++++++++ internal/grid/handlers.go | 5 +++++ 4 files changed, 55 insertions(+) create mode 100644 cmd/storage-rest-server_test.go diff --git a/cmd/storage-rest-common.go b/cmd/storage-rest-common.go index d8a28ae29..17bcc5f69 100644 --- a/cmd/storage-rest-common.go +++ b/cmd/storage-rest-common.go @@ -83,3 +83,13 @@ type nsScannerResp struct { Update *dataUsageEntry `msg:"u"` Final *dataUsageCache `msg:"f"` } + +// Recycle resets the response, so that recycled responses cannot +// leak stale scanner data into a subsequent scan. +func (r *nsScannerResp) Recycle() { + if r == nil { + return + } + r.Update = nil + r.Final = nil +} 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..354348882 --- /dev/null +++ b/cmd/storage-rest-server_test.go @@ -0,0 +1,36 @@ +// Copyright (c) 2015-2023 MinIO, Inc. +// +// This file is part of MinIO Object Storage stack +// +// 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 "testing" + +// TestNSScannerRespPooling verifies that recycled NSScanner stream +// responses cannot leak stale fields from a previous scan into new +// responses, which would otherwise produce incorrect bucket usage +// data. +func TestNSScannerRespPooling(t *testing.T) { + dirty := storageNSScannerRPC.NewResponse() + dirty.Update = &dataUsageEntry{} + dirty.Final = &dataUsageCache{} + storageNSScannerRPC.PutResponse(dirty) + + resp := storageNSScannerRPC.NewResponse() + if resp.Update != nil || resp.Final != nil { + t.Fatalf("pooled nsScannerResp leaked stale fields: Update %v, Final %v", resp.Update, resp.Final) + } +} diff --git a/internal/grid/handlers.go b/internal/grid/handlers.go index 1d20fa9b9..b0ca9e324 100644 --- a/internal/grid/handlers.go +++ b/internal/grid/handlers.go @@ -698,6 +698,11 @@ func (h *StreamTypeHandler[Payload, Req, Resp]) PutRequest(r Req) { // These should be returned by the caller. func (h *StreamTypeHandler[Payload, Req, Resp]) PutResponse(r Resp) { if r != h.nilResp { + if rc, ok := any(r).(Recycler); ok { + // Recycled stream responses must not leak fields + // from a previous use into the next response. + rc.Recycle() + } //nolint:staticcheck // SA6002 IT IS A GENERIC VALUE! (and always a pointer) h.respPool.Put(r) }