From 81b69bf28eaaa53990248df0b803e50be8824cd8 Mon Sep 17 00:00:00 2001 From: Andrey Smirnov Date: Tue, 4 Jun 2024 20:39:45 +0400 Subject: [PATCH] fix: use read full when reading data It's not guaranteed that any `.Read` call will read the full buffer. Signed-off-by: Andrey Smirnov --- blkid/blkid_linux.go | 2 +- blkid/blkid_linux_test.go | 13 +++++++--- blkid/internal/filesystems/ext/ext.go | 2 +- blkid/internal/filesystems/iso9660/iso9660.go | 3 ++- blkid/internal/filesystems/luks/luks.go | 3 ++- blkid/internal/filesystems/lvm2/lvm2.go | 3 ++- .../internal/filesystems/squashfs/squashfs.go | 3 ++- blkid/internal/filesystems/swap/swap.go | 3 ++- .../filesystems/talosmeta/talosmeta.go | 5 ++-- blkid/internal/filesystems/vfat/vfat.go | 4 +-- blkid/internal/filesystems/xfs/xfs.go | 6 +++-- blkid/internal/filesystems/zfs/zfs.go | 3 ++- blkid/internal/partitions/gpt/gpt.go | 5 ++-- blkid/internal/utils/utils.go | 25 ++++++++++++++++++ blkid/probe_linux.go | 26 ++++++++++++------- 15 files changed, 78 insertions(+), 28 deletions(-) diff --git a/blkid/blkid_linux.go b/blkid/blkid_linux.go index f9feaa4..40e4a6f 100644 --- a/blkid/blkid_linux.go +++ b/blkid/blkid_linux.go @@ -124,7 +124,7 @@ func Probe(f *os.File, opts ...ProbeOption) (*Info, error) { defer wholeDisk.Unlock() //nolint:errcheck } - if err := info.fillProbeResult(f); err != nil { + if err := info.fillProbeResult(f, options); err != nil { return nil, fmt.Errorf("failed to probe: %w", err) } diff --git a/blkid/blkid_linux_test.go b/blkid/blkid_linux_test.go index 44f37df..23cfb76 100644 --- a/blkid/blkid_linux_test.go +++ b/blkid/blkid_linux_test.go @@ -24,6 +24,7 @@ import ( "github.com/siderolabs/go-pointer" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "go.uber.org/zap/zaptest" "github.com/siderolabs/go-blockdevice/v2/blkid" "github.com/siderolabs/go-blockdevice/v2/block" @@ -437,7 +438,9 @@ func TestProbePathFilesystems(t *testing.T) { test.setup(t, probePath) - info, err := blkid.ProbePath(probePath) + logger := zaptest.NewLogger(t) + + info, err := blkid.ProbePath(probePath, blkid.WithProbeLogger(logger)) require.NoError(t, err) if useLoopDevice { @@ -652,7 +655,9 @@ func TestProbePathGPT(t *testing.T) { test.setup(t, probePath) - info, err := blkid.ProbePath(probePath) + logger := zaptest.NewLogger(t) + + info, err := blkid.ProbePath(probePath, blkid.WithProbeLogger(logger)) require.NoError(t, err) if useLoopDevice { @@ -745,7 +750,9 @@ func TestProbePathNested(t *testing.T) { test.setup(t, probePath) - info, err := blkid.ProbePath(probePath) + logger := zaptest.NewLogger(t) + + info, err := blkid.ProbePath(probePath, blkid.WithProbeLogger(logger)) require.NoError(t, err) assert.NotNil(t, info.BlockDevice) diff --git a/blkid/internal/filesystems/ext/ext.go b/blkid/internal/filesystems/ext/ext.go index 7b73d7f..0cdc67a 100644 --- a/blkid/internal/filesystems/ext/ext.go +++ b/blkid/internal/filesystems/ext/ext.go @@ -49,7 +49,7 @@ func (p *Probe) Name() string { func (p *Probe) Probe(r probe.Reader, _ magic.Magic) (*probe.Result, error) { buf := make([]byte, SUPERBLOCK_SIZE) - if _, err := r.ReadAt(buf, sbOffset); err != nil { + if err := utils.ReadFullAt(r, buf, sbOffset); err != nil { return nil, err } diff --git a/blkid/internal/filesystems/iso9660/iso9660.go b/blkid/internal/filesystems/iso9660/iso9660.go index 1dcc96a..5bfaffa 100644 --- a/blkid/internal/filesystems/iso9660/iso9660.go +++ b/blkid/internal/filesystems/iso9660/iso9660.go @@ -15,6 +15,7 @@ import ( "github.com/siderolabs/go-blockdevice/v2/blkid/internal/magic" "github.com/siderolabs/go-blockdevice/v2/blkid/internal/probe" + "github.com/siderolabs/go-blockdevice/v2/blkid/internal/utils" ) const ( @@ -61,7 +62,7 @@ vdLoop: for i := range vdMax { buf := make([]byte, VOLUMEDESCRIPTOR_SIZE) - if _, err := r.ReadAt(buf, superblockOffset+sectorSize*int64(i)); err != nil { + if err := utils.ReadFullAt(r, buf, superblockOffset+sectorSize*int64(i)); err != nil { break } diff --git a/blkid/internal/filesystems/luks/luks.go b/blkid/internal/filesystems/luks/luks.go index 26e96a8..e30674c 100644 --- a/blkid/internal/filesystems/luks/luks.go +++ b/blkid/internal/filesystems/luks/luks.go @@ -15,6 +15,7 @@ import ( "github.com/siderolabs/go-blockdevice/v2/blkid/internal/magic" "github.com/siderolabs/go-blockdevice/v2/blkid/internal/probe" + "github.com/siderolabs/go-blockdevice/v2/blkid/internal/utils" ) var luksMagic = magic.Magic{ @@ -39,7 +40,7 @@ func (p *Probe) Name() string { func (p *Probe) Probe(r probe.Reader, _ magic.Magic) (*probe.Result, error) { buf := make([]byte, LUKS2HEADER_SIZE) - if _, err := r.ReadAt(buf, 0); err != nil { + if err := utils.ReadFullAt(r, buf, 0); err != nil { return nil, err } diff --git a/blkid/internal/filesystems/lvm2/lvm2.go b/blkid/internal/filesystems/lvm2/lvm2.go index b016b09..1f722eb 100644 --- a/blkid/internal/filesystems/lvm2/lvm2.go +++ b/blkid/internal/filesystems/lvm2/lvm2.go @@ -10,6 +10,7 @@ package lvm2 import ( "github.com/siderolabs/go-blockdevice/v2/blkid/internal/magic" "github.com/siderolabs/go-blockdevice/v2/blkid/internal/probe" + "github.com/siderolabs/go-blockdevice/v2/blkid/internal/utils" ) var ( @@ -43,7 +44,7 @@ func (p *Probe) Name() string { func (p *Probe) probe(r probe.Reader, offset int64) (LVM2Header, error) { buf := make([]byte, LVM2HEADER_SIZE) - if _, err := r.ReadAt(buf, offset); err != nil { + if err := utils.ReadFullAt(r, buf, offset); err != nil { return nil, err } diff --git a/blkid/internal/filesystems/squashfs/squashfs.go b/blkid/internal/filesystems/squashfs/squashfs.go index ae84e68..59f3684 100644 --- a/blkid/internal/filesystems/squashfs/squashfs.go +++ b/blkid/internal/filesystems/squashfs/squashfs.go @@ -10,6 +10,7 @@ package squashfs import ( "github.com/siderolabs/go-blockdevice/v2/blkid/internal/magic" "github.com/siderolabs/go-blockdevice/v2/blkid/internal/probe" + "github.com/siderolabs/go-blockdevice/v2/blkid/internal/utils" ) var squashfsMagic1 = magic.Magic{ // big endian @@ -42,7 +43,7 @@ func (p *Probe) Name() string { func (p *Probe) Probe(r probe.Reader, _ magic.Magic) (*probe.Result, error) { buf := make([]byte, SUPERBLOCK_SIZE) - if _, err := r.ReadAt(buf, 0); err != nil { + if err := utils.ReadFullAt(r, buf, 0); err != nil { return nil, err } diff --git a/blkid/internal/filesystems/swap/swap.go b/blkid/internal/filesystems/swap/swap.go index f8d3658..cda0d0d 100644 --- a/blkid/internal/filesystems/swap/swap.go +++ b/blkid/internal/filesystems/swap/swap.go @@ -16,6 +16,7 @@ import ( "github.com/siderolabs/go-blockdevice/v2/blkid/internal/magic" "github.com/siderolabs/go-blockdevice/v2/blkid/internal/probe" + "github.com/siderolabs/go-blockdevice/v2/blkid/internal/utils" ) var ( @@ -98,7 +99,7 @@ func (p *Probe) Name() string { func (p *Probe) Probe(r probe.Reader, m magic.Magic) (*probe.Result, error) { buf := make([]byte, SWAPHEADER_SIZE) - if _, err := r.ReadAt(buf, 1024); err != nil { + if err := utils.ReadFullAt(r, buf, 1024); err != nil { return nil, err } diff --git a/blkid/internal/filesystems/talosmeta/talosmeta.go b/blkid/internal/filesystems/talosmeta/talosmeta.go index 00ba822..d67ce17 100644 --- a/blkid/internal/filesystems/talosmeta/talosmeta.go +++ b/blkid/internal/filesystems/talosmeta/talosmeta.go @@ -10,6 +10,7 @@ import ( "github.com/siderolabs/go-blockdevice/v2/blkid/internal/magic" "github.com/siderolabs/go-blockdevice/v2/blkid/internal/probe" + "github.com/siderolabs/go-blockdevice/v2/blkid/internal/utils" ) // META constants, from talos/internal/pkg/meta/internal/adv/talos. @@ -44,7 +45,7 @@ func (p *Probe) Probe(r probe.Reader, _ magic.Magic) (*probe.Result, error) { buf := make([]byte, 4) for _, offset := range []int64{0, length} { - if _, err := r.ReadAt(buf, offset); err != nil { + if err := utils.ReadFullAt(r, buf, offset); err != nil { return nil, err } @@ -52,7 +53,7 @@ func (p *Probe) Probe(r probe.Reader, _ magic.Magic) (*probe.Result, error) { continue } - if _, err := r.ReadAt(buf, offset+length-4); err != nil { + if err := utils.ReadFullAt(r, buf, offset+length-4); err != nil { return nil, err } diff --git a/blkid/internal/filesystems/vfat/vfat.go b/blkid/internal/filesystems/vfat/vfat.go index e2c699e..4ed0fe8 100644 --- a/blkid/internal/filesystems/vfat/vfat.go +++ b/blkid/internal/filesystems/vfat/vfat.go @@ -72,11 +72,11 @@ func (p *Probe) Probe(r probe.Reader, _ magic.Magic) (*probe.Result, error) { vfatBuf := make([]byte, VFATSB_SIZE) msdosBuf := make([]byte, MSDOSSB_SIZE) - if _, err := r.ReadAt(vfatBuf, 0); err != nil { + if err := utils.ReadFullAt(r, vfatBuf, 0); err != nil { return nil, err } - if _, err := r.ReadAt(msdosBuf, 0); err != nil { + if err := utils.ReadFullAt(r, msdosBuf, 0); err != nil { return nil, err } diff --git a/blkid/internal/filesystems/xfs/xfs.go b/blkid/internal/filesystems/xfs/xfs.go index b244e11..9bc7e7b 100644 --- a/blkid/internal/filesystems/xfs/xfs.go +++ b/blkid/internal/filesystems/xfs/xfs.go @@ -9,12 +9,14 @@ package xfs import ( "bytes" + "fmt" "github.com/google/uuid" "github.com/siderolabs/go-pointer" "github.com/siderolabs/go-blockdevice/v2/blkid/internal/magic" "github.com/siderolabs/go-blockdevice/v2/blkid/internal/probe" + "github.com/siderolabs/go-blockdevice/v2/blkid/internal/utils" ) var xfsMagic = magic.Magic{ @@ -39,13 +41,13 @@ func (p *Probe) Name() string { func (p *Probe) Probe(r probe.Reader, _ magic.Magic) (*probe.Result, error) { buf := make([]byte, SUPERBLOCK_SIZE) - if _, err := r.ReadAt(buf, 0); err != nil { + if err := utils.ReadFullAt(r, buf, 0); err != nil { return nil, err } sb := SuperBlock(buf) if !sb.Valid() { - return nil, nil //nolint:nilnil + return nil, fmt.Errorf("xfs superblock is not valid") } uuid, err := uuid.FromBytes(sb.Get_sb_uuid()) diff --git a/blkid/internal/filesystems/zfs/zfs.go b/blkid/internal/filesystems/zfs/zfs.go index 561ddab..385078a 100644 --- a/blkid/internal/filesystems/zfs/zfs.go +++ b/blkid/internal/filesystems/zfs/zfs.go @@ -12,6 +12,7 @@ import ( "github.com/siderolabs/go-blockdevice/v2/blkid/internal/magic" "github.com/siderolabs/go-blockdevice/v2/blkid/internal/probe" + "github.com/siderolabs/go-blockdevice/v2/blkid/internal/utils" ) // https://github.com/util-linux/util-linux/blob/c0207d354ee47fb56acfa64b03b5b559bb301280/libblkid/src/superblocks/zfs.c @@ -65,7 +66,7 @@ func (p *Probe) Probe(r probe.Reader, _ magic.Magic) (*probe.Result, error) { size - zfsStartOffset - lastLabelOffset, } { labelBuf := make([]byte, zfsVdevLabelSize) - if _, err := r.ReadAt(labelBuf, int64(labelOffset)); err != nil { + if err := utils.ReadFullAt(r, labelBuf, int64(labelOffset)); err != nil { return nil, err } diff --git a/blkid/internal/partitions/gpt/gpt.go b/blkid/internal/partitions/gpt/gpt.go index 6b14b10..17bc230 100644 --- a/blkid/internal/partitions/gpt/gpt.go +++ b/blkid/internal/partitions/gpt/gpt.go @@ -15,6 +15,7 @@ import ( "github.com/siderolabs/go-blockdevice/v2/blkid/internal/magic" "github.com/siderolabs/go-blockdevice/v2/blkid/internal/probe" + "github.com/siderolabs/go-blockdevice/v2/blkid/internal/utils" ) //go:generate go run ../../cstruct/cstruct.go -pkg gpt -struct Header -input header.h -endianness LittleEndian @@ -144,7 +145,7 @@ func readHeader(r probe.Reader, lba, lastLBA uint64) (*Header, []Entry, error) { sectorSize := r.GetSectorSize() buf := make([]byte, sectorSize) - if _, err := r.ReadAt(buf, int64(lba)*int64(sectorSize)); err != nil { + if err := utils.ReadFullAt(r, buf, int64(lba)*int64(sectorSize)); err != nil { return nil, nil, err } @@ -196,7 +197,7 @@ func readHeader(r probe.Reader, lba, lastLBA uint64) (*Header, []Entry, error) { // read partition entries, verify checksum entriesBuffer := make([]byte, hdr.Get_num_partition_entries()*ENTRY_SIZE) - if _, err := r.ReadAt(entriesBuffer, int64(hdr.Get_partition_entries_lba())*int64(sectorSize)); err != nil { + if err := utils.ReadFullAt(r, entriesBuffer, int64(hdr.Get_partition_entries_lba())*int64(sectorSize)); err != nil { return nil, nil, err } diff --git a/blkid/internal/utils/utils.go b/blkid/internal/utils/utils.go index 7dbc9ee..f88f446 100644 --- a/blkid/internal/utils/utils.go +++ b/blkid/internal/utils/utils.go @@ -7,6 +7,7 @@ package utils import ( "hash/crc32" + "io" "sync" ) @@ -23,3 +24,27 @@ func CRC32c(buf []byte) uint32 { func IsPowerOf2[T uint8 | uint16 | uint32 | uint64](num T) bool { return (num != 0 && ((num & (num - 1)) == 0)) } + +// ReadFullAt is io.ReadFull for io.ReaderAt. +func ReadFullAt(r io.ReaderAt, buf []byte, offset int64) error { + for n := 0; n < len(buf); { + m, err := r.ReadAt(buf[n:], offset) + + n += m + offset += int64(m) + + if err != nil { + if err == io.EOF && n == len(buf) { + return nil + } + + if err == io.EOF { + err = io.ErrUnexpectedEOF + } + + return err + } + } + + return nil +} diff --git a/blkid/probe_linux.go b/blkid/probe_linux.go index 7081fea..318ad1a 100644 --- a/blkid/probe_linux.go +++ b/blkid/probe_linux.go @@ -11,8 +11,11 @@ import ( "io" "os" + "go.uber.org/zap" + "github.com/siderolabs/go-blockdevice/v2/blkid/internal/chain" "github.com/siderolabs/go-blockdevice/v2/blkid/internal/probe" + "github.com/siderolabs/go-blockdevice/v2/blkid/internal/utils" ) type probeReader struct { @@ -30,10 +33,10 @@ func (r *probeReader) GetSize() uint64 { return r.size } -func (i *Info) fillProbeResult(f *os.File) error { +func (i *Info) fillProbeResult(f *os.File, options ProbeOptions) error { chain := chain.Default() - res, matched, err := i.probe(f, chain, 0, i.Size) + res, matched, err := i.probe(f, chain, 0, i.Size, options) if err != nil { return fmt.Errorf("error probing: %w", err) } @@ -49,14 +52,14 @@ func (i *Info) fillProbeResult(f *os.File) error { i.ProbedSize = res.ProbedSize i.Label = res.Label - if err = i.fillNested(f, chain, 0, &i.Parts, res.Parts); err != nil { + if err = i.fillNested(f, chain, 0, &i.Parts, res.Parts, options); err != nil { return fmt.Errorf("error probing nested: %w", err) } return nil } -func (i *Info) fillNested(f *os.File, chain chain.Chain, offset uint64, out *[]NestedProbeResult, parts []probe.Partition) error { +func (i *Info) fillNested(f *os.File, chain chain.Chain, offset uint64, out *[]NestedProbeResult, parts []probe.Partition, options ProbeOptions) error { if len(parts) == 0 { return nil } @@ -72,7 +75,7 @@ func (i *Info) fillNested(f *os.File, chain chain.Chain, offset uint64, out *[]N (*out)[idx].PartitionOffset = part.Offset (*out)[idx].PartitionSize = part.Size - res, matched, err := i.probe(f, chain, offset+part.Offset, part.Size) + res, matched, err := i.probe(f, chain, offset+part.Offset, part.Size, options) if err != nil { return fmt.Errorf("error probing nested: %w", err) } @@ -88,7 +91,7 @@ func (i *Info) fillNested(f *os.File, chain chain.Chain, offset uint64, out *[]N (*out)[idx].ProbedSize = res.ProbedSize (*out)[idx].Label = res.Label - if err = i.fillNested(f, chain, offset+part.Offset, &(*out)[idx].Parts, res.Parts); err != nil { + if err = i.fillNested(f, chain, offset+part.Offset, &(*out)[idx].Parts, res.Parts, options); err != nil { return fmt.Errorf("error probing nested: %w", err) } } @@ -96,7 +99,7 @@ func (i *Info) fillNested(f *os.File, chain chain.Chain, offset uint64, out *[]N return nil } -func (i *Info) probe(f *os.File, chain chain.Chain, offset, length uint64) (*probe.Result, probe.Prober, error) { +func (i *Info) probe(f *os.File, chain chain.Chain, offset, length uint64, options ProbeOptions) (*probe.Result, probe.Prober, error) { if offset+length > i.Size { return nil, nil, fmt.Errorf("probing range is out of bounds: offset %d + len %d > size %d", offset, length, i.Size) } @@ -113,8 +116,7 @@ func (i *Info) probe(f *os.File, chain chain.Chain, offset, length uint64) (*pro buf := make([]byte, magicReadSize) - _, err := f.ReadAt(buf, int64(offset)) - if err != nil { + if err := utils.ReadFullAt(f, buf, int64(offset)); err != nil { return nil, nil, fmt.Errorf("error reading magic buffer: %w", err) } @@ -128,6 +130,12 @@ func (i *Info) probe(f *os.File, chain chain.Chain, offset, length uint64) (*pro for _, matched := range chain.MagicMatches(buf) { res, err := matched.Prober.Probe(pR, matched.Magic) if err != nil || res == nil { + if err != nil { + options.Logger.Debug("magic matched, but probe failed", zap.Uint64("offset", offset), zap.String("name", matched.Prober.Name()), zap.Error(err)) + } else { + options.Logger.Debug("magic matched, but probe returned no result", zap.Uint64("offset", offset), zap.String("name", matched.Prober.Name())) + } + // skip failed probes continue }