diff mbox series

[2/9] avdtp: Fix manipulating struct as an array

Message ID 20240702084900.773620-3-hadess@hadess.net (mailing list archive)
State Superseded
Headers show
Series Fix a number of static analysis issues #4 | expand

Checks

Context Check Description
tedd_an/pre-ci_am success Success
tedd_an/CheckPatch warning WARNING:LINE_SPACING: Missing a blank line after declarations #134: FILE: profiles/audio/avdtp.c:1684: + struct seid seid = start->seids[i]; + if (seid.seid == id) { WARNING:LINE_SPACING: Missing a blank line after declarations #150: FILE: profiles/audio/avdtp.c:1699: + struct seid seid = suspend->seids[i]; + if (seid.seid == id) { WARNING:LINE_SPACING: Missing a blank line after declarations #194: FILE: profiles/audio/avdtp.c:1916: + struct seid seid = req->seids[i]; + failed_seid = seid.seid; /github/workspace/src/src/13719119.patch total: 0 errors, 3 warnings, 105 lines checked NOTE: For some of the reported defects, checkpatch may be able to mechanically convert to the typical style using --fix or --fix-inplace. /github/workspace/src/src/13719119.patch has style problems, please review. NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPDX_LICENSE_TAG SPLIT_STRING SSCANF_TO_KSTRTO NOTE: If any of the errors are false positives, please report them to the maintainer, see CHECKPATCH in MAINTAINERS.
tedd_an/GitLint fail WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search 7: B1 Line exceeds max length (122>80): "bluez-5.76/profiles/audio/avdtp.c:1675:2: address_of: Taking address with "&start->first_seid" yields a singleton pointer." 8: B1 Line exceeds max length (91>80): "bluez-5.76/profiles/audio/avdtp.c:1675:2: assign: Assigning: "seid" = "&start->first_seid"." 9: B1 Line exceeds max length (142>80): "bluez-5.76/profiles/audio/avdtp.c:1679:25: ptr_arith: Using "seid" as an array. This might corrupt or misinterpret adjacent memory locations." 17: B1 Line exceeds max length (124>80): "bluez-5.76/profiles/audio/avdtp.c:1690:2: address_of: Taking address with "&suspend->first_seid" yields a singleton pointer." 18: B1 Line exceeds max length (93>80): "bluez-5.76/profiles/audio/avdtp.c:1690:2: assign: Assigning: "seid" = "&suspend->first_seid"." 19: B1 Line exceeds max length (142>80): "bluez-5.76/profiles/audio/avdtp.c:1694:25: ptr_arith: Using "seid" as an array. This might corrupt or misinterpret adjacent memory locations." 20: B3 Line contains hard tab characters (\t): "1692| int i;" 22: B3 Line contains hard tab characters (\t): "1694|-> for (i = 0; i < count; i++, seid++) {" 23: B3 Line contains hard tab characters (\t): "1695| if (seid->seid == id) {" 24: B3 Line contains hard tab characters (\t): "1696| req->collided = TRUE;" 27: B1 Line exceeds max length (120>80): "bluez-5.76/profiles/audio/avdtp.c:1799:2: address_of: Taking address with "&req->first_seid" yields a singleton pointer." 28: B1 Line exceeds max length (89>80): "bluez-5.76/profiles/audio/avdtp.c:1799:2: assign: Assigning: "seid" = "&req->first_seid"." 29: B1 Line exceeds max length (142>80): "bluez-5.76/profiles/audio/avdtp.c:1801:30: ptr_arith: Using "seid" as an array. This might corrupt or misinterpret adjacent memory locations." 30: B3 Line contains hard tab characters (\t): "1799| seid = &req->first_seid;" 32: B3 Line contains hard tab characters (\t): "1801|-> for (i = 0; i < seid_count; i++, seid++) {" 33: B3 Line contains hard tab characters (\t): "1802| failed_seid = seid->seid;" 37: B1 Line exceeds max length (120>80): "bluez-5.76/profiles/audio/avdtp.c:1912:2: address_of: Taking address with "&req->first_seid" yields a singleton pointer." 38: B1 Line exceeds max length (89>80): "bluez-5.76/profiles/audio/avdtp.c:1912:2: assign: Assigning: "seid" = "&req->first_seid"." 39: B1 Line exceeds max length (142>80): "bluez-5.76/profiles/audio/avdtp.c:1914:30: ptr_arith: Using "seid" as an array. This might corrupt or misinterpret adjacent memory locations." 40: B3 Line contains hard tab characters (\t): "1912| seid = &req->first_seid;" 42: B3 Line contains hard tab characters (\t): "1914|-> for (i = 0; i < seid_count; i++, seid++) {" 43: B3 Line contains hard tab characters (\t): "1915| failed_seid = seid->seid;"
tedd_an/IncrementalBuild success Incremental Build PASS

