Skip to content

Commit

Permalink
Fix GH-10908: Bus error with PDO Firebird on RPI with 64 bit kernel a…
Browse files Browse the repository at this point in the history
…nd 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.
  • Loading branch information
nielsdos committed Mar 27, 2023
1 parent e1ec67a commit 21e0305
Show file tree
Hide file tree
Showing 3 changed files with 211 additions and 11 deletions.
4 changes: 4 additions & 0 deletions NEWS
Expand Up @@ -43,6 +43,10 @@ PHP NEWS
- OpenSSL:
. Add missing error checks on file writing functions. (nielsdos)

- PDO Firebird:
. Fixed bug GH-10908 (Bus error with PDO Firebird on RPI with 64 bit kernel
and 32 bit userland). (nielsdos)

- PDO ODBC:
. Fixed missing and inconsistent error checks on SQLAllocHandle. (nielsdos)

Expand Down
63 changes: 52 additions & 11 deletions ext/pdo_firebird/firebird_statement.c
Expand Up @@ -30,6 +30,37 @@

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

#define READ_AND_RETURN_USING_MEMCPY(type, sqldata) do { \
type ret; \
memcpy(&ret, sqldata, sizeof(ret)); \
return ret; \
} while (0);

static zend_always_inline ISC_INT64 get_isc_int64_from_sqldata(const ISC_SCHAR *sqldata)
{
READ_AND_RETURN_USING_MEMCPY(ISC_INT64, sqldata);
}

static zend_always_inline ISC_LONG get_isc_long_from_sqldata(const ISC_SCHAR *sqldata)
{
READ_AND_RETURN_USING_MEMCPY(ISC_LONG, sqldata);
}

static zend_always_inline double get_double_from_sqldata(const ISC_SCHAR *sqldata)
{
READ_AND_RETURN_USING_MEMCPY(double, sqldata);
}

static zend_always_inline ISC_TIMESTAMP get_isc_timestamp_from_sqldata(const ISC_SCHAR *sqldata)
{
READ_AND_RETURN_USING_MEMCPY(ISC_TIMESTAMP, sqldata);
}

static zend_always_inline ISC_QUAD get_isc_quad_from_sqldata(const ISC_SCHAR *sqldata)
{
READ_AND_RETURN_USING_MEMCPY(ISC_QUAD, sqldata);
}

/* free the allocated space for passing field values to the db and back */
static void free_sqlda(XSQLDA const *sqlda) /* {{{ */
{
Expand Down Expand Up @@ -377,18 +408,18 @@ static int firebird_stmt_get_col(
n = *(short*)var->sqldata;
break;
case SQL_LONG:
n = *(ISC_LONG*)var->sqldata;
n = get_isc_long_from_sqldata(var->sqldata);
break;
case SQL_INT64:
n = *(ISC_INT64*)var->sqldata;
n = get_isc_int64_from_sqldata(var->sqldata);
break;
case SQL_DOUBLE:
break;
EMPTY_SWITCH_DEFAULT_CASE()
}

if ((var->sqltype & ~1) == SQL_DOUBLE) {
str = zend_strpprintf(0, "%.*F", -var->sqlscale, *(double*)var->sqldata);
str = zend_strpprintf(0, "%.*F", -var->sqlscale, get_double_from_sqldata(var->sqldata));
} else if (n >= 0) {
str = zend_strpprintf(0, "%" LL_MASK "d.%0*" LL_MASK "d",
n / f, -var->sqlscale, n % f);
Expand All @@ -414,13 +445,13 @@ static int firebird_stmt_get_col(
ZVAL_LONG(result, *(short*)var->sqldata);
break;
case SQL_LONG:
ZVAL_LONG(result, *(ISC_LONG*)var->sqldata);
ZVAL_LONG(result, get_isc_long_from_sqldata(var->sqldata));
break;
case SQL_INT64:
#if SIZEOF_ZEND_LONG >= 8
ZVAL_LONG(result, *(ISC_INT64*)var->sqldata);
ZVAL_LONG(result, get_isc_int64_from_sqldata(var->sqldata));
#else
ZVAL_STR(result, zend_strpprintf(0, "%" LL_MASK "d", *(ISC_INT64*)var->sqldata));
ZVAL_STR(result, zend_strpprintf(0, "%" LL_MASK "d", get_isc_int64_from_sqldata(var->sqldata)));
#endif
break;
case SQL_FLOAT:
Expand All @@ -429,7 +460,7 @@ static int firebird_stmt_get_col(
break;
case SQL_DOUBLE:
/* TODO: Why is this not returned as the native type? */
ZVAL_STR(result, zend_strpprintf(0, "%F", *(double*)var->sqldata));
ZVAL_STR(result, zend_strpprintf(0, "%F", get_double_from_sqldata(var->sqldata)));
break;
#ifdef SQL_BOOLEAN
case SQL_BOOLEAN:
Expand All @@ -445,16 +476,21 @@ static int firebird_stmt_get_col(
fmt = S->H->time_format ? S->H->time_format : PDO_FB_DEF_TIME_FMT;
} else if (0) {
case SQL_TIMESTAMP:
isc_decode_timestamp((ISC_TIMESTAMP*)var->sqldata, &t);
{
ISC_TIMESTAMP timestamp = get_isc_timestamp_from_sqldata(var->sqldata);
isc_decode_timestamp(&timestamp, &t);
}
fmt = S->H->timestamp_format ? S->H->timestamp_format : PDO_FB_DEF_TIMESTAMP_FMT;
}
/* convert the timestamp into a string */
char buf[80];
size_t len = strftime(buf, sizeof(buf), fmt, &t);
ZVAL_STRINGL(result, buf, len);
break;
case SQL_BLOB:
return firebird_fetch_blob(stmt, colno, result, (ISC_QUAD*)var->sqldata);
case SQL_BLOB: {
ISC_QUAD quad = get_isc_quad_from_sqldata(var->sqldata);
return firebird_fetch_blob(stmt, colno, result, &quad);
}
}
}
}
Expand Down Expand Up @@ -607,7 +643,12 @@ static int firebird_stmt_param_hook(pdo_stmt_t *stmt, struct pdo_bound_param_dat
*var->sqlind = -1;
return 1;
}
return firebird_bind_blob(stmt, (ISC_QUAD*)var->sqldata, parameter);
ISC_QUAD quad = get_isc_quad_from_sqldata(var->sqldata);
if (firebird_bind_blob(stmt, &quad, parameter) != 0) {
memcpy(var->sqldata, &quad, sizeof(quad));
return 1;
}
return 0;
}
}

