lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <1f85cb70-d90e-36c0-bf94-cfbc8193e6b9@sberdevices.ru>
Date:   Thu, 1 Dec 2022 11:40:13 +0000
From:   Arseniy Krasnov <AVKrasnov@...rdevices.ru>
To:     Stefano Garzarella <sgarzare@...hat.com>
CC:     "David S. Miller" <davem@...emloft.net>,
        "edumazet@...gle.com" <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        "kys@...rosoft.com" <kys@...rosoft.com>,
        "haiyangz@...rosoft.com" <haiyangz@...rosoft.com>,
        "wei.liu@...nel.org" <wei.liu@...nel.org>,
        Dexuan Cui <decui@...rosoft.com>,
        Bryan Tan <bryantan@...are.com>,
        Vishnu Dasa <vdasa@...are.com>,
        Krasnov Arseniy <oxffffaa@...il.com>,
        Bobby Eshleman <bobby.eshleman@...il.com>,
        "Bobby Eshleman" <bobby.eshleman@...edance.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "virtualization@...ts.linux-foundation.org" 
        <virtualization@...ts.linux-foundation.org>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        kernel <kernel@...rdevices.ru>
Subject: Re: [RFC PATCH v2 4/6] test/vsock: rework message bounds test

On 01.12.2022 12:41, Stefano Garzarella wrote:
> On Fri, Nov 25, 2022 at 05:10:35PM +0000, Arseniy Krasnov wrote:
>> This updates message bound test making it more complex. Instead of
>> sending 1 bytes messages with one MSG_EOR bit, it sends messages of
>> random length(one half of messages are smaller than page size, second
>> half are bigger) with random number of MSG_EOR bits set. Receiver
>> also don't know total number of messages.
>>
>> Signed-off-by: Arseniy Krasnov <AVKrasnov@...rdevices.ru>
>> ---
>> tools/testing/vsock/control.c    |  28 +++++++
>> tools/testing/vsock/control.h    |   2 +
>> tools/testing/vsock/util.c       |  13 ++++
>> tools/testing/vsock/util.h       |   1 +
>> tools/testing/vsock/vsock_test.c | 124 +++++++++++++++++++++++++++----
>> 5 files changed, 155 insertions(+), 13 deletions(-)
>>
>> diff --git a/tools/testing/vsock/control.c b/tools/testing/vsock/control.c
>> index 4874872fc5a3..d2deb4b15b94 100644
>> --- a/tools/testing/vsock/control.c
>> +++ b/tools/testing/vsock/control.c
>> @@ -141,6 +141,34 @@ void control_writeln(const char *str)
>>     timeout_end();
>> }
>>
>> +void control_writeulong(unsigned long value)
>> +{
>> +    char str[32];
>> +
>> +    if (snprintf(str, sizeof(str), "%lu", value) >= sizeof(str)) {
>> +        perror("snprintf");
>> +        exit(EXIT_FAILURE);
>> +    }
>> +
>> +    control_writeln(str);
>> +}
>> +
>> +unsigned long control_readulong(void)
>> +{
>> +    unsigned long value;
>> +    char *str;
>> +
>> +    str = control_readln();
>> +
>> +    if (!str)
>> +        exit(EXIT_FAILURE);
>> +
>> +    value = strtoul(str, NULL, 10);
>> +    free(str);
>> +
>> +    return value;
>> +}
>> +
>> /* Return the next line from the control socket (without the trailing newline).
>>  *
>>  * The program terminates if a timeout occurs.
>> diff --git a/tools/testing/vsock/control.h b/tools/testing/vsock/control.h
>> index 51814b4f9ac1..c1f77fdb2c7a 100644
>> --- a/tools/testing/vsock/control.h
>> +++ b/tools/testing/vsock/control.h
>> @@ -9,7 +9,9 @@ void control_init(const char *control_host, const char *control_port,
>> void control_cleanup(void);
>> void control_writeln(const char *str);
>> char *control_readln(void);
>> +unsigned long control_readulong(void);
>> void control_expectln(const char *str);
>> bool control_cmpln(char *line, const char *str, bool fail);
>> +void control_writeulong(unsigned long value);
>>
>> #endif /* CONTROL_H */
>> diff --git a/tools/testing/vsock/util.c b/tools/testing/vsock/util.c
>> index 2acbb7703c6a..01b636d3039a 100644
>> --- a/tools/testing/vsock/util.c
>> +++ b/tools/testing/vsock/util.c
>> @@ -395,3 +395,16 @@ void skip_test(struct test_case *test_cases, size_t test_cases_len,
>>
>>     test_cases[test_id].skip = true;
>> }
>> +
>> +unsigned long hash_djb2(const void *data, size_t len)
>> +{
>> +    unsigned long hash = 5381;
>> +    int i = 0;
>> +
>> +    while (i < len) {
>> +        hash = ((hash << 5) + hash) + ((unsigned char *)data)[i];
>> +        i++;
>> +    }
>> +
>> +    return hash;
>> +}
>> diff --git a/tools/testing/vsock/util.h b/tools/testing/vsock/util.h
>> index a3375ad2fb7f..fb99208a95ea 100644
>> --- a/tools/testing/vsock/util.h
>> +++ b/tools/testing/vsock/util.h
>> @@ -49,4 +49,5 @@ void run_tests(const struct test_case *test_cases,
>> void list_tests(const struct test_case *test_cases);
>> void skip_test(struct test_case *test_cases, size_t test_cases_len,
>>            const char *test_id_str);
>> +unsigned long hash_djb2(const void *data, size_t len);
>> #endif /* UTIL_H */
>> diff --git a/tools/testing/vsock/vsock_test.c b/tools/testing/vsock/vsock_test.c
>> index bb6d691cb30d..12ef0cca6f93 100644
>> --- a/tools/testing/vsock/vsock_test.c
>> +++ b/tools/testing/vsock/vsock_test.c
>> @@ -284,10 +284,14 @@ static void test_stream_msg_peek_server(const struct test_opts *opts)
>>     close(fd);
>> }
>>
>> -#define MESSAGES_CNT 7
>> -#define MSG_EOR_IDX (MESSAGES_CNT / 2)
>> +#define SOCK_BUF_SIZE (2 * 1024 * 1024)
>> +#define MAX_MSG_SIZE (32 * 1024)
>> +
>> static void test_seqpacket_msg_bounds_client(const struct test_opts *opts)
>> {
>> +    unsigned long curr_hash;
>> +    int page_size;
>> +    int msg_count;
>>     int fd;
>>
>>     fd = vsock_seqpacket_connect(opts->peer_cid, 1234);
>> @@ -296,18 +300,78 @@ static void test_seqpacket_msg_bounds_client(const struct test_opts *opts)
>>         exit(EXIT_FAILURE);
>>     }
>>
>> -    /* Send several messages, one with MSG_EOR flag */
>> -    for (int i = 0; i < MESSAGES_CNT; i++)
>> -        send_byte(fd, 1, (i == MSG_EOR_IDX) ? MSG_EOR : 0);
>> +    /* Wait, until receiver sets buffer size. */
>> +    control_expectln("SRVREADY");
>> +
>> +    curr_hash = 0;
>> +    page_size = getpagesize();
>> +    msg_count = SOCK_BUF_SIZE / MAX_MSG_SIZE;
>> +
>> +    for (int i = 0; i < msg_count; i++) {
>> +        ssize_t send_size;
>> +        size_t buf_size;
>> +        int flags;
>> +        void *buf;
>> +
>> +        /* Use "small" buffers and "big" buffers. */
>> +        if (i & 1)
>> +            buf_size = page_size +
>> +                    (rand() % (MAX_MSG_SIZE - page_size));
>> +        else
>> +            buf_size = 1 + (rand() % page_size);
>> +
>> +        buf = malloc(buf_size);
>> +
>> +        if (!buf) {
>> +            perror("malloc");
>> +            exit(EXIT_FAILURE);
>> +        }
>> +
>> +        /* Set at least one MSG_EOR + some random. */
>> +        if (i == (msg_count / 2) || (rand() & 1)) {
>> +            flags = MSG_EOR;
>> +            curr_hash++;
>> +        } else {
>> +            flags = 0;
>> +        }
>> +
>> +        send_size = send(fd, buf, buf_size, flags);
>> +
>> +        if (send_size < 0) {
>> +            perror("send");
>> +            exit(EXIT_FAILURE);
>> +        }
>> +
>> +        if (send_size != buf_size) {
>> +            fprintf(stderr, "Invalid send size\n");
>> +            exit(EXIT_FAILURE);
>> +        }
>> +
>> +        /*
>> +         * Hash sum is computed at both client and server in
>> +         * the same way: H = hash(H + 'message length'). Idea
>> +         * is simple: such hash "accumulates" length and order
>> +         * of every sent/received message. After data exchange,
>> +         * both sums are compared using control socket, and if
>> +         * message bounds wasn't broken - two values must be
>> +         * equal.
>> +         */
>> +        curr_hash += send_size;
> 
> Sorry, I thought about it now, but instead of hashing the size, couldn't we just fill the buffer with a pattern (maybe random?) and hash the content?
> 
> That way we see both that the bounds were met and that the content was transferred correctly.
> 
> What do you think?
> 
> The rest LGTM!
> 
> Stefano
Yes, I'll implement it in the next version as it gives exactly the same result

Thanks
> 
>> +        curr_hash = hash_djb2(&curr_hash, sizeof(curr_hash));
>> +    }
>>
>>     control_writeln("SENDDONE");
>> +    control_writeulong(curr_hash);
>>     close(fd);
>> }
>>
>> static void test_seqpacket_msg_bounds_server(const struct test_opts *opts)
>> {
>> +    unsigned long sock_buf_size;
>> +    unsigned long remote_hash;
>> +    unsigned long curr_hash;
>>     int fd;
>> -    char buf[16];
>> +    char buf[MAX_MSG_SIZE];
>>     struct msghdr msg = {0};
>>     struct iovec iov = {0};
>>
>> @@ -317,25 +381,58 @@ static void test_seqpacket_msg_bounds_server(const struct test_opts *opts)
>>         exit(EXIT_FAILURE);
>>     }
>>
>> +    sock_buf_size = SOCK_BUF_SIZE;
>> +
>> +    if (setsockopt(fd, AF_VSOCK, SO_VM_SOCKETS_BUFFER_MAX_SIZE,
>> +               &sock_buf_size, sizeof(sock_buf_size))) {
>> +        perror("getsockopt");
>> +        exit(EXIT_FAILURE);
>> +    }
>> +
>> +    if (setsockopt(fd, AF_VSOCK, SO_VM_SOCKETS_BUFFER_SIZE,
>> +               &sock_buf_size, sizeof(sock_buf_size))) {
>> +        perror("getsockopt");
>> +        exit(EXIT_FAILURE);
>> +    }
>> +
>> +    /* Ready to receive data. */
>> +    control_writeln("SRVREADY");
>> +    /* Wait, until peer sends whole data. */
>>     control_expectln("SENDDONE");
>>     iov.iov_base = buf;
>>     iov.iov_len = sizeof(buf);
>>     msg.msg_iov = &iov;
>>     msg.msg_iovlen = 1;
>>
>> -    for (int i = 0; i < MESSAGES_CNT; i++) {
>> -        if (recvmsg(fd, &msg, 0) != 1) {
>> -            perror("message bound violated");
>> -            exit(EXIT_FAILURE);
>> -        }
>> +    curr_hash = 0;
>>
>> -        if ((i == MSG_EOR_IDX) ^ !!(msg.msg_flags & MSG_EOR)) {
>> -            perror("MSG_EOR");
>> +    while (1) {
>> +        ssize_t recv_size;
>> +
>> +        recv_size = recvmsg(fd, &msg, 0);
>> +
>> +        if (!recv_size)
>> +            break;
>> +
>> +        if (recv_size < 0) {
>> +            perror("recvmsg");
>>             exit(EXIT_FAILURE);
>>         }
>> +
>> +        if (msg.msg_flags & MSG_EOR)
>> +            curr_hash++;
>> +
>> +        curr_hash += recv_size;
>> +        curr_hash = hash_djb2(&curr_hash, sizeof(curr_hash));
>>     }
>>
>>     close(fd);
>> +    remote_hash = control_readulong();
>> +
>> +    if (curr_hash != remote_hash) {
>> +        fprintf(stderr, "Message bounds broken\n");
>> +        exit(EXIT_FAILURE);
>> +    }
>> }
>>
>> #define MESSAGE_TRUNC_SZ 32
>> @@ -837,6 +934,7 @@ int main(int argc, char **argv)
>>         .peer_cid = VMADDR_CID_ANY,
>>     };
>>
>> +    srand(time(NULL));
>>     init_signals();
>>
>>     for (;;) {
>> -- 
>> 2.25.1
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