Commit Message

Bastien Nocera July 2, 2024, 8:47 a.m. UTC
Don't manipulate the "req" structs as if they were flat arrays, static
analysis and humans are both equally confused by this kind of usage.

Error: ARRAY_VS_SINGLETON (CWE-119): [#def26] [important]
bluez-5.76/profiles/audio/avdtp.c:1675:2: address_of: Taking address with "&start->first_seid" yields a singleton pointer.
bluez-5.76/profiles/audio/avdtp.c:1675:2: assign: Assigning: "seid" = "&start->first_seid".
bluez-5.76/profiles/audio/avdtp.c:1679:25: ptr_arith: Using "seid" as an array.  This might corrupt or misinterpret adjacent memory locations.
1677|           int i;
1678|
1679|->         for (i = 0; i < count; i++, seid++) {
1680|                   if (seid->seid == id) {
1681|                           req->collided = TRUE;

Error: ARRAY_VS_SINGLETON (CWE-119): [#def27] [important]
bluez-5.76/profiles/audio/avdtp.c:1690:2: address_of: Taking address with "&suspend->first_seid" yields a singleton pointer.
bluez-5.76/profiles/audio/avdtp.c:1690:2: assign: Assigning: "seid" = "&suspend->first_seid".
bluez-5.76/profiles/audio/avdtp.c:1694:25: ptr_arith: Using "seid" as an array.  This might corrupt or misinterpret adjacent memory locations.
1692|		int i;
1693|
1694|->		for (i = 0; i < count; i++, seid++) {
1695|			if (seid->seid == id) {
1696|				req->collided = TRUE;

Error: ARRAY_VS_SINGLETON (CWE-119): [#def28] [important]
bluez-5.76/profiles/audio/avdtp.c:1799:2: address_of: Taking address with "&req->first_seid" yields a singleton pointer.
bluez-5.76/profiles/audio/avdtp.c:1799:2: assign: Assigning: "seid" = "&req->first_seid".
bluez-5.76/profiles/audio/avdtp.c:1801:30: ptr_arith: Using "seid" as an array.  This might corrupt or misinterpret adjacent memory locations.
1799|		seid = &req->first_seid;
1800|
1801|->		for (i = 0; i < seid_count; i++, seid++) {
1802|			failed_seid = seid->seid;
1803|

Error: ARRAY_VS_SINGLETON (CWE-119): [#def29] [important]
bluez-5.76/profiles/audio/avdtp.c:1912:2: address_of: Taking address with "&req->first_seid" yields a singleton pointer.
bluez-5.76/profiles/audio/avdtp.c:1912:2: assign: Assigning: "seid" = "&req->first_seid".
bluez-5.76/profiles/audio/avdtp.c:1914:30: ptr_arith: Using "seid" as an array.  This might corrupt or misinterpret adjacent memory locations.
1912|		seid = &req->first_seid;
1913|
1914|->	for (i = 0; i < seid_count; i++, seid++) {
1915|			failed_seid = seid->seid;
1916|
---
 profiles/audio/avdtp.c | 45 +++++++++++++++++++++---------------------
 1 file changed, 23 insertions(+), 22 deletions(-)
diff mbox series

Patch

diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index 3667e08400dd..45d1b120b760 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -184,13 +184,17 @@  struct getcap_resp {
 } __attribute__ ((packed));
 
 struct start_req {
-	struct seid first_seid;
-	struct seid other_seids[0];
+	union {
+		struct seid required[1];
+		struct seid seids[0];
+	};
 } __attribute__ ((packed));
 
 struct suspend_req {
-	struct seid first_seid;
-	struct seid other_seids[0];
+	union {
+		struct seid required[1];
+		struct seid seids[0];
+	};
 } __attribute__ ((packed));
 
 struct seid_rej {
@@ -1672,12 +1676,12 @@  static void check_seid_collision(struct pending_req *req, uint8_t id)
 static void check_start_collision(struct pending_req *req, uint8_t id)
 {
 	struct start_req *start = req->data;
-	struct seid *seid = &start->first_seid;
 	int count = 1 + req->data_size - sizeof(struct start_req);
 	int i;
 
-	for (i = 0; i < count; i++, seid++) {
-		if (seid->seid == id) {
+	for (i = 0; i < count; i++) {
+		struct seid seid = start->seids[i];
+		if (seid.seid == id) {
 			req->collided = TRUE;
 			return;
 		}
@@ -1687,12 +1691,12 @@  static void check_start_collision(struct pending_req *req, uint8_t id)
 static void check_suspend_collision(struct pending_req *req, uint8_t id)
 {
 	struct suspend_req *suspend = req->data;
-	struct seid *seid = &suspend->first_seid;
 	int count = 1 + req->data_size - sizeof(struct suspend_req);
 	int i;
 
-	for (i = 0; i < count; i++, seid++) {
-		if (seid->seid == id) {
+	for (i = 0; i < count; i++) {
+		struct seid seid = suspend->seids[i];
+		if (seid.seid == id) {
 			req->collided = TRUE;
 			return;
 		}
@@ -1785,7 +1789,6 @@  static gboolean avdtp_start_cmd(struct avdtp *session, uint8_t transaction,
 	struct avdtp_local_sep *sep;
 	struct avdtp_stream *stream;
 	struct stream_rej rej;
-	struct seid *seid;
 	uint8_t err, failed_seid;
 	int seid_count, i;
 
@@ -1796,12 +1799,12 @@  static gboolean avdtp_start_cmd(struct avdtp *session, uint8_t transaction,
 
 	seid_count = 1 + size - sizeof(struct start_req);
 
-	seid = &req->first_seid;
+	for (i = 0; i < seid_count; i++) {
+		struct seid seid = req->seids[i];
 
-	for (i = 0; i < seid_count; i++, seid++) {
-		failed_seid = seid->seid;
+		failed_seid = seid.seid;
 
-		sep = find_local_sep_by_seid(session, seid->seid);
+		sep = find_local_sep_by_seid(session, seid.seid);
 		if (!sep || !sep->stream) {
 			err = AVDTP_BAD_ACP_SEID;
 			goto failed;
@@ -1898,7 +1901,6 @@  static gboolean avdtp_suspend_cmd(struct avdtp *session, uint8_t transaction,
 	struct avdtp_local_sep *sep;
 	struct avdtp_stream *stream;
 	struct stream_rej rej;
-	struct seid *seid;
 	uint8_t err, failed_seid;
 	int seid_count, i;
 
@@ -1909,12 +1911,11 @@  static gboolean avdtp_suspend_cmd(struct avdtp *session, uint8_t transaction,
 
 	seid_count = 1 + size - sizeof(struct suspend_req);
 
-	seid = &req->first_seid;
+	for (i = 0; i < seid_count; i++) {
+		struct seid seid = req->seids[i];
+		failed_seid = seid.seid;
 
-	for (i = 0; i < seid_count; i++, seid++) {
-		failed_seid = seid->seid;
-
-		sep = find_local_sep_by_seid(session, seid->seid);
+		sep = find_local_sep_by_seid(session, seid.seid);
 		if (!sep || !sep->stream) {
 			err = AVDTP_BAD_ACP_SEID;
 			goto failed;
@@ -3663,7 +3664,7 @@  int avdtp_start(struct avdtp *session, struct avdtp_stream *stream)
 	}
 
 	memset(&req, 0, sizeof(req));
-	req.first_seid.seid = stream->rseid;
+	req.required->seid = stream->rseid;
 
 	ret = send_request(session, FALSE, stream, AVDTP_START,
 							&req, sizeof(req));