net/handshake: Fix sock->file allocation

	sock->file = sock_alloc_file(sock, O_NONBLOCK, NULL);
	^^^^                         ^^^^

sock_alloc_file() calls release_sock() on error but the left hand
side of the assignment dereferences "sock".  This isn't the bug and
I didn't report this earlier because there is an assert that it
doesn't fail.

net/handshake/handshake-test.c:221 handshake_req_submit_test4() error: dereferencing freed memory 'sock'
net/handshake/handshake-test.c:233 handshake_req_submit_test4() warn: 'req' was already freed.
net/handshake/handshake-test.c:254 handshake_req_submit_test5() error: dereferencing freed memory 'sock'
net/handshake/handshake-test.c:290 handshake_req_submit_test6() error: dereferencing freed memory 'sock'
net/handshake/handshake-test.c:321 handshake_req_cancel_test1() error: dereferencing freed memory 'sock'
net/handshake/handshake-test.c:355 handshake_req_cancel_test2() error: dereferencing freed memory 'sock'
net/handshake/handshake-test.c:367 handshake_req_cancel_test2() warn: 'req' was already freed.
net/handshake/handshake-test.c:395 handshake_req_cancel_test3() error: dereferencing freed memory 'sock'
net/handshake/handshake-test.c:407 handshake_req_cancel_test3() warn: 'req' was already freed.
net/handshake/handshake-test.c:451 handshake_req_destroy_test1() error: dereferencing freed memory 'sock'
net/handshake/handshake-test.c:463 handshake_req_destroy_test1() warn: 'req' was already freed.

Reported-by: Dan Carpenter <dan.carpenter@linaro.org>
Fixes: 88232ec1ec5e ("net/handshake: Add Kunit tests for the handshake consumer API")
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Link: https://lore.kernel.org/r/168451609436.45209.15407022385441542980.stgit@oracle-102.nfsv4bat.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/net/handshake/handshake-test.c b/net/handshake/handshake-test.c
index 6193e46..6d37bab 100644 (file)
--- a/net/handshake/handshake-test.c
+++ b/net/handshake/handshake-test.c
@@ -209,6 +209,7 @@ static void handshake_req_submit_test4(struct kunit *test)
 {
        struct handshake_req *req, *result;
        struct socket *sock;
+       struct file *filp;
        int err;
 
        /* Arrange */
@@ -218,9 +219,10 @@ static void handshake_req_submit_test4(struct kunit *test)
        err = __sock_create(&init_net, PF_INET, SOCK_STREAM, IPPROTO_TCP,
                            &sock, 1);
        KUNIT_ASSERT_EQ(test, err, 0);
-       sock->file = sock_alloc_file(sock, O_NONBLOCK, NULL);
-       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, sock->file);
+       filp = sock_alloc_file(sock, O_NONBLOCK, NULL);
+       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filp);
        KUNIT_ASSERT_NOT_NULL(test, sock->sk);
+       sock->file = filp;
 
        err = handshake_req_submit(sock, req, GFP_KERNEL);
        KUNIT_ASSERT_EQ(test, err, 0);
@@ -241,6 +243,7 @@ static void handshake_req_submit_test5(struct kunit *test)
        struct handshake_req *req;
        struct handshake_net *hn;
        struct socket *sock;
+       struct file *filp;
        struct net *net;
        int saved, err;
 
@@ -251,9 +254,10 @@ static void handshake_req_submit_test5(struct kunit *test)
        err = __sock_create(&init_net, PF_INET, SOCK_STREAM, IPPROTO_TCP,
                            &sock, 1);
        KUNIT_ASSERT_EQ(test, err, 0);
-       sock->file = sock_alloc_file(sock, O_NONBLOCK, NULL);
-       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, sock->file);
+       filp = sock_alloc_file(sock, O_NONBLOCK, NULL);
+       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filp);
        KUNIT_ASSERT_NOT_NULL(test, sock->sk);
+       sock->file = filp;
 
        net = sock_net(sock->sk);
        hn = handshake_pernet(net);
