[PATCH liburing 0/3] Fix undefined behavior, acessing dead object

public inbox for [email protected]
 help / color / mirror / Atom feed

* [PATCH liburing 0/3] Fix undefined behavior, acessing dead object
@ 2022-01-07 13:02 Ammar Faizi
  2022-01-07 13:02 ` [PATCH liburing 1/3] test/socket-rw-eagain: Fix UB, accessing " Ammar Faizi
                   ` (3 more replies)
  0 siblings, 4 replies; 5+ messages in thread
From: Ammar Faizi @ 2022-01-07 13:02 UTC (permalink / raw)
  To: Jens Axboe; +Cc: GNU/Weeb Mailing List, io-uring Mailing List, Ammar Faizi

This series fixes undefined behavior caused by accessing local
variables that have been out of their scope.

FWIW, compile the following code with gcc (Ubuntu 11.2.0-7ubuntu2) 11.2.0:
```
#include <stdio.h>

int main(void)
{
	int *pa, *pb;

	{
		int a;
		pa = &a;
	}

	{
		int b;
		pb = &b;
	}

	*pa = 100;
	*pb = 200;

	printf("(pa == pb) = %d\n", pa == pb);
	printf("*pa == %d; *pb == %d\n", *pa, *pb);
	return 0;
}
```

produces the following output:

```
  ammarfaizi2@integral2:/tmp$ gcc q.c -o q
  ammarfaizi2@integral2:/tmp$ ./q
  (pa == pb) = 1
  *pa == 200; *pb == 200
  ammarfaizi2@integral2:/tmp$
  ammarfaizi2@integral2:/tmp$ gcc -O3 q.c -o q
  ammarfaizi2@integral2:/tmp$ ./q
  (pa == pb) = 0
  *pa == 100; *pb == 200
  ammarfaizi2@integral2:/tmp$
```

Note that the `int a` and `int b` lifetime have ended, but we still
hold the references to them dereference them.

Also the result differs for the different optimization levels.
That's to say, there is no guarantee due to UB. Compiler is free
to reuse "out of scope variable"'s storage.

The same happens with test/socket-rw{,-eagain,-offset}.c.

Signed-off-by: Ammar Faizi <[email protected]>
---
Ammar Faizi (3):
  test/socket-rw-eagain: Fix UB, accessing dead object
  test/socket-rw: Fix UB, accessing dead object
  test/socket-rw-offset: Fix UB, accessing dead object

 test/socket-rw-eagain.c | 17 +++++++----------
 test/socket-rw-offset.c | 17 +++++++----------
 test/socket-rw.c        | 17 +++++++----------
 3 files changed, 21 insertions(+), 30 deletions(-)


base-commit: 918d8061ffdfdf253806a1e8e141c71644e678bd
-- 
2.32.0

-- 
GWML mailing list
[email protected]
https://gwml.gnuweeb.org/listinfo/gwml

^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH liburing 1/3] test/socket-rw-eagain: Fix UB, accessing dead object
  2022-01-07 13:02 [PATCH liburing 0/3] Fix undefined behavior, acessing dead object Ammar Faizi
@ 2022-01-07 13:02 ` Ammar Faizi
  2022-01-07 13:02 ` [PATCH liburing 2/3] test/socket-rw: " Ammar Faizi
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 5+ messages in thread
From: Ammar Faizi @ 2022-01-07 13:02 UTC (permalink / raw)
  To: Jens Axboe; +Cc: GNU/Weeb Mailing List, io-uring Mailing List, Ammar Faizi

Dereference to a local variable that has been out of its scope is
undefined behavior, it may contain garbage or the compiler may
reuse it for other local variables.

Fix this by moving the struct iov variable declarations so their
lifetime is extended.

Cc: Jens Axboe <[email protected]>
Fixes: 76e3b7921fee98a5627cd270628b6a5160d3857d ("Add nonblock empty socket read test")
Signed-off-by: Ammar Faizi <[email protected]>
---
 test/socket-rw-eagain.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/test/socket-rw-eagain.c b/test/socket-rw-eagain.c
index 9854e00..2d6a817 100644
--- a/test/socket-rw-eagain.c
+++ b/test/socket-rw-eagain.c
@@ -25,6 +25,7 @@ int main(int argc, char *argv[])
 	int32_t recv_s0;
 	int32_t val = 1;
 	struct sockaddr_in addr;
+	struct iovec iov_r[1], iov_w[1];
 
 	if (argc > 1)
 		return 0;
