Skip to content

Commit 21e0305

Browse files
committed
Fix GH-10908: Bus error with PDO Firebird on RPI with 64 bit kernel and 32 bit userland
The alignment of sqldata is in most cases only the basic alignment, so the code type-puns it to a larger type, it *can* crash due to the misaligned access. This is only an issue for types > 4 bytes because every sensible system requires an alignment of at least 4 bytes for allocated data. Even though this patch uses memcpy, the compiler is smart enough to optimise it to something more efficient, especially on x86. This is just the usual approach to solve these alignment problems. Actually, unaligned memory access is undefined behaviour, so even on x86 platforms, where the bug doesn't cause a crash, this can be problematic. Furthermore, even though the issue talks about a 64-bit kernel and 32-bit userspace, this doesn't necessarily need to be the case to trigger this crash. Test was Co-authored-by: rvk01 Closes GH-10920.
1 parent e1ec67a commit 21e0305

File tree

3 files changed

+211
-11
lines changed

3 files changed

+211
-11
lines changed

NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,10 @@ PHP NEWS
4343
- OpenSSL:
4444
. Add missing error checks on file writing functions. (nielsdos)
4545

46+
- PDO Firebird:
47+
. Fixed bug GH-10908 (Bus error with PDO Firebird on RPI with 64 bit kernel
48+
and 32 bit userland). (nielsdos)
49+
4650
- PDO ODBC:
4751
. Fixed missing and inconsistent error checks on SQLAllocHandle. (nielsdos)
4852

ext/pdo_firebird/firebird_statement.c

Lines changed: 52 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,37 @@
3030

3131
#define RECORD_ERROR(stmt) _firebird_error(NULL, stmt, __FILE__, __LINE__)
3232

