SymbX/ModemManager
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

intel: XMM7360: better handle negative ASN integers from modem

Browse Source 
The original code for xmm7360_byte_array_read_asn_int() returned
signed integers both from the function and from the returned
Xmm7360RpcMsgArg, but didn't handle sign extension on the
value returned from the function. That would lead to different
values between the function return and the Xmm7360RpcMsgArg
value, which seems wrong.

Make those two correspond and add checks in various places that
shouldn't ever see negative ints.

Signed-off-by: Dan Williams <dan@ioncontrol.co>
This commit is contained in:
Dan Williams
2026-05-24 08:33:48 -05:00
parent d6fe0722c9
commit a486776a44
2 changed files with 108 additions and 21 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/plugins/intel/mm-xmmrpc-xmm7360-protocol.c
+47 -21
View File
@@ -61,9 +61,10 @@ xmm7360_byte_array_read_asn_int (GByteArray *buf,
GError **error)
{
const gsize orig_offset = offset;
gint size;
gint bytes_read;
gint val;
gint size;
gint bytes_read;
gint val;
Xmm7360RpcMsgArg arg = { 0 };
if (buf->len <= offset + 2) {
PARSE_ERROR ("initial buffer size %u too small (need %zu)",
@@ -94,26 +95,37 @@ xmm7360_byte_array_read_asn_int (GByteArray *buf,
val |= buf->data[offset++];
}
if (out_arg) {
if (size == 0x00) {
out_arg->type = XMM7360_RPC_MSG_ARG_TYPE_UNKNOWN;
} else if (size == 0x01) {
out_arg->type = XMM7360_RPC_MSG_ARG_TYPE_BYTE;
out_arg->value.b = (gint8) val;
} else if (size == 0x02) {
out_arg->type = XMM7360_RPC_MSG_ARG_TYPE_SHORT;
out_arg->value.s = (gint16) val;
} else if (size == 0x04) {
out_arg->type = XMM7360_RPC_MSG_ARG_TYPE_LONG;
out_arg->value.l = (gint32) val;
} else {
/* size is validated above */
g_assert_not_reached ();
}
if (size == 0x00) {
arg.type = XMM7360_RPC_MSG_ARG_TYPE_UNKNOWN;
} else if (size == 0x01) {
arg.type = XMM7360_RPC_MSG_ARG_TYPE_BYTE;
arg.value.b = (gint8) val;
} else if (size == 0x02) {
arg.type = XMM7360_RPC_MSG_ARG_TYPE_SHORT;
arg.value.s = (gint16) val;
} else if (size == 0x04) {
arg.type = XMM7360_RPC_MSG_ARG_TYPE_LONG;
arg.value.l = (gint32) val;
} else {
/* size is validated above */
g_assert_not_reached ();
}
if (out_val)
*out_val = val;
if (out_arg)
*out_arg = arg;
if (out_val) {
/* If the ASN int is negative, we must convert a int8/int16
* value to the appropriate u32 value to avoid returning a
* positive 32-bit gint.
*/
if ((arg.type == XMM7360_RPC_MSG_ARG_TYPE_BYTE) && (arg.value.b < 0))
*out_val = (arg.value.b << 24) >> 24;
else if ((arg.type == XMM7360_RPC_MSG_ARG_TYPE_SHORT) && (arg.value.s < 0))
*out_val = (arg.value.s << 16) >> 16;
else
*out_val = arg.value.l;
}
/* return bytes consumed by this function */
return offset - orig_offset;
@@ -175,11 +187,19 @@ xmm7360_byte_array_read_string (GByteArray *buf,
consumed = xmm7360_byte_array_read_asn_int (buf, offset, &string_len_padded, NULL, error);
if (consumed < 0)
return -1;
else if (string_len_padded < 0) {
PARSE_ERROR ("string length padded should be >= 0 (got %d)", string_len_padded);
return -1;
}
offset += consumed;
consumed = xmm7360_byte_array_read_asn_int (buf, offset, &pad_len, NULL, error);
if (consumed < 0)
return -1;
else if (pad_len < 0) {
PARSE_ERROR ("pad length should be >= 0 (got %d)", pad_len);
return -1;
}
offset += consumed;
if (string_len_padded > 0) {
@@ -490,6 +510,9 @@ xmm7360_parse_response (GByteArray *buf, GError **error)
if (consumed < 0) {
g_prefix_error (error, "error parsing RPC message: error reading ASN message size: ");
return NULL;
} else if (asn_msg_len < 0) {
PARSE_ERROR ("ASN message size should be >= 0 (got %d)", asn_msg_len);
return NULL;
}
offset += consumed;
/* Make sure the ASN message size and message size agree */
@@ -503,6 +526,9 @@ xmm7360_parse_response (GByteArray *buf, GError **error)
if (consumed < 0) {
g_prefix_error (error, "error parsing RPC message: error reading unsolicited message id: ");
return NULL;
} else if (unsol_id < 0) {
PARSE_ERROR ("Unsolicited message ID should be >= 0 (got %d)", unsol_id);
return NULL;
}
offset += consumed;
src/plugins/intel/tests/test-xmmrpc-xmm7360-protocol.c
+61
View File
@@ -262,6 +262,66 @@ static const ReadAsnIntTest read_asn_int_tests[] = {
.expected_error_string = "unhandled int size 6",
XMM_BUFFER( 0x02, 0x06, 0x01, 0x02, 0x03, 0x04, 0x05, 0x06 )
},
{
.detail = "u8_negative_max",
.expected_consumed = 3,
.expected_val = -128,
XMM_BUFFER( 0x02, 0x01, 0x80 )
.expected_arg = {
.type = XMM7360_RPC_MSG_ARG_TYPE_BYTE,
.value.b = -128,
},
},
{
.detail = "u16_negative_max",
.expected_consumed = 4,
.expected_val = -32768,
XMM_BUFFER( 0x02, 0x02, 0x80, 0x00 )
.expected_arg = {
.type = XMM7360_RPC_MSG_ARG_TYPE_SHORT,
.value.s = -32768,
},
},
{
.detail = "u32_negative_max",
.expected_consumed = 6,
.expected_val = -2147483648,
XMM_BUFFER( 0x02, 0x04, 0x80, 0x00, 0x00, 0x00 )
.expected_arg = {
.type = XMM7360_RPC_MSG_ARG_TYPE_LONG,
.value.l = -2147483648,
},
},
{
.detail = "u8_negative_min",
.expected_consumed = 3,
.expected_val = -1,
XMM_BUFFER( 0x02, 0x01, 0xFF )
.expected_arg = {
.type = XMM7360_RPC_MSG_ARG_TYPE_BYTE,
.value.b = -1,
},
},
{
.detail = "u16_negative_min",
.expected_consumed = 4,
.expected_val = -1,
XMM_BUFFER( 0x02, 0x02, 0xFF, 0xFF )
.expected_arg = {
.type = XMM7360_RPC_MSG_ARG_TYPE_SHORT,
.value.s = -1,
},
},
{
.detail = "u32_negative_min",
.expected_consumed = 6,
.expected_val = -1,
XMM_BUFFER( 0x02, 0x04, 0xFF, 0xFF, 0xFF, 0xFF )
.expected_arg = {
.type = XMM7360_RPC_MSG_ARG_TYPE_LONG,
.value.l = -1,
},
},
};
static void
@@ -289,6 +349,7 @@ test_read_asn_int (gconstpointer data)
g_assert_cmpint (test->expected_error, ==, 0);
g_assert_false (!!test->expected_error_string);
g_assert_cmpint (consumed, ==, test->expected_consumed);
g_message ("v = %d 0x%x\n", val, val);
g_assert_cmpint (val, ==, test->expected_val);
if (test->expected_arg.type != XMM7360_RPC_MSG_ARG_TYPE_UNKNOWN) {
g_assert_cmpint (arg.type, ==, test->expected_arg.type);