2022-01-07 21:32:20

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: [PATCH BlueZ] avdtp: Fix runtime errors passing NULL to memcpy

From: Luiz Augusto von Dentz <[email protected]>

Passing NULL to memcpy is considered undefined behavior which leads to
the following runtime errors:

profiles/audio/avdtp.c:2709:2: runtime error: null pointer passed as
argument 1, which is declared to never be null
profiles/audio/avdtp.c:2709:2: runtime error: null pointer passed as
argument 2, which is declared to never be null
profiles/audio/avdtp.c:3326:2: runtime error: null pointer passed as
argument 2, which is declared to never be null
profiles/audio/avdtp.c:500:3: runtime error: null pointer passed as
argument 2, which is declared to never be null
---
profiles/audio/avdtp.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/profiles/audio/avdtp.c b/profiles/audio/avdtp.c
index f2b461330..da4114e0f 100644
--- a/profiles/audio/avdtp.c
+++ b/profiles/audio/avdtp.c
@@ -497,7 +497,9 @@ static gboolean avdtp_send(struct avdtp *session, uint8_t transaction,
single.signal_id = signal_id;

memcpy(session->buf, &single, sizeof(single));
- memcpy(session->buf + sizeof(single), data, len);
+
+ if (data)
+ memcpy(session->buf + sizeof(single), data, len);

return try_send(sock, session->buf, sizeof(single) + len);
}
@@ -569,7 +571,7 @@ static void pending_req_free(void *data)

if (req->timeout)
timeout_remove(req->timeout);
- g_free(req->data);
+ free(req->data);
g_free(req);
}