@@ -276,6 +280,7 @@ static void handshake_req_submit_test6(struct kunit *test)
 {
        struct handshake_req *req1, *req2;
        struct socket *sock;
+       struct file *filp;
        int err;
 
        /* Arrange */
@@ -287,9 +292,10 @@ static void handshake_req_submit_test6(struct kunit *test)
        err = __sock_create(&init_net, PF_INET, SOCK_STREAM, IPPROTO_TCP,
                            &sock, 1);
        KUNIT_ASSERT_EQ(test, err, 0);
-       sock->file = sock_alloc_file(sock, O_NONBLOCK, NULL);
-       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, sock->file);
+       filp = sock_alloc_file(sock, O_NONBLOCK, NULL);
+       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filp);
        KUNIT_ASSERT_NOT_NULL(test, sock->sk);
+       sock->file = filp;
 
        /* Act */
        err = handshake_req_submit(sock, req1, GFP_KERNEL);
@@ -307,6 +313,7 @@ static void handshake_req_cancel_test1(struct kunit *test)
 {
        struct handshake_req *req;
        struct socket *sock;
+       struct file *filp;
        bool result;
        int err;
 
@@ -318,8 +325,9 @@ static void handshake_req_cancel_test1(struct kunit *test)
                            &sock, 1);
        KUNIT_ASSERT_EQ(test, err, 0);
 
-       sock->file = sock_alloc_file(sock, O_NONBLOCK, NULL);
-       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, sock->file);
+       filp = sock_alloc_file(sock, O_NONBLOCK, NULL);
+       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filp);
+       sock->file = filp;
 
        err = handshake_req_submit(sock, req, GFP_KERNEL);
        KUNIT_ASSERT_EQ(test, err, 0);
@@ -340,6 +348,7 @@ static void handshake_req_cancel_test2(struct kunit *test)
        struct handshake_req *req, *next;
        struct handshake_net *hn;
        struct socket *sock;
+       struct file *filp;
        struct net *net;
        bool result;
        int err;
@@ -352,8 +361,9 @@ static void handshake_req_cancel_test2(struct kunit *test)
                            &sock, 1);
        KUNIT_ASSERT_EQ(test, err, 0);
 
-       sock->file = sock_alloc_file(sock, O_NONBLOCK, NULL);
-       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, sock->file);
+       filp = sock_alloc_file(sock, O_NONBLOCK, NULL);
+       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filp);
+       sock->file = filp;
 
        err = handshake_req_submit(sock, req, GFP_KERNEL);
        KUNIT_ASSERT_EQ(test, err, 0);
@@ -380,6 +390,7 @@ static void handshake_req_cancel_test3(struct kunit *test)
        struct handshake_req *req, *next;
        struct handshake_net *hn;
        struct socket *sock;
+       struct file *filp;
        struct net *net;
        bool result;
        int err;
@@ -392,8 +403,9 @@ static void handshake_req_cancel_test3(struct kunit *test)
                            &sock, 1);
        KUNIT_ASSERT_EQ(test, err, 0);
 
-       sock->file = sock_alloc_file(sock, O_NONBLOCK, NULL);
-       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, sock->file);
+       filp = sock_alloc_file(sock, O_NONBLOCK, NULL);
+       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filp);
+       sock->file = filp;
 
        err = handshake_req_submit(sock, req, GFP_KERNEL);
        KUNIT_ASSERT_EQ(test, err, 0);
@@ -436,6 +448,7 @@ static void handshake_req_destroy_test1(struct kunit *test)
 {
        struct handshake_req *req;
        struct socket *sock;
+       struct file *filp;
        int err;
 
        /* Arrange */
@@ -448,8 +461,9 @@ static void handshake_req_destroy_test1(struct kunit *test)
                            &sock, 1);
        KUNIT_ASSERT_EQ(test, err, 0);
 
-       sock->file = sock_alloc_file(sock, O_NONBLOCK, NULL);
-       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, sock->file);
+       filp = sock_alloc_file(sock, O_NONBLOCK, NULL);
+       KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filp);
+       sock->file = filp;
 
        err = handshake_req_submit(sock, req, GFP_KERNEL);
        KUNIT_ASSERT_EQ(test, err, 0);