Expand Down
155 changes: 155 additions & 0 deletions ext/pdo_firebird/tests/gh10908.phpt
@@ -0,0 +1,155 @@
--TEST--
GH-10908 (Bus error with PDO Firebird on RPI with 64 bit kernel and 32 bit userland)
--EXTENSIONS--
pdo_firebird
--SKIPIF--
<?php require('skipif.inc'); ?>
--ENV--
LSAN_OPTIONS=detect_leaks=0
--FILE--
<?php

require("testdb.inc");

$sql = <<<EOT
CREATE TABLE gh10908(
ID BIGINT NOT NULL,
CODE VARCHAR(60) NOT NULL,
NUM NUMERIC(18, 3),
DBL DOUBLE PRECISION,
FLT FLOAT,
TS TIMESTAMP,
MYDATE DATE,
MYTIME TIME,
MYBLOB BLOB,
MYBINARY BINARY(2),
MYVARBINARY VARBINARY(2),
MYSMALLINT SMALLINT,
MYINT INT,
MYCHAR CHAR(10),
MYVARCHAR VARCHAR(5),
MYBOOL BOOLEAN
);
EOT;
$dbh->exec($sql);
$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);");

function query_and_dump($dbh, $sql) {
foreach ($dbh->query($sql) as $row) {
print_r($row);
print("\n");
}
}

query_and_dump($dbh, "SELECT CODE FROM gh10908"); // works fine
query_and_dump($dbh, "SELECT ID FROM gh10908"); // Used to "bus error"
query_and_dump($dbh, "SELECT NUM FROM gh10908"); // Used to "bus error"
query_and_dump($dbh, "SELECT DBL FROM gh10908"); // Used to "bus error"
query_and_dump($dbh, "SELECT TS FROM gh10908"); // Used to "bus error"
query_and_dump($dbh, "SELECT MYBLOB FROM gh10908"); // Used to "bus error"
query_and_dump($dbh, "SELECT * FROM gh10908"); // Used to "bus error"

query_and_dump($dbh, "SELECT CAST(NUM AS NUMERIC(9, 3)) FROM gh10908"); // works fine
query_and_dump($dbh, "SELECT CAST(ID AS INTEGER) FROM gh10908"); // works fine
query_and_dump($dbh, "SELECT CAST(ID AS BIGINT) FROM gh10908"); // Used to "bus error"

echo "Did not crash\n";

?>
--CLEAN--
<?php
require 'testdb.inc';
$dbh->exec("DROP TABLE gh10908");
?>
--EXPECT--
Array
(
[CODE] => ABC
[0] => ABC
)

Array
(
[ID] => 1
[0] => 1
)

Array
(
[NUM] => 12.340
[0] => 12.340
)

Array
(
[DBL] => 1.000000
[0] => 1.000000
)

Array
(
[TS] => 2023-03-24 17:39:00
[0] => 2023-03-24 17:39:00
)

Array
(
[MYBLOB] => abcdefg
[0] => abcdefg
)

Array
(
[ID] => 1
[0] => 1
[CODE] => ABC
[1] => ABC
[NUM] => 12.340
[2] => 12.340
[DBL] => 1.000000
[3] => 1.000000
[FLT] => 2.000000
[4] => 2.000000
[TS] => 2023-03-24 17:39:00
[5] => 2023-03-24 17:39:00
[MYDATE] => 2023-03-24
[6] => 2023-03-24
[MYTIME] => 17:39:00
[7] => 17:39:00
[MYBLOB] => abcdefg
[8] => abcdefg
[MYBINARY] => ab
[9] => ab
[MYVARBINARY] => a
[10] => a
[MYSMALLINT] => 32767
[11] => 32767
[MYINT] => 200000
[12] => 200000
[MYCHAR] => azertyuiop
[13] => azertyuiop
[MYVARCHAR] => ab
[14] => ab
[MYBOOL] =>
[15] =>
)

Array
(
[CAST] => 12.340
[0] => 12.340
)

Array
(
[CAST] => 1
[0] => 1
)

Array
(
[CAST] => 1
[0] => 1
)

Did not crash

0 comments on commit 21e0305

Please sign in to comment.