From a486776a445bf39a51cb9f93d4c73601329c0c7b Mon Sep 17 00:00:00 2001 From: Dan Williams Date: Fri, 9 Jan 2026 08:45:19 -0600 Subject: [PATCH] intel: XMM7360: better handle negative ASN integers from modem 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 --- .../intel/mm-xmmrpc-xmm7360-protocol.c | 68 +++++++++++++------ .../tests/test-xmmrpc-xmm7360-protocol.c | 61 +++++++++++++++++ 2 files changed, 108 insertions(+), 21 deletions(-) diff --git a/src/plugins/intel/mm-xmmrpc-xmm7360-protocol.c b/src/plugins/intel/mm-xmmrpc-xmm7360-protocol.c index e4440e0a..dcf16280 100644 --- a/src/plugins/intel/mm-xmmrpc-xmm7360-protocol.c +++ b/src/plugins/intel/mm-xmmrpc-xmm7360-protocol.c @@ -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; diff --git a/src/plugins/intel/tests/test-xmmrpc-xmm7360-protocol.c b/src/plugins/intel/tests/test-xmmrpc-xmm7360-protocol.c index 2de796cf..9b1b8f87 100644 --- a/src/plugins/intel/tests/test-xmmrpc-xmm7360-protocol.c +++ b/src/plugins/intel/tests/test-xmmrpc-xmm7360-protocol.c @@ -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);