Skip to content

Commit 341f741

Browse files
dhowellsdhowells
committed
afs: Refcount the afs_call struct
A static checker warning occurs in the AFS filesystem: fs/afs/cmservice.c:155 SRXAFSCB_CallBack() error: dereferencing freed memory 'call' due to the reply being sent before we access the server it points to. The act of sending the reply causes the call to be freed if an error occurs (but not if it doesn't). On top of this, the lifetime handling of afs_call structs is fragile because they get passed around through workqueues without any sort of refcounting. Deal with the issues by: (1) Fix the maybe/maybe not nature of the reply sending functions with regards to whether they release the call struct. (2) Refcount the afs_call struct and sort out places that need to get/put references. (3) Pass a ref through the work queue and release (or pass on) that ref in the work function. Care has to be taken because a work queue may already own a ref to the call. (4) Do the cleaning up in the put function only. (5) Simplify module cleanup by always incrementing afs_outstanding_calls whenever a call is allocated. (6) Set the backlog to 0 with kernel_listen() at the beginning of the process of closing the socket to prevent new incoming calls from occurring and to remove the contribution of preallocated calls from afs_outstanding_calls before we wait on it. A tracepoint is also added to monitor the afs_call refcount and lifetime. Reported-by: Dan Carpenter <dan.carpenter@oracle.com> Signed-off-by: David Howells <dhowells@redhat.com> Fixes: 08e0e7c: "[AF_RXRPC]: Make the in-kernel AFS filesystem use AF_RXRPC."
1 parent 210f035 commit 341f741

File tree

4 files changed

+199
-79
lines changed

4 files changed

+199
-79
lines changed

‎fs/afs/cmservice.c

Lines changed: 22 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@ static int afs_deliver_cb_callback(struct afs_call *);
2424
static int afs_deliver_cb_probe_uuid(struct afs_call *);
2525
static int afs_deliver_cb_tell_me_about_yourself(struct afs_call *);
2626
static void afs_cm_destructor(struct afs_call *);
27+
static void SRXAFSCB_CallBack(struct work_struct *);
28+
static void SRXAFSCB_InitCallBackState(struct work_struct *);
29+
static void SRXAFSCB_Probe(struct work_struct *);
30+
static void SRXAFSCB_ProbeUuid(struct work_struct *);
31+
static void SRXAFSCB_TellMeAboutYourself(struct work_struct *);
2732

2833
#define CM_NAME(name) \
2934
const char afs_SRXCB##name##_name[] __tracepoint_string = \
@@ -38,6 +43,7 @@ static const struct afs_call_type afs_SRXCBCallBack = {
3843
.deliver = afs_deliver_cb_callback,
3944
.abort_to_error = afs_abort_to_error,
4045
.destructor = afs_cm_destructor,
46+
.work = SRXAFSCB_CallBack,
4147
};
4248

4349
/*
@@ -49,6 +55,7 @@ static const struct afs_call_type afs_SRXCBInitCallBackState = {
4955
.deliver = afs_deliver_cb_init_call_back_state,
5056
.abort_to_error = afs_abort_to_error,
5157
.destructor = afs_cm_destructor,
58+
.work = SRXAFSCB_InitCallBackState,
5259
};
5360

5461
/*
@@ -60,6 +67,7 @@ static const struct afs_call_type afs_SRXCBInitCallBackState3 = {
6067
.deliver = afs_deliver_cb_init_call_back_state3,
6168
.abort_to_error = afs_abort_to_error,
6269
.destructor = afs_cm_destructor,
70+
.work = SRXAFSCB_InitCallBackState,
6371
};
6472

6573
/*
@@ -71,6 +79,7 @@ static const struct afs_call_type afs_SRXCBProbe = {
7179
.deliver = afs_deliver_cb_probe,
7280
.abort_to_error = afs_abort_to_error,
7381
.destructor = afs_cm_destructor,
82+
.work = SRXAFSCB_Probe,
7483
};
7584

7685
/*
@@ -82,6 +91,7 @@ static const struct afs_call_type afs_SRXCBProbeUuid = {
8291
.deliver = afs_deliver_cb_probe_uuid,
8392
.abort_to_error = afs_abort_to_error,
8493
.destructor = afs_cm_destructor,
94+
.work = SRXAFSCB_ProbeUuid,
8595
};
8696

8797
/*
@@ -93,6 +103,7 @@ static const struct afs_call_type afs_SRXCBTellMeAboutYourself = {
93103
.deliver = afs_deliver_cb_tell_me_about_yourself,
94104
.abort_to_error = afs_abort_to_error,
95105
.destructor = afs_cm_destructor,
106+
.work = SRXAFSCB_TellMeAboutYourself,
96107
};
97108

98109
/*
@@ -163,6 +174,7 @@ static void SRXAFSCB_CallBack(struct work_struct *work)
163174
afs_send_empty_reply(call);
164175

165176
afs_break_callbacks(call->server, call->count, call->request);
177+
afs_put_call(call);
166178
_leave("");
167179
}
168180

@@ -284,9 +296,7 @@ static int afs_deliver_cb_callback(struct afs_call *call)
284296
return -ENOTCONN;
285297
call->server = server;
286298

287-
INIT_WORK(&call->work, SRXAFSCB_CallBack);
288-
queue_work(afs_wq, &call->work);
289-
return 0;
299+
return afs_queue_call_work(call);
290300
}
291301

292302
/*
@@ -300,6 +310,7 @@ static void SRXAFSCB_InitCallBackState(struct work_struct *work)
300310

301311
afs_init_callback_state(call->server);
302312
afs_send_empty_reply(call);
313+
afs_put_call(call);
303314
_leave("");
304315
}
305316

@@ -330,9 +341,7 @@ static int afs_deliver_cb_init_call_back_state(struct afs_call *call)
330341
return -ENOTCONN;
331342
call->server = server;
332343

333-
INIT_WORK(&call->work, SRXAFSCB_InitCallBackState);
334-
queue_work(afs_wq, &call->work);
335-
return 0;
344+
return afs_queue_call_work(call);
336345
}
337346

338347
/*
@@ -404,9 +413,7 @@ static int afs_deliver_cb_init_call_back_state3(struct afs_call *call)
404413
return -ENOTCONN;
405414
call->server = server;
406415

407-
INIT_WORK(&call->work, SRXAFSCB_InitCallBackState);
408-
queue_work(afs_wq, &call->work);
409-
return 0;
416+
return afs_queue_call_work(call);
410417
}
411418

412419
/*
@@ -418,6 +425,7 @@ static void SRXAFSCB_Probe(struct work_struct *work)
418425

419426
_enter("");
420427
afs_send_empty_reply(call);
428+
afs_put_call(call);
421429
_leave("");
422430
}
423431

@@ -437,9 +445,7 @@ static int afs_deliver_cb_probe(struct afs_call *call)
437445
/* no unmarshalling required */
438446
call->state = AFS_CALL_REPLYING;
439447

440-
INIT_WORK(&call->work, SRXAFSCB_Probe);
441-
queue_work(afs_wq, &call->work);
442-
return 0;
448+
return afs_queue_call_work(call);
443449
}
444450

445451
/*
@@ -462,6 +468,7 @@ static void SRXAFSCB_ProbeUuid(struct work_struct *work)
462468
reply.match = htonl(1);
463469

464470
afs_send_simple_reply(call, &reply, sizeof(reply));
471+
afs_put_call(call);
465472
_leave("");
466473
}
467474

@@ -520,9 +527,7 @@ static int afs_deliver_cb_probe_uuid(struct afs_call *call)
520527

521528
call->state = AFS_CALL_REPLYING;
522529

523-
INIT_WORK(&call->work, SRXAFSCB_ProbeUuid);
524-
queue_work(afs_wq, &call->work);
525-
return 0;
530+
return afs_queue_call_work(call);
526531
}
527532

528533
/*
@@ -584,7 +589,7 @@ static void SRXAFSCB_TellMeAboutYourself(struct work_struct *work)
584589
reply.cap.capcount = htonl(1);
585590
reply.cap.caps[0] = htonl(AFS_CAP_ERROR_TRANSLATION);
586591
afs_send_simple_reply(call, &reply, sizeof(reply));
587-
592+
afs_put_call(call);
588593
_leave("");
589594
}
590595

@@ -604,7 +609,5 @@ static int afs_deliver_cb_tell_me_about_yourself(struct afs_call *call)
604609
/* no unmarshalling required */
605610
call->state = AFS_CALL_REPLYING;
606611

607-
INIT_WORK(&call->work, SRXAFSCB_TellMeAboutYourself);
608-
queue_work(afs_wq, &call->work);
609-
return 0;
612+
return afs_queue_call_work(call);
610613
}

‎fs/afs/internal.h

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ enum afs_call_state {
6666
struct afs_call {
6767
const struct afs_call_type *type; /* type of call */
6868
wait_queue_head_t waitq; /* processes awaiting completion */
69-
struct work_struct async_work; /* asynchronous work processor */
69+
struct work_struct async_work; /* async I/O processor */
7070
struct work_struct work; /* actual work processor */
7171
struct rxrpc_call *rxcall; /* RxRPC call handle */
7272
struct key *key; /* security for this call */
@@ -82,6 +82,7 @@ struct afs_call {
8282
pgoff_t first; /* first page in mapping to deal with */
8383
pgoff_t last; /* last page in mapping to deal with */
8484
size_t offset; /* offset into received data store */
85+
atomic_t usage;
8586
enum afs_call_state state;
8687
int error; /* error code */
8788
u32 abort_code; /* Remote abort ID or 0 */
@@ -115,6 +116,9 @@ struct afs_call_type {
115116

116117
/* clean up a call */
117118
void (*destructor)(struct afs_call *call);
119+
120+
/* Work function */
121+
void (*work)(struct work_struct *work);
118122
};
119123

120124
/*
@@ -591,9 +595,12 @@ extern void afs_proc_cell_remove(struct afs_cell *);
591595
* rxrpc.c
592596
*/
593597
extern struct socket *afs_socket;
598+
extern atomic_t afs_outstanding_calls;
594599

595600
extern int afs_open_socket(void);
596601
extern void afs_close_socket(void);
602+
extern void afs_put_call(struct afs_call *);
603+
extern int afs_queue_call_work(struct afs_call *);
597604
extern int afs_make_call(struct in_addr *, struct afs_call *, gfp_t, bool);
598605
extern struct afs_call *afs_alloc_flat_call(const struct afs_call_type *,
599606
size_t, size_t);

0 commit comments

Comments
 (0)