On Fri, Mar 11, 2022 at 10:58:32AM +0000, Krasnov Arseniy Vladimirovich wrote:
Add test where sender sends two message, each with own
data pattern. Reader tries to read first to broken buffer:
it has three pages size, but middle page is unmapped. Then,
reader tries to read second message to valid buffer. Test
checks, that uncopied part of first message was dropped
and thus not copied as part of second message.
Signed-off-by: Arseniy Krasnov <AVKrasnov@xxxxxxxxxxxxxx>
---
tools/testing/vsock/vsock_test.c | 121 +++++++++++++++++++++++++++++++
1 file changed, 121 insertions(+)
diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
index aa2de27d0f77..686af712b4ad 100644
--- a/tools/testing/vsock/vsock_test.c
+++ b/tools/testing/vsock/vsock_test.c
@@ -16,6 +16,7 @@
#include <linux/kernel.h>
#include <sys/types.h>
#include <sys/socket.h>
+#include <sys/mman.h>
#include "timeout.h"
#include "control.h"
@@ -435,6 +436,121 @@ static void test_seqpacket_timeout_server(const struct test_opts *opts)
close(fd);
}
+#define BUF_PATTERN_1 'a'
+#define BUF_PATTERN_2 'b'
+
+static void test_seqpacket_invalid_rec_buffer_client(const struct test_opts *opts)
+{
+ int fd;
+ unsigned char *buf1;
+ unsigned char *buf2;
+ int buf_size = getpagesize() * 3;
+
+ fd = vsock_seqpacket_connect(opts->peer_cid, 1234);
+ if (fd < 0) {
+ perror("connect");
+ exit(EXIT_FAILURE);
+ }
+
+ buf1 = malloc(buf_size);
+ if (buf1 == NULL) {
+ perror("'malloc()' for 'buf1'");
+ exit(EXIT_FAILURE);
+ }
+
+ buf2 = malloc(buf_size);
+ if (buf2 == NULL) {
+ perror("'malloc()' for 'buf2'");
+ exit(EXIT_FAILURE);
+ }
+
+ memset(buf1, BUF_PATTERN_1, buf_size);
+ memset(buf2, BUF_PATTERN_2, buf_size);
+
+ if (send(fd, buf1, buf_size, 0) != buf_size) {
+ perror("send failed");
+ exit(EXIT_FAILURE);
+ }
+
+ if (send(fd, buf2, buf_size, 0) != buf_size) {
+ perror("send failed");
+ exit(EXIT_FAILURE);
+ }
+
+ close(fd);
+}
+
+static void test_seqpacket_invalid_rec_buffer_server(const struct test_opts *opts)
+{
+ int fd;
+ unsigned char *broken_buf;
+ unsigned char *valid_buf;
+ int page_size = getpagesize();
+ int buf_size = page_size * 3;
+ ssize_t res;
+ int prot = PROT_READ | PROT_WRITE;
+ int flags = MAP_PRIVATE | MAP_ANONYMOUS;
+ int i;
+
+ fd = vsock_seqpacket_accept(VMADDR_CID_ANY, 1234, NULL);
+ if (fd < 0) {
+ perror("accept");
+ exit(EXIT_FAILURE);
+ }
+
+ /* Setup first buffer. */
+ broken_buf = mmap(NULL, buf_size, prot, flags, -1, 0);
+ if (broken_buf == MAP_FAILED) {
+ perror("mmap for 'broken_buf'");
+ exit(EXIT_FAILURE);
+ }
+
+ /* Unmap "hole" in buffer. */
+ if (munmap(broken_buf + page_size, page_size)) {
+ perror("'broken_buf' setup");
+ exit(EXIT_FAILURE);
+ }
+
+ valid_buf = mmap(NULL, buf_size, prot, flags, -1, 0);
+ if (valid_buf == MAP_FAILED) {
+ perror("mmap for 'valid_buf'");
+ exit(EXIT_FAILURE);
+ }
+
+ /* Try to fill buffer with unmapped middle. */
+ res = read(fd, broken_buf, buf_size);
+ if (res != -1) {
+ perror("invalid read result of 'broken_buf'");
if `res` is valid, errno is not set, better to use fprintf(stderr, ...)
printing the expected and received result.
Take a look at test_stream_connection_reset()
+ exit(EXIT_FAILURE);
+ }
+
+ if (errno != ENOMEM) {
+ perror("invalid errno of 'broken_buf'");
Instead of "invalid", I would say "unexpected".
+ exit(EXIT_FAILURE);
+ }
+
+ /* Try to fill valid buffer. */
+ res = read(fd, valid_buf, buf_size);
+ if (res != buf_size) {
+ perror("invalid read result of 'valid_buf'");
I would split in 2 checks:
- (res < 0) then use perror()
- (res != buf_size) then use fprintf(stderr, ...) printing the expected
and received result.
+ exit(EXIT_FAILURE);
+ }
+
+ for (i = 0; i < buf_size; i++) {
+ if (valid_buf[i] != BUF_PATTERN_2) {
+ perror("invalid pattern for valid buf");
errno is not set here, better to use fprintf(stderr, ...)
+ exit(EXIT_FAILURE);
+ }
+ }
What about replace this for with a memcmp()?
+
+
+ /* Unmap buffers. */
+ munmap(broken_buf, page_size);
+ munmap(broken_buf + page_size * 2, page_size);
+ munmap(valid_buf, buf_size);
+ close(fd);
+}
+
static struct test_case test_cases[] = {
{
.name = "SOCK_STREAM connection reset",
@@ -480,6 +596,11 @@ static struct test_case test_cases[] = {
.run_client = test_seqpacket_timeout_client,
.run_server = test_seqpacket_timeout_server,
},
+ {
+ .name = "SOCK_SEQPACKET invalid receive buffer",
+ .run_client = test_seqpacket_invalid_rec_buffer_client,
+ .run_server = test_seqpacket_invalid_rec_buffer_server,
+ },
Is this the right behavior? If read() fails because the buffer is
invalid, do we throw out the whole packet?
I was expecting the packet not to be consumed, have you tried AF_UNIX,
does it have the same behavior?
Thanks,
Stefano
