netdev - Re: [PATCH net-next 0/4] vsock/test: Tests for memory leaks

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: <gm7qmwewqroqjyengpluw5xdr2mkv5u4fgjrwvly24pc5k2fl7@qelrw3hzq33h>
Date: Tue, 10 Dec 2024 17:25:46 +0100
From: Stefano Garzarella <sgarzare@...hat.com>
To: Michal Luczaj <mhal@...x.co>
Cc: netdev@...r.kernel.org
Subject: Re: [PATCH net-next 0/4] vsock/test: Tests for memory leaks

Hi Michal,

On Fri, Dec 06, 2024 at 07:34:50PM +0100, Michal Luczaj wrote:
>Series adds tests for recently fixed memory leaks[1]:
>
>d7b0ff5a8667 ("virtio/vsock: Fix accept_queue memory leak")
>fbf7085b3ad1 ("vsock: Fix sk_error_queue memory leak")
>60cf6206a1f5 ("virtio/vsock: Improve MSG_ZEROCOPY error handling")

Great! Thanks for these new tests!

>
>First patch is a non-functional preparatory cleanup.
>
>I initially considered triggering (and parsing) a kmemleak scan after each
>test, but ultimately concluded that the slowdown and the required
>privileges would be too much.

Yeah, what about adding something in the README to suggest using
kmemleak and how to check that everything is okay after a run?

I'd suggest also to add something about that in each patch that
introduce tests where we expects the user to check kmemleak,
at least with a comment on top of the test functions, and maybe
also in the commit description.

WDYT?

I left some comments and reported an issue with the last tests.

Thanks,
Stefano

>
>[1]: https://lore.kernel.org/netdev/20241107-vsock-mem-leaks-v2-0-4e21bfcfc818@rbox.co/
>
>Signed-off-by: Michal Luczaj <mhal@...x.co>
>---
>Michal Luczaj (4):
>      vsock/test: Use NSEC_PER_SEC
>      vsock/test: Add test for accept_queue memory leak
>      vsock/test: Add test for sk_error_queue memory leak
>      vsock/test: Add test for MSG_ZEROCOPY completion memory leak
>
> tools/testing/vsock/vsock_test.c | 159 ++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 157 insertions(+), 2 deletions(-)
>---
>base-commit: 51db5c8943001186be0b5b02456e7d03b3be1f12
>change-id: 20241203-test-vsock-leaks-38f9559f5636
>
>Best regards,
>-- 
>Michal Luczaj <mhal@...x.co>
>