@@ -2687,7 +2689,7 @@ static int send_req(struct avdtp *session, gboolean priority,
return 0;

failed:
- g_free(req->data);
+ free(req->data);
g_free(req);
return err;
}
@@ -2705,8 +2707,7 @@ static int send_request(struct avdtp *session, gboolean priority,

req = g_new0(struct pending_req, 1);
req->signal_id = signal_id;
- req->data = g_malloc(size);
- memcpy(req->data, buffer, size);
+ req->data = util_memdup(buffer, size);
req->data_size = size;
req->stream = stream;

@@ -3323,7 +3324,9 @@ struct avdtp_service_capability *avdtp_service_cap_new(uint8_t category,
cap = g_malloc(sizeof(struct avdtp_service_capability) + length);
cap->category = category;
cap->length = length;
- memcpy(cap->data, data, length);
+
+ if (data)
+ memcpy(cap->data, data, length);

return cap;
}
--
2.33.1



2022-01-07 22:34:44

by bluez.test.bot

[permalink] [raw]
Subject: RE: [BlueZ] avdtp: Fix runtime errors passing NULL to memcpy

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=603674

---Test result---

Test Summary:
CheckPatch PASS 1.65 seconds
GitLint PASS 0.99 seconds
Prep - Setup ELL PASS 42.29 seconds
Build - Prep PASS 0.74 seconds
Build - Configure PASS 8.38 seconds
Build - Make FAIL 1256.13 seconds
Make Check FAIL 3.67 seconds
Make Check w/Valgrind FAIL 261.62 seconds
Make Distcheck FAIL 158.23 seconds
Build w/ext ELL - Configure PASS 8.46 seconds
Build w/ext ELL - Make FAIL 1230.37 seconds
Incremental Build with patchesPASS 0.00 seconds

Details
##############################
Test: Build - Make - FAIL
Desc: Build the BlueZ source tree
Output:
tools/mgmt-tester.c: In function ‘main’:
tools/mgmt-tester.c:12364:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
12364 | int main(int argc, char *argv[])
| ^~~~
unit/test-avdtp.c: In function ‘main’:
unit/test-avdtp.c:766:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
766 | int main(int argc, char *argv[])
| ^~~~
unit/test-avrcp.c: In function ‘main’:
unit/test-avrcp.c:989:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
989 | int main(int argc, char *argv[])
| ^~~~
profiles/audio/avdtp.c: In function ‘send_request’:
profiles/audio/avdtp.c:2710:14: error: implicit declaration of function ‘util_memdup’; did you mean ‘util_hexdump’? [-Werror=implicit-function-declaration]
2710 | req->data = util_memdup(buffer, size);
| ^~~~~~~~~~~
| util_hexdump
profiles/audio/avdtp.c:2710:12: error: assignment to ‘void *’ from ‘int’ makes pointer from integer without a cast [-Werror=int-conversion]
2710 | req->data = util_memdup(buffer, size);
| ^
cc1: all warnings being treated as errors
make[1]: *** [Makefile:9445: profiles/audio/bluetoothd-avdtp.o] Error 1
make: *** [Makefile:4302: all] Error 2


##############################
Test: Make Check - FAIL
Desc: Run 'make check'
Output:
profiles/audio/avdtp.c: In function ‘send_request’:
profiles/audio/avdtp.c:2710:14: error: implicit declaration of function ‘util_memdup’; did you mean ‘util_hexdump’? [-Werror=implicit-function-declaration]
2710 | req->data = util_memdup(buffer, size);
| ^~~~~~~~~~~
| util_hexdump
profiles/audio/avdtp.c:2710:12: error: assignment to ‘void *’ from ‘int’ makes pointer from integer without a cast [-Werror=int-conversion]
2710 | req->data = util_memdup(buffer, size);
| ^
cc1: all warnings being treated as errors
make[1]: *** [Makefile:9445: profiles/audio/bluetoothd-avdtp.o] Error 1
make: *** [Makefile:11306: check] Error 2


##############################
Test: Make Check w/Valgrind - FAIL
Desc: Run 'make check' with Valgrind
Output:
tools/mgmt-tester.c: In function ‘main’:
tools/mgmt-tester.c:12364:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
12364 | int main(int argc, char *argv[])
| ^~~~
profiles/audio/avdtp.c: In function ‘send_request’:
profiles/audio/avdtp.c:2710:14: error: implicit declaration of function ‘util_memdup’; did you mean ‘util_hexdump’? [-Werror=implicit-function-declaration]
2710 | req->data = util_memdup(buffer, size);
| ^~~~~~~~~~~
| util_hexdump
profiles/audio/avdtp.c:2710:12: error: assignment to ‘void *’ from ‘int’ makes pointer from integer without a cast [-Werror=int-conversion]
2710 | req->data = util_memdup(buffer, size);
| ^
cc1: all warnings being treated as errors
make[1]: *** [Makefile:9445: profiles/audio/bluetoothd-avdtp.o] Error 1
make: *** [Makefile:4302: all] Error 2


##############################
Test: Make Distcheck - FAIL
Desc: Run distcheck to check the distribution
Output:
../../profiles/audio/avdtp.c: In function ‘send_request’:
../../profiles/audio/avdtp.c:2710:14: warning: implicit declaration of function ‘util_memdup’; did you mean ‘util_hexdump’? [-Wimplicit-function-declaration]
2710 | req->data = util_memdup(buffer, size);
| ^~~~~~~~~~~
| util_hexdump
../../profiles/audio/avdtp.c:2710:12: warning: assignment to ‘void *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
2710 | req->data = util_memdup(buffer, size);
| ^
/usr/bin/ld: profiles/audio/bluetoothd-avdtp.o: in function `send_request':
/github/workspace/src/bluez-5.62/_build/sub/../../profiles/audio/avdtp.c:2710: undefined reference to `util_memdup'
collect2: error: ld returned 1 exit status
make[2]: *** [Makefile:5877: src/bluetoothd] Error 1
make[1]: *** [Makefile:4302: all] Error 2
make: *** [Makefile:11227: distcheck] Error 1


##############################
Test: Build w/ext ELL - Make - FAIL
Desc: Build BlueZ source with '--enable-external-ell' configuration
Output:
tools/mgmt-tester.c: In function ‘main’:
tools/mgmt-tester.c:12364:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
12364 | int main(int argc, char *argv[])
| ^~~~
unit/test-avdtp.c: In function ‘main’:
unit/test-avdtp.c:766:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
766 | int main(int argc, char *argv[])
| ^~~~
unit/test-avrcp.c: In function ‘main’:
unit/test-avrcp.c:989:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
989 | int main(int argc, char *argv[])
| ^~~~
profiles/audio/avdtp.c: In function ‘send_request’:
profiles/audio/avdtp.c:2710:14: error: implicit declaration of function ‘util_memdup’; did you mean ‘util_hexdump’? [-Werror=implicit-function-declaration]
2710 | req->data = util_memdup(buffer, size);
| ^~~~~~~~~~~
| util_hexdump
profiles/audio/avdtp.c:2710:12: error: assignment to ‘void *’ from ‘int’ makes pointer from integer without a cast [-Werror=int-conversion]
2710 | req->data = util_memdup(buffer, size);
| ^
cc1: all warnings being treated as errors
make[1]: *** [Makefile:9445: profiles/audio/bluetoothd-avdtp.o] Error 1
make: *** [Makefile:4302: all] Error 2




---
Regards,
Linux Bluetooth

2022-01-07 22:48:05

by Luiz Augusto von Dentz

[permalink] [raw]
Subject: Re: [BlueZ] avdtp: Fix runtime errors passing NULL to memcpy

Hi,

On Fri, Jan 7, 2022 at 2:34 PM <[email protected]> wrote:
>
> This is automated email and please do not reply to this email!
>
> Dear submitter,
>
> Thank you for submitting the patches to the linux bluetooth mailing list.
> This is a CI test results with your patch series:
> PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=603674
>
> ---Test result---
>
> Test Summary:
> CheckPatch PASS 1.65 seconds
> GitLint PASS 0.99 seconds
> Prep - Setup ELL PASS 42.29 seconds
> Build - Prep PASS 0.74 seconds
> Build - Configure PASS 8.38 seconds
> Build - Make FAIL 1256.13 seconds
> Make Check FAIL 3.67 seconds
> Make Check w/Valgrind FAIL 261.62 seconds
> Make Distcheck FAIL 158.23 seconds
> Build w/ext ELL - Configure PASS 8.46 seconds
> Build w/ext ELL - Make FAIL 1230.37 seconds
> Incremental Build with patchesPASS 0.00 seconds
>
> Details
> ##############################
> Test: Build - Make - FAIL
> Desc: Build the BlueZ source tree
> Output:
> tools/mgmt-tester.c: In function ‘main’:
> tools/mgmt-tester.c:12364:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
> 12364 | int main(int argc, char *argv[])
> | ^~~~
> unit/test-avdtp.c: In function ‘main’:
> unit/test-avdtp.c:766:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
> 766 | int main(int argc, char *argv[])
> | ^~~~
> unit/test-avrcp.c: In function ‘main’:
> unit/test-avrcp.c:989:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
> 989 | int main(int argc, char *argv[])
> | ^~~~
> profiles/audio/avdtp.c: In function ‘send_request’:
> profiles/audio/avdtp.c:2710:14: error: implicit declaration of function ‘util_memdup’; did you mean ‘util_hexdump’? [-Werror=implicit-function-declaration]
> 2710 | req->data = util_memdup(buffer, size);
> | ^~~~~~~~~~~
> | util_hexdump
> profiles/audio/avdtp.c:2710:12: error: assignment to ‘void *’ from ‘int’ makes pointer from integer without a cast [-Werror=int-conversion]
> 2710 | req->data = util_memdup(buffer, size);
> | ^
> cc1: all warnings being treated as errors
> make[1]: *** [Makefile:9445: profiles/audio/bluetoothd-avdtp.o] Error 1
> make: *** [Makefile:4302: all] Error 2
>
>
> ##############################
> Test: Make Check - FAIL
> Desc: Run 'make check'
> Output:
> profiles/audio/avdtp.c: In function ‘send_request’:
> profiles/audio/avdtp.c:2710:14: error: implicit declaration of function ‘util_memdup’; did you mean ‘util_hexdump’? [-Werror=implicit-function-declaration]
> 2710 | req->data = util_memdup(buffer, size);
> | ^~~~~~~~~~~
> | util_hexdump
> profiles/audio/avdtp.c:2710:12: error: assignment to ‘void *’ from ‘int’ makes pointer from integer without a cast [-Werror=int-conversion]
> 2710 | req->data = util_memdup(buffer, size);
> | ^
> cc1: all warnings being treated as errors
> make[1]: *** [Makefile:9445: profiles/audio/bluetoothd-avdtp.o] Error 1
> make: *** [Makefile:11306: check] Error 2
>
>
> ##############################
> Test: Make Check w/Valgrind - FAIL
> Desc: Run 'make check' with Valgrind
> Output:
> tools/mgmt-tester.c: In function ‘main’:
> tools/mgmt-tester.c:12364:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
> 12364 | int main(int argc, char *argv[])
> | ^~~~
> profiles/audio/avdtp.c: In function ‘send_request’:
> profiles/audio/avdtp.c:2710:14: error: implicit declaration of function ‘util_memdup’; did you mean ‘util_hexdump’? [-Werror=implicit-function-declaration]
> 2710 | req->data = util_memdup(buffer, size);
> | ^~~~~~~~~~~
> | util_hexdump
> profiles/audio/avdtp.c:2710:12: error: assignment to ‘void *’ from ‘int’ makes pointer from integer without a cast [-Werror=int-conversion]
> 2710 | req->data = util_memdup(buffer, size);
> | ^
> cc1: all warnings being treated as errors
> make[1]: *** [Makefile:9445: profiles/audio/bluetoothd-avdtp.o] Error 1
> make: *** [Makefile:4302: all] Error 2
>
>
> ##############################
> Test: Make Distcheck - FAIL
> Desc: Run distcheck to check the distribution
> Output:
> ../../profiles/audio/avdtp.c: In function ‘send_request’:
> ../../profiles/audio/avdtp.c:2710:14: warning: implicit declaration of function ‘util_memdup’; did you mean ‘util_hexdump’? [-Wimplicit-function-declaration]
> 2710 | req->data = util_memdup(buffer, size);
> | ^~~~~~~~~~~
> | util_hexdump
> ../../profiles/audio/avdtp.c:2710:12: warning: assignment to ‘void *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion]
> 2710 | req->data = util_memdup(buffer, size);
> | ^
> /usr/bin/ld: profiles/audio/bluetoothd-avdtp.o: in function `send_request':
> /github/workspace/src/bluez-5.62/_build/sub/../../profiles/audio/avdtp.c:2710: undefined reference to `util_memdup'
> collect2: error: ld returned 1 exit status
> make[2]: *** [Makefile:5877: src/bluetoothd] Error 1
> make[1]: *** [Makefile:4302: all] Error 2
> make: *** [Makefile:11227: distcheck] Error 1
>
>
> ##############################
> Test: Build w/ext ELL - Make - FAIL
> Desc: Build BlueZ source with '--enable-external-ell' configuration
> Output:
> tools/mgmt-tester.c: In function ‘main’:
> tools/mgmt-tester.c:12364:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
> 12364 | int main(int argc, char *argv[])
> | ^~~~
> unit/test-avdtp.c: In function ‘main’:
> unit/test-avdtp.c:766:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
> 766 | int main(int argc, char *argv[])
> | ^~~~
> unit/test-avrcp.c: In function ‘main’:
> unit/test-avrcp.c:989:5: note: variable tracking size limit exceeded with ‘-fvar-tracking-assignments’, retrying without
> 989 | int main(int argc, char *argv[])
> | ^~~~
> profiles/audio/avdtp.c: In function ‘send_request’:
> profiles/audio/avdtp.c:2710:14: error: implicit declaration of function ‘util_memdup’; did you mean ‘util_hexdump’? [-Werror=implicit-function-declaration]
> 2710 | req->data = util_memdup(buffer, size);
> | ^~~~~~~~~~~
> | util_hexdump
> profiles/audio/avdtp.c:2710:12: error: assignment to ‘void *’ from ‘int’ makes pointer from integer without a cast [-Werror=int-conversion]
> 2710 | req->data = util_memdup(buffer, size);
> | ^
> cc1: all warnings being treated as errors
> make[1]: *** [Makefile:9445: profiles/audio/bluetoothd-avdtp.o] Error 1
> make: *** [Makefile:4302: all] Error 2
>
>
>
>
> ---
> Regards,
> Linux Bluetooth

Pushed.

--
Luiz Augusto von Dentz