@@ -105,28 +106,24 @@ int main(int argc, char *argv[])
 	char send_buff[128];
 
 	{
-		struct iovec iov[1];
-
-		iov[0].iov_base = recv_buff;
-		iov[0].iov_len = sizeof(recv_buff);
+		iov_r[0].iov_base = recv_buff;
+		iov_r[0].iov_len = sizeof(recv_buff);
 
 		struct io_uring_sqe* sqe = io_uring_get_sqe(&m_io_uring);
 		assert(sqe != NULL);
 
-		io_uring_prep_readv(sqe, p_fd[0], iov, 1, 0);
+		io_uring_prep_readv(sqe, p_fd[0], iov_r, 1, 0);
 		sqe->user_data = 1;
 	}
 
 	{
-		struct iovec iov[1];
-
-		iov[0].iov_base = send_buff;
-		iov[0].iov_len = sizeof(send_buff);
+		iov_w[0].iov_base = send_buff;
+		iov_w[0].iov_len = sizeof(send_buff);
 
 		struct io_uring_sqe* sqe = io_uring_get_sqe(&m_io_uring);
 		assert(sqe != NULL);
 
-		io_uring_prep_writev(sqe, p_fd[1], iov, 1, 0);
+		io_uring_prep_writev(sqe, p_fd[1], iov_w, 1, 0);
 		sqe->user_data = 2;
 	}
 
-- 
2.32.0

-- 
GWML mailing list
[email protected]
https://gwml.gnuweeb.org/listinfo/gwml

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH liburing 2/3] test/socket-rw: Fix UB, accessing dead object
  2022-01-07 13:02 [PATCH liburing 0/3] Fix undefined behavior, acessing dead object Ammar Faizi
  2022-01-07 13:02 ` [PATCH liburing 1/3] test/socket-rw-eagain: Fix UB, accessing " Ammar Faizi
@ 2022-01-07 13:02 ` Ammar Faizi
  2022-01-07 13:02 ` [PATCH liburing 3/3] test/socket-rw-offset: " Ammar Faizi
  2022-01-09 16:47 ` [PATCH liburing 0/3] Fix undefined behavior, acessing " Jens Axboe
  3 siblings, 0 replies; 5+ messages in thread
From: Ammar Faizi @ 2022-01-07 13:02 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Hrvoje Zeba, GNU/Weeb Mailing List, io-uring Mailing List,
	Ammar Faizi

Dereference to a local variable that has been out of its scope is
undefined behavior, it may contain garbage or the compiler may
reuse it for other local variables.

Fix this by moving the struct iov variable declarations so their
lifetime is extended.

Cc: Jens Axboe <[email protected]>
Cc: Hrvoje Zeba <[email protected]>
Fixes: 79ba71a4881fb1cd300520553d7285b3c5ee1293 ("Add deadlock socket read/write test case")
Signed-off-by: Ammar Faizi <[email protected]>
---
 test/socket-rw.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/test/socket-rw.c b/test/socket-rw.c
index 5afd14d..4fbf032 100644
--- a/test/socket-rw.c
+++ b/test/socket-rw.c
@@ -27,6 +27,7 @@ int main(int argc, char *argv[])
 	int32_t recv_s0;
 	int32_t val = 1;
 	struct sockaddr_in addr;
+	struct iovec iov_r[1], iov_w[1];
 
 	if (argc > 1)
 		return 0;
@@ -103,27 +104,23 @@ int main(int argc, char *argv[])
 	char send_buff[128];
 
 	{
-		struct iovec iov[1];
-
-		iov[0].iov_base = recv_buff;
-		iov[0].iov_len = sizeof(recv_buff);
+		iov_r[0].iov_base = recv_buff;
+		iov_r[0].iov_len = sizeof(recv_buff);
 
 		struct io_uring_sqe* sqe = io_uring_get_sqe(&m_io_uring);
 		assert(sqe != NULL);
 
-		io_uring_prep_readv(sqe, p_fd[0], iov, 1, 0);
+		io_uring_prep_readv(sqe, p_fd[0], iov_r, 1, 0);
 	}
 
 	{
-		struct iovec iov[1];
-
-		iov[0].iov_base = send_buff;
-		iov[0].iov_len = sizeof(send_buff);
+		iov_w[0].iov_base = send_buff;
+		iov_w[0].iov_len = sizeof(send_buff);
 
 		struct io_uring_sqe* sqe = io_uring_get_sqe(&m_io_uring);
 		assert(sqe != NULL);
 
-		io_uring_prep_writev(sqe, p_fd[1], iov, 1, 0);
+		io_uring_prep_writev(sqe, p_fd[1], iov_w, 1, 0);
 	}
 
 	ret = io_uring_submit_and_wait(&m_io_uring, 2);
-- 
2.32.0

-- 
GWML mailing list
[email protected]
https://gwml.gnuweeb.org/listinfo/gwml

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH liburing 3/3] test/socket-rw-offset: Fix UB, accessing dead object
  2022-01-07 13:02 [PATCH liburing 0/3] Fix undefined behavior, acessing dead object Ammar Faizi
  2022-01-07 13:02 ` [PATCH liburing 1/3] test/socket-rw-eagain: Fix UB, accessing " Ammar Faizi
  2022-01-07 13:02 ` [PATCH liburing 2/3] test/socket-rw: " Ammar Faizi
@ 2022-01-07 13:02 ` Ammar Faizi
  2022-01-09 16:47 ` [PATCH liburing 0/3] Fix undefined behavior, acessing " Jens Axboe
  3 siblings, 0 replies; 5+ messages in thread