33+
#define READ_AND_RETURN_USING_MEMCPY(type, sqldata) do { \
34+
type ret; \
35+
memcpy(&ret, sqldata, sizeof(ret)); \
36+
return ret; \
37+
} while (0);
38+
39+
static zend_always_inline ISC_INT64 get_isc_int64_from_sqldata(const ISC_SCHAR *sqldata)
40+
{
41+
READ_AND_RETURN_USING_MEMCPY(ISC_INT64, sqldata);
42+
}
43+
44+
static zend_always_inline ISC_LONG get_isc_long_from_sqldata(const ISC_SCHAR *sqldata)
45+
{
46+
READ_AND_RETURN_USING_MEMCPY(ISC_LONG, sqldata);
47+
}
48+
49+
static zend_always_inline double get_double_from_sqldata(const ISC_SCHAR *sqldata)
50+
{
51+
READ_AND_RETURN_USING_MEMCPY(double, sqldata);
52+
}
53+
54+
static zend_always_inline ISC_TIMESTAMP get_isc_timestamp_from_sqldata(const ISC_SCHAR *sqldata)
55+
{
56+
READ_AND_RETURN_USING_MEMCPY(ISC_TIMESTAMP, sqldata);
57+
}
58+
59+
static zend_always_inline ISC_QUAD get_isc_quad_from_sqldata(const ISC_SCHAR *sqldata)
60+
{
61+
READ_AND_RETURN_USING_MEMCPY(ISC_QUAD, sqldata);
62+
}
63+
3364
/* free the allocated space for passing field values to the db and back */
3465
static void free_sqlda(XSQLDA const *sqlda) /* {{{ */
3566
{
@@ -377,18 +408,18 @@ static int firebird_stmt_get_col(
377408
n = *(short*)var->sqldata;
378409
break;
379410
case SQL_LONG:
380-
n = *(ISC_LONG*)var->sqldata;
411+
n = get_isc_long_from_sqldata(var->sqldata);
381412
break;
382413
case SQL_INT64:
383-
n = *(ISC_INT64*)var->sqldata;
414+
n = get_isc_int64_from_sqldata(var->sqldata);
384415
break;
385416
case SQL_DOUBLE:
386417
break;
387418
EMPTY_SWITCH_DEFAULT_CASE()
388419
}
389420

390421
if ((var->sqltype & ~1) == SQL_DOUBLE) {
391-
str = zend_strpprintf(0, "%.*F", -var->sqlscale, *(double*)var->sqldata);
422+
str = zend_strpprintf(0, "%.*F", -var->sqlscale, get_double_from_sqldata(var->sqldata));
392423
} else if (n >= 0) {
393424
str = zend_strpprintf(0, "%" LL_MASK "d.%0*" LL_MASK "d",
394425
n / f, -var->sqlscale, n % f);
@@ -414,13 +445,13 @@ static int firebird_stmt_get_col(
414445
ZVAL_LONG(result, *(short*)var->sqldata);
415446
break;
416447
case SQL_LONG:
417-
ZVAL_LONG(result, *(ISC_LONG*)var->sqldata);
448+
ZVAL_LONG(result, get_isc_long_from_sqldata(var->sqldata));
418449
break;
419450
case SQL_INT64:
420451
#if SIZEOF_ZEND_LONG >= 8
421-
ZVAL_LONG(result, *(ISC_INT64*)var->sqldata);
452+
ZVAL_LONG(result, get_isc_int64_from_sqldata(var->sqldata));
422453
#else
423-
ZVAL_STR(result, zend_strpprintf(0, "%" LL_MASK "d", *(ISC_INT64*)var->sqldata));
454+
ZVAL_STR(result, zend_strpprintf(0, "%" LL_MASK "d", get_isc_int64_from_sqldata(var->sqldata)));
424455
#endif
425456
break;
426457
case SQL_FLOAT:
@@ -429,7 +460,7 @@ static int firebird_stmt_get_col(
429460
break;
430461
case SQL_DOUBLE:
431462
/* TODO: Why is this not returned as the native type? */
432-
ZVAL_STR(result, zend_strpprintf(0, "%F", *(double*)var->sqldata));
463+
ZVAL_STR(result, zend_strpprintf(0, "%F", get_double_from_sqldata(var->sqldata)));
433464
break;
434465
#ifdef SQL_BOOLEAN
435466
case SQL_BOOLEAN:
@@ -445,16 +476,21 @@ static int firebird_stmt_get_col(
445476
fmt = S->H->time_format ? S->H->time_format : PDO_FB_DEF_TIME_FMT;
446477
} else if (0) {
447478
case SQL_TIMESTAMP:
448-
isc_decode_timestamp((ISC_TIMESTAMP*)var->sqldata, &t);
479+
{
480+
ISC_TIMESTAMP timestamp = get_isc_timestamp_from_sqldata(var->sqldata);
481+
isc_decode_timestamp(&timestamp, &t);
482+
}
449483
fmt = S->H->timestamp_format ? S->H->timestamp_format : PDO_FB_DEF_TIMESTAMP_FMT;
450484
}
451485
/* convert the timestamp into a string */
452486
char buf[80];
453487
size_t len = strftime(buf, sizeof(buf), fmt, &t);
454488
ZVAL_STRINGL(result, buf, len);
455489
break;
456-
case SQL_BLOB:
457-
return firebird_fetch_blob(stmt, colno, result, (ISC_QUAD*)var->sqldata);
490+
case SQL_BLOB: {
491+
ISC_QUAD quad = get_isc_quad_from_sqldata(var->sqldata);
492+
return firebird_fetch_blob(stmt, colno, result, &quad);
493+
}
458494
}
459495
}
460496
}
@@ -607,7 +643,12 @@ static int firebird_stmt_param_hook(pdo_stmt_t *stmt, struct pdo_bound_param_dat
607643
*var->sqlind = -1;
608644
return 1;
609645
}
610-
return firebird_bind_blob(stmt, (ISC_QUAD*)var->sqldata, parameter);
646+
ISC_QUAD quad = get_isc_quad_from_sqldata(var->sqldata);
647+
if (firebird_bind_blob(stmt, &quad, parameter) != 0) {
648+
memcpy(var->sqldata, &quad, sizeof(quad));
649+
return 1;
650+
}
651+
return 0;
611652
}
612653
}
613654

ext/pdo_firebird/tests/gh10908.phpt

Lines changed: 155 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,155 @@
1+
--TEST--
2+
GH-10908 (Bus error with PDO Firebird on RPI with 64 bit kernel and 32 bit userland)
3+
--EXTENSIONS--
4+
pdo_firebird
5+
--SKIPIF--
6+
<?php require('skipif.inc'); ?>
7+
--ENV--
8+
LSAN_OPTIONS=detect_leaks=0
9+
--FILE--
10+
<?php
11+
12+
require("testdb.inc");
13+
14+
$sql = <<<EOT
15+
CREATE TABLE gh10908(
16+
ID BIGINT NOT NULL,
17+
CODE VARCHAR(60) NOT NULL,
18+
NUM NUMERIC(18, 3),
19+
DBL DOUBLE PRECISION,
20+
FLT FLOAT,
21+
TS TIMESTAMP,
22+
MYDATE DATE,
23+
MYTIME TIME,
24+
MYBLOB BLOB,
25+
MYBINARY BINARY(2),
26+
MYVARBINARY VARBINARY(2),
27+
MYSMALLINT SMALLINT,
28+
MYINT INT,
29+
MYCHAR CHAR(10),
30+
MYVARCHAR VARCHAR(5),
31+
MYBOOL BOOLEAN
32+
);
33+
EOT;
34+
$dbh->exec($sql);
35+
$dbh->exec("INSERT INTO gh10908 VALUES(1, 'ABC', 12.34, 1.0, 2.0, '2023-03-24 17:39', '2023-03-24', '17:39', 'abcdefg', 'ab', 'a', 32767, 200000, 'azertyuiop', 'ab', false);");
36+
37+
function query_and_dump($dbh, $sql) {
38+
foreach ($dbh->query($sql) as $row) {
39+
print_r($row);
40+
print("\n");
41+
}
42+
}
43+
44+
query_and_dump($dbh, "SELECT CODE FROM gh10908"); // works fine
45+
query_and_dump($dbh, "SELECT ID FROM gh10908"); // Used to "bus error"
46+
query_and_dump($dbh, "SELECT NUM FROM gh10908"); // Used to "bus error"
47+
query_and_dump($dbh, "SELECT DBL FROM gh10908"); // Used to "bus error"
48+
query_and_dump($dbh, "SELECT TS FROM gh10908"); // Used to "bus error"
49+
query_and_dump($dbh, "SELECT MYBLOB FROM gh10908"); // Used to "bus error"
50+
query_and_dump($dbh, "SELECT * FROM gh10908"); // Used to "bus error"
51+
52+
query_and_dump($dbh, "SELECT CAST(NUM AS NUMERIC(9, 3)) FROM gh10908"); // works fine
53+
query_and_dump($dbh, "SELECT CAST(ID AS INTEGER) FROM gh10908"); // works fine
54+
query_and_dump($dbh, "SELECT CAST(ID AS BIGINT) FROM gh10908"); // Used to "bus error"
55+
56+
echo "Did not crash\n";
57+
58+
?>
59+
--CLEAN--
60+
<?php
61+
require 'testdb.inc';
62+
$dbh->exec("DROP TABLE gh10908");
63+
?>
64+
--EXPECT--
65+
Array
66+
(
67+
[CODE] => ABC
68+
[0] => ABC
69+
)
70+
71+
Array
72+
(
73+
[ID] => 1
74+
[0] => 1
75+
)
76+
77+
Array
78+
(
79+
[NUM] => 12.340
80+
[0] => 12.340
81+
)
82+
83+
Array
84+
(
85+
[DBL] => 1.000000
86+
[0] => 1.000000
87+
)
88+
89+
Array
90+
(
91+
[TS] => 2023-03-24 17:39:00
92+
[0] => 2023-03-24 17:39:00
93+
)
94+
95+
Array
96+
(
97+
[MYBLOB] => abcdefg
98+
[0] => abcdefg
99+
)
100+
101+
Array
102+
(
103+
[ID] => 1
104+
[0] => 1
105+
[CODE] => ABC
106+
[1] => ABC
107+
[NUM] => 12.340
108+
[2] => 12.340
109+
[DBL] => 1.000000
110+
[3] => 1.000000
111+
[FLT] => 2.000000
112+
[4] => 2.000000
113+
[TS] => 2023-03-24 17:39:00
114+
[5] => 2023-03-24 17:39:00
115+
[MYDATE] => 2023-03-24
116+
[6] => 2023-03-24
117+
[MYTIME] => 17:39:00
118+
[7] => 17:39:00
119+
[MYBLOB] => abcdefg
120+
[8] => abcdefg
121+
[MYBINARY] => ab
122+
[9] => ab
123+
[MYVARBINARY] => a
124+
[10] => a
125+
[MYSMALLINT] => 32767
126+
[11] => 32767
127+
[MYINT] => 200000
128+
[12] => 200000
129+
[MYCHAR] => azertyuiop
130+
[13] => azertyuiop
131+
[MYVARCHAR] => ab
132+
[14] => ab
133+
[MYBOOL] =>
134+
[15] =>
135+
)
136+
137+
Array
138+
(
139+
[CAST] => 12.340
140+
[0] => 12.340
141+
)
142+
143+
Array
144+
(
145+
[CAST] => 1
146+
[0] => 1
147+
)
148+
149+
Array
150+
(
151+
[CAST] => 1
152+
[0] => 1
153+
)
154+
155+
Did not crash

0 commit comments

Comments
 (0)