Skip to content

Commit

Permalink
meta: fix data race caused by global buffer (#57)
Browse files Browse the repository at this point in the history
* meta: fix data race caused by global buffer

Use function local backing array instead. As the size of
the function local backing array is known at compile time,
hopefully it can be allocated on the stack, to avoid heap
allocations.

This issue was identified by @zachorosz.

Fixes #56.

* flac: update dependencies, run `go get -u ./...`

* readme: add release notes for v1.0.8

Issues identified:
- @ktmf01 (issue encoding 8-bps WAV audio samples)
- @zachorosz (race condition when reading meta data)
- @karlek (incorrect variable used for metadata block type in error string)
  • Loading branch information
mewmew authored Apr 9, 2023
1 parent 8358308 commit f358872
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 46 deletions.
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,11 @@ Documentation provided by GoDoc.

## Changes

* Version 1.0.8 (TBD)
- Fix race condition when reading meta data (see [#56](https://github.com/mewkiz/flac/pull/56)). Thanks to [Zach Orosz](https://github.com/zachorosz).
- Fix encoding of 8-bps WAV audio samples (see [#52](https://github.com/mewkiz/flac/pull/52)). Thanks to [Martijn van Beurden](https://github.com/ktmf01).
- Fix StreamInfo block type error message (see [#49](https://github.com/mewkiz/flac/pull/49)).

* Version 1.0.7 (2021-01-28)
- Add seek API (see [#44](https://github.com/mewkiz/flac/pull/44) and [#46](https://github.com/mewkiz/flac/pull/46)). Thanks to [Craig Swank](https://github.com/cswank).

Expand Down
8 changes: 4 additions & 4 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ go 1.14

require (
github.com/go-audio/audio v1.0.0
github.com/go-audio/wav v1.0.0
github.com/icza/bitio v1.0.0
github.com/mewkiz/pkg v0.0.0-20190919212034-518ade7978e2
github.com/pkg/errors v0.8.1
github.com/go-audio/wav v1.1.0
github.com/icza/bitio v1.1.0
github.com/mewkiz/pkg v0.0.0-20230226050401-4010bf0fec14
github.com/pkg/errors v0.9.1
)
45 changes: 36 additions & 9 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,43 @@ github.com/go-audio/audio v1.0.0 h1:zS9vebldgbQqktK4H0lUqWrG8P0NxCJVqcj7ZpNnwd4=
github.com/go-audio/audio v1.0.0/go.mod h1:6uAu0+H2lHkwdGsAY+j2wHPNPpPoeg5AaEFh9FlA+Zs=
github.com/go-audio/riff v1.0.0 h1:d8iCGbDvox9BfLagY94fBynxSPHO80LmZCaOsmKxokA=
github.com/go-audio/riff v1.0.0/go.mod h1:l3cQwc85y79NQFCRB7TiPoNiaijp6q8Z0Uv38rVG498=
github.com/go-audio/wav v1.0.0 h1:WdSGLhtyud6bof6XHL28xKeCQRzCV06pOFo3LZsFdyE=
github.com/go-audio/wav v1.0.0/go.mod h1:3yoReyQOsiARkvPl3ERCi8JFjihzG6WhjYpZCf5zAWE=
github.com/icza/bitio v1.0.0 h1:squ/m1SHyFeCA6+6Gyol1AxV9nmPPlJFT8c2vKdj3U8=
github.com/icza/bitio v1.0.0/go.mod h1:0jGnlLAx8MKMr9VGnn/4YrvZiprkvBelsVIbA9Jjr9A=
github.com/go-audio/wav v1.1.0 h1:jQgLtbqBzY7G+BM8fXF7AHUk1uHUviWS4X39d5rsL2g=
github.com/go-audio/wav v1.1.0/go.mod h1:mpe9qfwbScEbkd8uybLuIpTgHyrISw/OTuvjUW2iGtE=
github.com/icza/bitio v1.1.0 h1:ysX4vtldjdi3Ygai5m1cWy4oLkhWTAi+SyO6HC8L9T0=
github.com/icza/bitio v1.1.0/go.mod h1:0jGnlLAx8MKMr9VGnn/4YrvZiprkvBelsVIbA9Jjr9A=
github.com/icza/mighty v0.0.0-20180919140131-cfd07d671de6 h1:8UsGZ2rr2ksmEru6lToqnXgA8Mz1DP11X4zSJ159C3k=
github.com/icza/mighty v0.0.0-20180919140131-cfd07d671de6/go.mod h1:xQig96I1VNBDIWGCdTt54nHt6EeI639SmHycLYL7FkA=
github.com/mewkiz/pkg v0.0.0-20190919212034-518ade7978e2 h1:EyTNMdePWaoWsRSGQnXiSoQu0r6RS1eA557AwJhlzHU=
github.com/mewkiz/pkg v0.0.0-20190919212034-518ade7978e2/go.mod h1:3E2FUC/qYUfM8+r9zAwpeHJzqRVVMIYnpzD/clwWxyA=
github.com/pkg/errors v0.8.1 h1:iURUrRGxPUNPdy5/HRSm+Yj6okJ6UtLINN0Q9M4+h3I=
github.com/jszwec/csvutil v1.5.1/go.mod h1:Rpu7Uu9giO9subDyMCIQfHVDuLrcaC36UA4YcJjGBkg=
github.com/mewkiz/pkg v0.0.0-20230226050401-4010bf0fec14 h1:tnAPMExbRERsyEYkmR1YjhTgDM0iqyiBYf8ojRXxdbA=
github.com/mewkiz/pkg v0.0.0-20230226050401-4010bf0fec14/go.mod h1:QYCFBiH5q6XTHEbWhR0uhR3M9qNPoD2CSQzr0g75kE4=
github.com/pkg/errors v0.8.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
golang.org/x/image v0.0.0-20190220214146-31aff87c08e9/go.mod h1:kZ7UVZpmo3dzQBMxlp+ypCbDeSB+sBbTgSJuh5dn5js=
golang.org/x/net v0.0.0-20190213061140-3a22650c66bd/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
github.com/pkg/errors v0.9.1 h1:FEBLx1zS214owpjy7qsBeixbURkuhQAwrK5UwLGTwt4=
github.com/pkg/errors v0.9.1/go.mod h1:bwawxfHBFNV+L2hUp1rHADufV3IMtnDRdf1r5NINEl0=
github.com/yuin/goldmark v1.4.13/go.mod h1:6yULJ656Px+3vBD8DxQVa3kxgyrAnzto9xy5taEt/CY=
golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w=
golang.org/x/crypto v0.0.0-20210921155107-089bfa567519/go.mod h1:GvvjBRRGRdwPK5ydBHafDWAxML/pGHZbMvKqRZ5+Abc=
golang.org/x/image v0.5.0/go.mod h1:FVC7BI/5Ym8R25iw5OLsgshdUBbT1h5jZTpA+mvAdZ4=
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91VN4djpZkiMVwK6gcyfeH4XE8wZrZaV4=
golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s=
golang.org/x/net v0.0.0-20210226172049-e18ecbb05110/go.mod h1:m0MpNAwzfU5UDzcl9v0D8zg8gWTRqZa9RBIspLL5mdg=
golang.org/x/net v0.0.0-20220722155237-a158d28d115b/go.mod h1:XRhObCWvk6IyKnWLug+ECip1KBveYUHfp+8e9klMJ9c=
golang.org/x/net v0.7.0/go.mod h1:2Tu9+aMcznHK/AK1HMvgo6xiTLG5rD5rZLDS+rp2Bjs=
golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sync v0.0.0-20220722155255-886fb9371eb4/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
golang.org/x/sys v0.0.0-20201119102817-f84b799fce68/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
golang.org/x/sys v0.0.0-20210615035016-665e8c7367d1/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220520151302-bc2c85ada10a/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.0.0-20220722155257-8c9f86f7a55f/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/sys v0.5.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k=
golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ=
golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
golang.org/x/text v0.7.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8=
golang.org/x/tools v0.0.0-20180917221912-90fa682c2a6e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ=
golang.org/x/tools v0.0.0-20191119224855-298f0cb1881e/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo=
golang.org/x/tools v0.1.12/go.mod h1:hNGJHUnrk76NpqgfD5Aqm5Crs+Hm0VOH/i9J2+nxYbc=
golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
22 changes: 11 additions & 11 deletions meta/cuesheet.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
package meta

import (
"bytes"
"encoding/binary"
"errors"
"fmt"
"io"
"io/ioutil"
"strings"
)

// A CueSheet describes how tracks are laid out within a FLAC stream.
Expand All @@ -30,12 +30,12 @@ type CueSheet struct {
func (block *Block) parseCueSheet() error {
// Parse cue sheet.
// 128 bytes: MCN.
buf, err := readBytes(block.lr, 128)
szMCN, err := readString(block.lr, 128)
if err != nil {
return unexpected(err)
}
cs := &CueSheet{
MCN: stringFromSZ(buf),
MCN: stringFromSZ(szMCN),
}
block.Body = cs

Expand Down Expand Up @@ -130,11 +130,11 @@ func (block *Block) parseTrack(cs *CueSheet, i int, uniq map[uint8]struct{}) err
}

// 12 bytes: ISRC.
buf, err := readBytes(block.lr, 12)
szISRC, err := readString(block.lr, 12)
if err != nil {
return unexpected(err)
}
track.ISRC = stringFromSZ(buf)
track.ISRC = stringFromSZ(szISRC)

// 1 bit: IsAudio.
var x uint8
Expand Down Expand Up @@ -200,14 +200,14 @@ func (block *Block) parseTrack(cs *CueSheet, i int, uniq map[uint8]struct{}) err
return nil
}

// stringFromSZ converts the provided byte slice to a string after terminating
// it at the first occurrence of a NULL character.
func stringFromSZ(buf []byte) string {
pos := bytes.IndexByte(buf, 0)
// stringFromSZ returns a copy of the given string terminated at the first
// occurrence of a NULL character.
func stringFromSZ(szStr string) string {
pos := strings.IndexByte(szStr, '\x00')
if pos == -1 {
return string(buf)
return szStr
}
return string(buf[:pos])
return string(szStr[:pos])
}

// CueSheetTrack contains the start offset of a track and other track specific
Expand Down
8 changes: 4 additions & 4 deletions meta/picture.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,23 +67,23 @@ func (block *Block) parsePicture() error {
}

// (MIME type length) bytes: MIME.
buf, err := readBytes(block.lr, int(x))
mime, err := readString(block.lr, int(x))
if err != nil {
return unexpected(err)
}
pic.MIME = string(buf)
pic.MIME = mime

// 32 bits: (description length).
if err = binary.Read(block.lr, binary.BigEndian, &x); err != nil {
return unexpected(err)
}

// (description length) bytes: Desc.
buf, err = readBytes(block.lr, int(x))
desc, err := readString(block.lr, int(x))
if err != nil {
return unexpected(err)
}
pic.Desc = string(buf)
pic.Desc = desc

// 32 bits: Width.
if err = binary.Read(block.lr, binary.BigEndian, &pic.Width); err != nil {
Expand Down
26 changes: 12 additions & 14 deletions meta/reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,23 @@ package meta

import "io"

// readBuf is the local buffer used by readBytes.
var readBuf = make([]byte, 4096)

// readBytes reads and returns exactly n bytes from the provided io.Reader. The
// local buffer is reused in between calls to reduce garbage generation. It is
// the callers responsibility to make a copy of the returned data. The error is
// io.EOF only if no bytes were read. If an io.EOF happens after reading some
// but not all the bytes, ReadFull returns io.ErrUnexpectedEOF. On return, n ==
// len(buf) if and only if err == nil.
// readString reads and returns exactly n bytes from the provided io.Reader.
//
// The local buffer is initially 4096 bytes and will grow automatically if so
// required.
func readBytes(r io.Reader, n int) ([]byte, error) {
// The error is io.EOF only if no bytes were read. If an io.EOF happens after
// reading some but not all the bytes, ReadFull returns io.ErrUnexpectedEOF. On
// return, n == len(buf) if and only if err == nil.
func readString(r io.Reader, n int) (string, error) {
// readBuf is the local buffer used by readBytes.
var backingArray [4096]byte // hopefully allocated on stack.
readBuf := backingArray[:]
if n > len(readBuf) {
// The local buffer is initially 4096 bytes and will grow automatically if
// so required.
readBuf = make([]byte, n)
}
_, err := io.ReadFull(r, readBuf[:n])
if err != nil {
return nil, err
return "", err
}
return readBuf[:n:n], nil
return string(readBuf[:n]), nil
}
7 changes: 3 additions & 4 deletions meta/vorbiscomment.go
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ func (block *Block) parseVorbisComment() (err error) {
}

// (vendor length) bits: Vendor.
buf, err := readBytes(block.lr, int(x))
vendor, err := readString(block.lr, int(x))
if err != nil {
return unexpected(err)
}
comment := new(VorbisComment)
block.Body = comment
comment.Vendor = string(buf)
comment.Vendor = vendor

// Parse tags.
// 32 bits: number of tags.
Expand All @@ -50,11 +50,10 @@ func (block *Block) parseVorbisComment() (err error) {
}

// (vector length): vector.
buf, err = readBytes(block.lr, int(x))
vector, err := readString(block.lr, int(x))
if err != nil {
return unexpected(err)
}
vector := string(buf)

// Parse tag, which has the following format:
// NAME=VALUE
Expand Down

0 comments on commit f358872

Please sign in to comment.