Skip to content

Commit 18c40a1

Browse files
chuckleverkuba-moo
chucklever
authored and
kuba-moo
committed
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: 88232ec ("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>
1 parent b21c7ba commit 18c40a1

File tree

1 file changed

+28
-14
lines changed

1 file changed

+28
-14
lines changed

‎net/handshake/handshake-test.c‎

Lines changed: 28 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,7 @@ static void handshake_req_submit_test4(struct kunit *test)
209209
{
210210
struct handshake_req *req, *result;
211211
struct socket *sock;
212+
struct file *filp;
212213
int err;
213214

214215
/* Arrange */
@@ -218,9 +219,10 @@ static void handshake_req_submit_test4(struct kunit *test)
218219
err = __sock_create(&init_net, PF_INET, SOCK_STREAM, IPPROTO_TCP,
219220
&sock, 1);
220221
KUNIT_ASSERT_EQ(test, err, 0);
221-
sock->file = sock_alloc_file(sock, O_NONBLOCK, NULL);
222-
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, sock->file);
222+
filp = sock_alloc_file(sock, O_NONBLOCK, NULL);
223+
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filp);
223224
KUNIT_ASSERT_NOT_NULL(test, sock->sk);
225+
sock->file = filp;
224226

225227
err = handshake_req_submit(sock, req, GFP_KERNEL);
226228
KUNIT_ASSERT_EQ(test, err, 0);
@@ -241,6 +243,7 @@ static void handshake_req_submit_test5(struct kunit *test)
241243
struct handshake_req *req;
242244
struct handshake_net *hn;
243245
struct socket *sock;
246+
struct file *filp;
244247
struct net *net;
245248
int saved, err;
246249

@@ -251,9 +254,10 @@ static void handshake_req_submit_test5(struct kunit *test)
251254
err = __sock_create(&init_net, PF_INET, SOCK_STREAM, IPPROTO_TCP,
252255
&sock, 1);
253256
KUNIT_ASSERT_EQ(test, err, 0);
254-
sock->file = sock_alloc_file(sock, O_NONBLOCK, NULL);
255-
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, sock->file);
257+
filp = sock_alloc_file(sock, O_NONBLOCK, NULL);
258+
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filp);
256259
KUNIT_ASSERT_NOT_NULL(test, sock->sk);
260+
sock->file = filp;
257261

258262
net = sock_net(sock->sk);
259263
hn = handshake_pernet(net);
@@ -276,6 +280,7 @@ static void handshake_req_submit_test6(struct kunit *test)
276280
{
277281
struct handshake_req *req1, *req2;
278282
struct socket *sock;
283+
struct file *filp;
279284
int err;
280285

281286
/* Arrange */
@@ -287,9 +292,10 @@ static void handshake_req_submit_test6(struct kunit *test)
287292
err = __sock_create(&init_net, PF_INET, SOCK_STREAM, IPPROTO_TCP,
288293
&sock, 1);
289294
KUNIT_ASSERT_EQ(test, err, 0);
290-
sock->file = sock_alloc_file(sock, O_NONBLOCK, NULL);
291-
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, sock->file);
295+
filp = sock_alloc_file(sock, O_NONBLOCK, NULL);
296+
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filp);
292297
KUNIT_ASSERT_NOT_NULL(test, sock->sk);
298+
sock->file = filp;
293299

294300
/* Act */
295301
err = handshake_req_submit(sock, req1, GFP_KERNEL);
@@ -307,6 +313,7 @@ static void handshake_req_cancel_test1(struct kunit *test)
307313
{
308314
struct handshake_req *req;
309315
struct socket *sock;
316+
struct file *filp;
310317
bool result;
311318
int err;
312319

@@ -318,8 +325,9 @@ static void handshake_req_cancel_test1(struct kunit *test)
318325
&sock, 1);
319326
KUNIT_ASSERT_EQ(test, err, 0);
320327

321-
sock->file = sock_alloc_file(sock, O_NONBLOCK, NULL);
322-
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, sock->file);
328+
filp = sock_alloc_file(sock, O_NONBLOCK, NULL);
329+
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filp);
330+
sock->file = filp;
323331

324332
err = handshake_req_submit(sock, req, GFP_KERNEL);
325333
KUNIT_ASSERT_EQ(test, err, 0);
@@ -340,6 +348,7 @@ static void handshake_req_cancel_test2(struct kunit *test)
340348
struct handshake_req *req, *next;
341349
struct handshake_net *hn;
342350
struct socket *sock;
351+
struct file *filp;
343352
struct net *net;
344353
bool result;
345354
int err;
@@ -352,8 +361,9 @@ static void handshake_req_cancel_test2(struct kunit *test)
352361
&sock, 1);
353362
KUNIT_ASSERT_EQ(test, err, 0);
354363

355-
sock->file = sock_alloc_file(sock, O_NONBLOCK, NULL);
356-
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, sock->file);
364+
filp = sock_alloc_file(sock, O_NONBLOCK, NULL);
365+
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filp);
366+
sock->file = filp;
357367

358368
err = handshake_req_submit(sock, req, GFP_KERNEL);
359369
KUNIT_ASSERT_EQ(test, err, 0);
@@ -380,6 +390,7 @@ static void handshake_req_cancel_test3(struct kunit *test)
380390
struct handshake_req *req, *next;
381391
struct handshake_net *hn;
382392
struct socket *sock;
393+
struct file *filp;
383394
struct net *net;
384395
bool result;
385396
int err;
@@ -392,8 +403,9 @@ static void handshake_req_cancel_test3(struct kunit *test)
392403
&sock, 1);
393404
KUNIT_ASSERT_EQ(test, err, 0);
394405

395-
sock->file = sock_alloc_file(sock, O_NONBLOCK, NULL);
396-
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, sock->file);
406+
filp = sock_alloc_file(sock, O_NONBLOCK, NULL);
407+
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filp);
408+
sock->file = filp;
397409

398410
err = handshake_req_submit(sock, req, GFP_KERNEL);
399411
KUNIT_ASSERT_EQ(test, err, 0);
@@ -436,6 +448,7 @@ static void handshake_req_destroy_test1(struct kunit *test)
436448
{
437449
struct handshake_req *req;
438450
struct socket *sock;
451+
struct file *filp;
439452
int err;
440453

441454
/* Arrange */
@@ -448,8 +461,9 @@ static void handshake_req_destroy_test1(struct kunit *test)
448461
&sock, 1);
449462
KUNIT_ASSERT_EQ(test, err, 0);
450463

451-
sock->file = sock_alloc_file(sock, O_NONBLOCK, NULL);
452-
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, sock->file);
464+
filp = sock_alloc_file(sock, O_NONBLOCK, NULL);
465+
KUNIT_ASSERT_NOT_ERR_OR_NULL(test, filp);
466+
sock->file = filp;
453467

454468
err = handshake_req_submit(sock, req, GFP_KERNEL);
455469
KUNIT_ASSERT_EQ(test, err, 0);

0 commit comments

Comments
 (0)