1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
|
From 298fe517a9333c05143a8a8e1f9d5499f0c6e59b Mon Sep 17 00:00:00 2001
From: Brad Fitzpatrick <bradfitz@golang.org>
Date: Tue, 23 May 2023 15:12:47 -0700
Subject: [PATCH] database/sql: make RawBytes safely usable with contexts
sql.RawBytes was added the very first Go release, Go 1. Its docs
say:
> RawBytes is a byte slice that holds a reference to memory owned by
> the database itself. After a Scan into a RawBytes, the slice is only
> valid until the next call to Next, Scan, or Close.
That "only valid until the next call" bit was true at the time,
until contexts were added to database/sql in Go 1.8.
In the past ~dozen releases it's been unsafe to use QueryContext with
a context that might become Done to get an *sql.Rows that's scanning
into a RawBytes. The Scan can succeed, but then while the caller's
reading the memory, a database/sql-managed goroutine can see the
context becoming done and call Close on the database/sql/driver and
make the caller's view of the RawBytes memory no longer valid,
introducing races, crashes, or database corruption. See #60304
and #53970 for details.
This change does the minimal surgery on database/sql to make it safe
again: Rows.Scan was already acquiring a mutex to check whether the
rows had been closed, so this change make Rows.Scan notice whether
*RawBytes was used and, if so, doesn't release the mutex on exit
before returning. That mean it's still locked while the user code
operates on the RawBytes memory and the concurrent context-watching
goroutine to close the database still runs, but if it fires, it then
gets blocked on the mutex until the next call to a Rows method (Next,
NextResultSet, Err, Close).
Updates #60304
Updates #53970 (earlier one I'd missed)
Change-Id: Ie41c0c6f32c24887b2f53ec3686c2aab73a1bfff
Reviewed-on: https://go-review.googlesource.com/c/go/+/497675
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Reviewed-by: Russ Cox <rsc@golang.org>
CVE: CVE-2025-47907
Upstream-Status: Backport [https://github.com/golang/go/commit/298fe517a9333c05143a8a8e1f9d5499f0c6e59b]
Signed-off-by: Praveen Kumar <praveen.kumar@windriver.com>
---
src/database/sql/fakedb_test.go | 13 +++++-
src/database/sql/sql.go | 72 ++++++++++++++++++++++++++++++++-
src/database/sql/sql_test.go | 58 ++++++++++++++++++++++++++
3 files changed, 141 insertions(+), 2 deletions(-)
diff --git a/src/database/sql/fakedb_test.go b/src/database/sql/fakedb_test.go
index 4b68f1c..33c57b9 100644
--- a/src/database/sql/fakedb_test.go
+++ b/src/database/sql/fakedb_test.go
@@ -15,6 +15,7 @@ import (
"strconv"
"strings"
"sync"
+ "sync/atomic"
"testing"
"time"
)
@@ -90,6 +91,8 @@ func (cc *fakeDriverCtx) OpenConnector(name string) (driver.Connector, error) {
type fakeDB struct {
name string
+ useRawBytes atomic.Bool
+
mu sync.Mutex
tables map[string]*table
badConn bool
@@ -680,6 +683,8 @@ func (c *fakeConn) PrepareContext(ctx context.Context, query string) (driver.Stm
switch cmd {
case "WIPE":
// Nothing
+ case "USE_RAWBYTES":
+ c.db.useRawBytes.Store(true)
case "SELECT":
stmt, err = c.prepareSelect(stmt, parts)
case "CREATE":
@@ -783,6 +788,9 @@ func (s *fakeStmt) ExecContext(ctx context.Context, args []driver.NamedValue) (d
case "WIPE":
db.wipe()
return driver.ResultNoRows, nil
+ case "USE_RAWBYTES":
+ s.c.db.useRawBytes.Store(true)
+ return driver.ResultNoRows, nil
case "CREATE":
if err := db.createTable(s.table, s.colName, s.colType); err != nil {
return nil, err
@@ -912,6 +920,7 @@ func (s *fakeStmt) QueryContext(ctx context.Context, args []driver.NamedValue) (
txStatus = "transaction"
}
cursor := &rowsCursor{
+ db: s.c.db,
parentMem: s.c,
posRow: -1,
rows: [][]*row{
@@ -1008,6 +1017,7 @@ func (s *fakeStmt) QueryContext(ctx context.Context, args []driver.NamedValue) (
}
cursor := &rowsCursor{
+ db: s.c.db,
parentMem: s.c,
posRow: -1,
rows: setMRows,
@@ -1050,6 +1060,7 @@ func (tx *fakeTx) Rollback() error {
}
type rowsCursor struct {
+ db *fakeDB
parentMem memToucher
cols [][]string
colType [][]string
@@ -1121,7 +1132,7 @@ func (rc *rowsCursor) Next(dest []driver.Value) error {
// messing up conversions or doing them differently.
dest[i] = v
- if bs, ok := v.([]byte); ok {
+ if bs, ok := v.([]byte); ok && !rc.db.useRawBytes.Load() {
if rc.bytesClone == nil {
rc.bytesClone = make(map[*byte][]byte)
}
diff --git a/src/database/sql/sql.go b/src/database/sql/sql.go
index 68fb392..ef49e70 100644
--- a/src/database/sql/sql.go
+++ b/src/database/sql/sql.go
@@ -2879,6 +2879,8 @@ type Rows struct {
cancel func() // called when Rows is closed, may be nil.
closeStmt *driverStmt // if non-nil, statement to Close on close
+ contextDone atomic.Value // error that awaitDone saw; set before close attempt
+
// closemu prevents Rows from closing while there
// is an active streaming result. It is held for read during non-close operations
// and exclusively during close.
@@ -2891,6 +2893,15 @@ type Rows struct {
// lastcols is only used in Scan, Next, and NextResultSet which are expected
// not to be called concurrently.
lastcols []driver.Value
+
+ // closemuScanHold is whether the previous call to Scan kept closemu RLock'ed
+ // without unlocking it. It does that when the user passes a *RawBytes scan
+ // target. In that case, we need to prevent awaitDone from closing the Rows
+ // while the user's still using the memory. See go.dev/issue/60304.
+ //
+ // It is only used by Scan, Next, and NextResultSet which are expected
+ // not to be called concurrently.
+ closemuScanHold bool
}
// lasterrOrErrLocked returns either lasterr or the provided err.
@@ -2928,7 +2939,11 @@ func (rs *Rows) awaitDone(ctx, txctx context.Context) {
}
select {
case <-ctx.Done():
+ err := ctx.Err()
+ rs.contextDone.Store(&err)
case <-txctxDone:
+ err := txctx.Err()
+ rs.contextDone.Store(&err)
}
rs.close(ctx.Err())
}
@@ -2940,6 +2955,15 @@ func (rs *Rows) awaitDone(ctx, txctx context.Context) {
//
// Every call to Scan, even the first one, must be preceded by a call to Next.
func (rs *Rows) Next() bool {
+ // If the user's calling Next, they're done with their previous row's Scan
+ // results (any RawBytes memory), so we can release the read lock that would
+ // be preventing awaitDone from calling close.
+ rs.closemuRUnlockIfHeldByScan()
+
+ if rs.contextDone.Load() != nil {
+ return false
+ }
+
var doClose, ok bool
withLock(rs.closemu.RLocker(), func() {
doClose, ok = rs.nextLocked()
@@ -2994,6 +3018,11 @@ func (rs *Rows) nextLocked() (doClose, ok bool) {
// scanning. If there are further result sets they may not have rows in the result
// set.
func (rs *Rows) NextResultSet() bool {
+ // If the user's calling NextResultSet, they're done with their previous
+ // row's Scan results (any RawBytes memory), so we can release the read lock
+ // that would be preventing awaitDone from calling close.
+ rs.closemuRUnlockIfHeldByScan()
+
var doClose bool
defer func() {
if doClose {
@@ -3030,6 +3059,10 @@ func (rs *Rows) NextResultSet() bool {
// Err returns the error, if any, that was encountered during iteration.
// Err may be called after an explicit or implicit Close.
func (rs *Rows) Err() error {
+ if errp := rs.contextDone.Load(); errp != nil {
+ return *(errp.(*error))
+ }
+
rs.closemu.RLock()
defer rs.closemu.RUnlock()
return rs.lasterrOrErrLocked(nil)
@@ -3223,6 +3256,11 @@ func rowsColumnInfoSetupConnLocked(rowsi driver.Rows) []*ColumnType {
// If any of the first arguments implementing Scanner returns an error,
// that error will be wrapped in the returned error
func (rs *Rows) Scan(dest ...interface{}) error {
+ if rs.closemuScanHold {
+ // This should only be possible if the user calls Scan twice in a row
+ // without calling Next.
+ return fmt.Errorf("sql: Scan called without calling Next (closemuScanHold)")
+ }
rs.closemu.RLock()
if rs.lasterr != nil && rs.lasterr != io.EOF {
@@ -3234,23 +3272,50 @@ func (rs *Rows) Scan(dest ...interface{}) error {
rs.closemu.RUnlock()
return err
}
- rs.closemu.RUnlock()
+
+ if scanArgsContainRawBytes(dest) {
+ rs.closemuScanHold = true
+ } else {
+ rs.closemu.RUnlock()
+ }
if rs.lastcols == nil {
+ rs.closemuRUnlockIfHeldByScan()
return errors.New("sql: Scan called without calling Next")
}
if len(dest) != len(rs.lastcols) {
+ rs.closemuRUnlockIfHeldByScan()
return fmt.Errorf("sql: expected %d destination arguments in Scan, not %d", len(rs.lastcols), len(dest))
}
+
for i, sv := range rs.lastcols {
err := convertAssignRows(dest[i], sv, rs)
if err != nil {
+ rs.closemuRUnlockIfHeldByScan()
return fmt.Errorf(`sql: Scan error on column index %d, name %q: %w`, i, rs.rowsi.Columns()[i], err)
}
}
return nil
}
+// closemuRUnlockIfHeldByScan releases any closemu.RLock held open by a previous
+// call to Scan with *RawBytes.
+func (rs *Rows) closemuRUnlockIfHeldByScan() {
+ if rs.closemuScanHold {
+ rs.closemuScanHold = false
+ rs.closemu.RUnlock()
+ }
+}
+
+func scanArgsContainRawBytes(args []interface{}) bool {
+ for _, a := range args {
+ if _, ok := a.(*RawBytes); ok {
+ return true
+ }
+ }
+ return false
+}
+
// rowsCloseHook returns a function so tests may install the
// hook through a test only mutex.
var rowsCloseHook = func() func(*Rows, *error) { return nil }
@@ -3260,6 +3325,11 @@ var rowsCloseHook = func() func(*Rows, *error) { return nil }
// the Rows are closed automatically and it will suffice to check the
// result of Err. Close is idempotent and does not affect the result of Err.
func (rs *Rows) Close() error {
+ // If the user's calling Close, they're done with their previous row's Scan
+ // results (any RawBytes memory), so we can release the read lock that would
+ // be preventing awaitDone from calling the unexported close before we do so.
+ rs.closemuRUnlockIfHeldByScan()
+
return rs.close(nil)
}
diff --git a/src/database/sql/sql_test.go b/src/database/sql/sql_test.go
index f771dee..53b38d1 100644
--- a/src/database/sql/sql_test.go
+++ b/src/database/sql/sql_test.go
@@ -4255,6 +4255,64 @@ func TestRowsScanProperlyWrapsErrors(t *testing.T) {
}
}
+// From go.dev/issue/60304
+func TestContextCancelDuringRawBytesScan(t *testing.T) {
+ db := newTestDB(t, "people")
+ defer closeDB(t, db)
+
+ if _, err := db.Exec("USE_RAWBYTES"); err != nil {
+ t.Fatal(err)
+ }
+
+ ctx, cancel := context.WithCancel(context.Background())
+ defer cancel()
+
+ r, err := db.QueryContext(ctx, "SELECT|people|name|")
+ if err != nil {
+ t.Fatal(err)
+ }
+ numRows := 0
+ var sink byte
+ for r.Next() {
+ numRows++
+ var s RawBytes
+ err = r.Scan(&s)
+ if !r.closemuScanHold {
+ t.Errorf("expected closemu to be held")
+ }
+ if err != nil {
+ t.Fatal(err)
+ }
+ t.Logf("read %q", s)
+ if numRows == 2 {
+ cancel() // invalidate the context, which used to call close asynchronously
+ }
+ for _, b := range s { // some operation reading from the raw memory
+ sink += b
+ }
+ }
+ if r.closemuScanHold {
+ t.Errorf("closemu held; should not be")
+ }
+
+ // There are 3 rows. We canceled after reading 2 so we expect either
+ // 2 or 3 depending on how the awaitDone goroutine schedules.
+ switch numRows {
+ case 0, 1:
+ t.Errorf("got %d rows; want 2+", numRows)
+ case 2:
+ if err := r.Err(); err != context.Canceled {
+ t.Errorf("unexpected error: %v (%T)", err, err)
+ }
+ default:
+ // Made it to the end. This is rare, but fine. Permit it.
+ }
+
+ if err := r.Close(); err != nil {
+ t.Fatal(err)
+ }
+}
+
// badConn implements a bad driver.Conn, for TestBadDriver.
// The Exec method panics.
type badConn struct{}
|