From: Ammar Faizi @ 2022-01-07 13:02 UTC (permalink / raw)
  To: Jens Axboe
  Cc: Hrvoje Zeba, GNU/Weeb Mailing List, io-uring Mailing List,
	Ammar Faizi

Dereference to a local variable that has been out of its scope is
undefined behavior, it may contain garbage or the compiler may
reuse it for other local variables.

Fix this by moving the struct iov variable declarations so their
lifetime is extended.

Cc: Jens Axboe <[email protected]>
Cc: Hrvoje Zeba <[email protected]>
Fixes: 03be3e4fbddd491ef0426b6f9c9085a168acc1c4 ("Add test case for socket read with offset == -1")
Signed-off-by: Ammar Faizi <[email protected]>
---
 test/socket-rw-offset.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/test/socket-rw-offset.c b/test/socket-rw-offset.c
index fe6ace3..987b6c9 100644
--- a/test/socket-rw-offset.c
+++ b/test/socket-rw-offset.c
@@ -27,6 +27,7 @@ int main(int argc, char *argv[])
 	int32_t recv_s0;
 	int32_t val = 1;
 	struct sockaddr_in addr;
+	struct iovec iov_r[1], iov_w[1];
 
 	if (argc > 1)
 		return 0;
@@ -108,27 +109,23 @@ int main(int argc, char *argv[])
 	char send_buff[128];
 
 	{
-		struct iovec iov[1];
-
-		iov[0].iov_base = recv_buff;
-		iov[0].iov_len = sizeof(recv_buff);
+		iov_r[0].iov_base = recv_buff;
+		iov_r[0].iov_len = sizeof(recv_buff);
 
 		struct io_uring_sqe* sqe = io_uring_get_sqe(&m_io_uring);
 		assert(sqe != NULL);
 
-		io_uring_prep_readv(sqe, p_fd[0], iov, 1, -1);
+		io_uring_prep_readv(sqe, p_fd[0], iov_r, 1, -1);
 	}
 
 	{
-		struct iovec iov[1];
-
-		iov[0].iov_base = send_buff;
-		iov[0].iov_len = sizeof(send_buff);
+		iov_w[0].iov_base = send_buff;
+		iov_w[0].iov_len = sizeof(send_buff);
 
 		struct io_uring_sqe* sqe = io_uring_get_sqe(&m_io_uring);
 		assert(sqe != NULL);
 
-		io_uring_prep_writev(sqe, p_fd[1], iov, 1, 0);
+		io_uring_prep_writev(sqe, p_fd[1], iov_w, 1, 0);
 	}
 
 	ret = io_uring_submit_and_wait(&m_io_uring, 2);
-- 
2.32.0

-- 
GWML mailing list
[email protected]
https://gwml.gnuweeb.org/listinfo/gwml

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH liburing 0/3] Fix undefined behavior, acessing dead object
  2022-01-07 13:02 [PATCH liburing 0/3] Fix undefined behavior, acessing dead object Ammar Faizi
                   ` (2 preceding siblings ...)
  2022-01-07 13:02 ` [PATCH liburing 3/3] test/socket-rw-offset: " Ammar Faizi
@ 2022-01-09 16:47 ` Jens Axboe
  3 siblings, 0 replies; 5+ messages in thread
From: Jens Axboe @ 2022-01-09 16:47 UTC (permalink / raw)
  To: Ammar Faizi; +Cc: GNU/Weeb Mailing List, io-uring Mailing List

On Fri, 7 Jan 2022 20:02:15 +0700, Ammar Faizi wrote:
> This series fixes undefined behavior caused by accessing local
> variables that have been out of their scope.
> 
> FWIW, compile the following code with gcc (Ubuntu 11.2.0-7ubuntu2) 11.2.0:
> ```
> #include <stdio.h>
> 
> [...]

Applied, thanks!

[1/3] test/socket-rw-eagain: Fix UB, accessing dead object
      commit: 5ee4feeac88d42c8c4cadee1f242279ff5fc0277
[2/3] test/socket-rw: Fix UB, accessing dead object
      commit: e5bb9f3e65f0e18132b27ba0322e2419d87f4f92
[3/3] test/socket-rw-offset: Fix UB, accessing dead object
      commit: 3f10277e6412d56cb52424d07f685128112498fa

Best regards,
-- 
Jens Axboe


-- 
GWML mailing list
[email protected]
https://gwml.gnuweeb.org/listinfo/gwml

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2022-01-09 16:47 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-01-07 13:02 [PATCH liburing 0/3] Fix undefined behavior, acessing dead object Ammar Faizi
2022-01-07 13:02 ` [PATCH liburing 1/3] test/socket-rw-eagain: Fix UB, accessing " Ammar Faizi
2022-01-07 13:02 ` [PATCH liburing 2/3] test/socket-rw: " Ammar Faizi
2022-01-07 13:02 ` [PATCH liburing 3/3] test/socket-rw-offset: " Ammar Faizi
2022-01-09 16:47 ` [PATCH liburing 0/3] Fix undefined behavior, acessing " Jens Axboe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox