| Message ID | 170724699027.91401.7839730697326806733.stgit@oracle-102.nfsv4bat.org (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 4e1d71cabb19ec2586827adfc60d68689c68c194 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v1] net/handshake: Fix handshake_req_destroy_test1 | expand |
On 2/7/24 03:16, Chuck Lever wrote: > From: Chuck Lever <chuck.lever@oracle.com> > > Recently, handshake_req_destroy_test1 started failing: > > Expected handshake_req_destroy_test == req, but > handshake_req_destroy_test == 0000000000000000 > req == 0000000060f99b40 > not ok 11 req_destroy works > > This is because "sock_release(sock)" was replaced with "fput(filp)" > to address a memory leak. Note that sock_release() is synchronous > but fput() usually delays the final close and clean-up. > > The delay is not consequential in the other cases that were changed > but handshake_req_destroy_test1 is testing that handshake_req_cancel() > followed by closing the file actually does call the ->hp_destroy > method. Thus the PTR_EQ test at the end has to be sure that the > final close is complete before it checks the pointer. > > We cannot use a completion here because if ->hp_destroy is never > called (ie, there is an API bug) then the test will hang. > > Reported by: Guenter Roeck <linux@roeck-us.net> > Closes: https://lore.kernel.org/netdev/ZcKDd1to4MPANCrn@tissot.1015granger.net/T/#mac5c6299f86799f1c71776f3a07f9c566c7c3c40 > Fixes: 4a0f07d71b04 ("net/handshake: Fix memory leak in __sock_create() and sock_alloc_file()") > Signed-off-by: Chuck Lever <chuck.lever@oracle.com> > --- > net/handshake/handshake-test.c | 5 ++++- > 1 file changed, 4 insertions(+), 1 deletion(-) > > diff --git a/net/handshake/handshake-test.c b/net/handshake/handshake-test.c > index 16ed7bfd29e4..34fd1d9b2db8 100644 > --- a/net/handshake/handshake-test.c > +++ b/net/handshake/handshake-test.c > @@ -471,7 +471,10 @@ static void handshake_req_destroy_test1(struct kunit *test) > handshake_req_cancel(sock->sk); > > /* Act */ > - fput(filp); > + /* Ensure the close/release/put process has run to > + * completion before checking the result. > + */ > + __fput_sync(filp); > > /* Assert */ > KUNIT_EXPECT_PTR_EQ(test, handshake_req_destroy_test, req); > > > Reviewed-by: Hannes Reinecke <hare@suse.de> Cheers, Hannes
Hello: This patch was applied to netdev/net.git (main) by Jakub Kicinski <kuba@kernel.org>: On Tue, 06 Feb 2024 14:16:31 -0500 you wrote: > From: Chuck Lever <chuck.lever@oracle.com> > > Recently, handshake_req_destroy_test1 started failing: > > Expected handshake_req_destroy_test == req, but > handshake_req_destroy_test == 0000000000000000 > req == 0000000060f99b40 > not ok 11 req_destroy works > > [...] Here is the summary with links: - [v1] net/handshake: Fix handshake_req_destroy_test1 https://git.kernel.org/netdev/net/c/4e1d71cabb19 You are awesome, thank you!
diff --git a/net/handshake/handshake-test.c b/net/handshake/handshake-test.c index 16ed7bfd29e4..34fd1d9b2db8 100644 --- a/net/handshake/handshake-test.c +++ b/net/handshake/handshake-test.c @@ -471,7 +471,10 @@ static void handshake_req_destroy_test1(struct kunit *test) handshake_req_cancel(sock->sk); /* Act */ - fput(filp); + /* Ensure the close/release/put process has run to + * completion before checking the result. + */ + __fput_sync(filp); /* Assert */ KUNIT_EXPECT_PTR_EQ(test, handshake_req_destroy_test, req);