Skip to content

Commit

Permalink
rxrpc: Don't expose skbs to in-kernel users [ver andy-shev#2]
Browse files Browse the repository at this point in the history 
Don't expose skbs to in-kernel users, such as the AFS filesystem, but
instead provide a notification hook the indicates that a call needs
attention and another that indicates that there's a new call to be
collected.

This makes the following possibilities more achievable:

 (1) Call refcounting can be made simpler if skbs don't hold refs to calls.

 (2) skbs referring to non-data events will be able to be freed much sooner
     rather than being queued for AFS to pick up as rxrpc_kernel_recv_data
     will be able to consult the call state.

 (3) We can shortcut the receive phase when a call is remotely aborted
     because we don't have to go through all the packets to get to the one
     cancelling the operation.

 (4) It makes it easier to do encryption/decryption directly between AFS's
     buffers and sk_buffs.

 (5) Encryption/decryption can more easily be done in the AFS's thread
     contexts - usually that of the userspace process that issued a syscall
     - rather than in one of rxrpc's background threads on a workqueue.

 (6) AFS will be able to wait synchronously on a call inside AF_RXRPC.

To make this work, the following interface function has been added:

     int rxrpc_kernel_recv_data(
		struct socket *sock, struct rxrpc_call *call,
		void *buffer, size_t bufsize, size_t *_offset,
		bool want_more, u32 *_abort_code);

This is the recvmsg equivalent.  It allows the caller to find out about the
state of a specific call and to transfer received data into a buffer
piecemeal.

afs_extract_data() and rxrpc_kernel_recv_data() now do all the extraction
logic between them.  They don't wait synchronously yet because the socket
lock needs to be dealt with.

Five interface functions have been removed:

	rxrpc_kernel_is_data_last()
    	rxrpc_kernel_get_abort_code()
    	rxrpc_kernel_get_error_number()
    	rxrpc_kernel_free_skb()
    	rxrpc_kernel_data_consumed()

As a temporary hack, sk_buffs going to an in-kernel call are queued on the
rxrpc_call struct (->knlrecv_queue) rather than being handed over to the
in-kernel user.  To process the queue internally, a temporary function,
temp_deliver_data() has been added.  This will be replaced with common code
between the rxrpc_recvmsg() path and the kernel_rxrpc_recv_data() path in a
future patch.

Signed-off-by: David Howells <dhowells@redhat.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
  • Loading branch information
@dhowells @davem330
dhowells authored and davem330 committed Sep 1, 2016
1 parent 95ac399 commit d001648
Show file tree
Hide file tree
Showing 16 changed files with 565 additions and 586 deletions.
72 changes: 31 additions & 41 deletions Documentation/networking/rxrpc.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -748,6 +748,37 @@ The kernel interface functions are as follows:
The msg must not specify a destination address, control data or any flags
other than MSG_MORE. len is the total amount of data to transmit.

(*) Receive data from a call.

int rxrpc_kernel_recv_data(struct socket *sock,
struct rxrpc_call *call,
void *buf,
size_t size,
size_t *_offset,
bool want_more,
u32 *_abort)

This is used to receive data from either the reply part of a client call
or the request part of a service call. buf and size specify how much
data is desired and where to store it. *_offset is added on to buf and
subtracted from size internally; the amount copied into the buffer is
added to *_offset before returning.

want_more should be true if further data will be required after this is
satisfied and false if this is the last item of the receive phase.

There are three normal returns: 0 if the buffer was filled and want_more
was true; 1 if the buffer was filled, the last DATA packet has been
emptied and want_more was false; and -EAGAIN if the function needs to be
called again.

If the last DATA packet is processed but the buffer contains less than
the amount requested, EBADMSG is returned. If want_more wasn't set, but
more data was available, EMSGSIZE is returned.

If a remote ABORT is detected, the abort code received will be stored in
*_abort and ECONNABORTED will be returned.

(*) Abort a call.

void rxrpc_kernel_abort_call(struct socket *sock,
Expand Down Expand Up @@ -825,47 +856,6 @@ The kernel interface functions are as follows:
Other errors may be returned if the call had been aborted (-ECONNABORTED)
or had timed out (-ETIME).

(*) Record the delivery of a data message.

void rxrpc_kernel_data_consumed(struct rxrpc_call *call,
struct sk_buff *skb);

This is used to record a data message as having been consumed and to
update the ACK state for the call. The message must still be passed to
rxrpc_kernel_free_skb() for disposal by the caller.

(*) Free a message.

void rxrpc_kernel_free_skb(struct sk_buff *skb);

This is used to free a non-DATA socket buffer intercepted from an AF_RXRPC
socket.

(*) Determine if a data message is the last one on a call.

bool rxrpc_kernel_is_data_last(struct sk_buff *skb);

This is used to determine if a socket buffer holds the last data message
to be received for a call (true will be returned if it does, false
if not).

The data message will be part of the reply on a client call and the
request on an incoming call. In the latter case there will be more
messages, but in the former case there will not.

(*) Get the abort code from an abort message.

u32 rxrpc_kernel_get_abort_code(struct sk_buff *skb);

This is used to extract the abort code from a remote abort message.

(*) Get the error number from a local or network error message.

int rxrpc_kernel_get_error_number(struct sk_buff *skb);

This is used to extract the error number from a message indicating either
a local error occurred or a network error occurred.

(*) Allocate a null key for doing anonymous security.

struct key *rxrpc_get_null_key(const char *keyname);
Expand Down
142 changes: 77 additions & 65 deletions fs/afs/cmservice.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,12 @@
#include "internal.h"
#include "afs_cm.h"

static int afs_deliver_cb_init_call_back_state(struct afs_call *,
struct sk_buff *, bool);
static int afs_deliver_cb_init_call_back_state3(struct afs_call *,
struct sk_buff *, bool);
static int afs_deliver_cb_probe(struct afs_call *, struct sk_buff *, bool);
static int afs_deliver_cb_callback(struct afs_call *, struct sk_buff *, bool);
static int afs_deliver_cb_probe_uuid(struct afs_call *, struct sk_buff *, bool);
static int afs_deliver_cb_tell_me_about_yourself(struct afs_call *,
struct sk_buff *, bool);
static int afs_deliver_cb_init_call_back_state(struct afs_call *);
static int afs_deliver_cb_init_call_back_state3(struct afs_call *);
static int afs_deliver_cb_probe(struct afs_call *);
static int afs_deliver_cb_callback(struct afs_call *);
static int afs_deliver_cb_probe_uuid(struct afs_call *);
static int afs_deliver_cb_tell_me_about_yourself(struct afs_call *);
static void afs_cm_destructor(struct afs_call *);

/*
Expand Down Expand Up @@ -130,7 +127,7 @@ static void afs_cm_destructor(struct afs_call *call)
* received. The step number here must match the final number in
* afs_deliver_cb_callback().
*/
if (call->unmarshall == 6) {
if (call->unmarshall == 5) {
ASSERT(call->server && call->count && call->request);
afs_break_callbacks(call->server, call->count, call->request);
}
Expand Down Expand Up @@ -164,8 +161,7 @@ static void SRXAFSCB_CallBack(struct work_struct *work)
/*
* deliver request data to a CB.CallBack call
*/
static int afs_deliver_cb_callback(struct afs_call *call, struct sk_buff *skb,
bool last)
static int afs_deliver_cb_callback(struct afs_call *call)
{
struct sockaddr_rxrpc srx;
struct afs_callback *cb;
Expand All @@ -174,7 +170,7 @@ static int afs_deliver_cb_callback(struct afs_call *call, struct sk_buff *skb,
u32 tmp;
int ret, loop;

_enter("{%u},{%u},%d", call->unmarshall, skb->len, last);
_enter("{%u}", call->unmarshall);

switch (call->unmarshall) {
case 0:
Expand All @@ -185,7 +181,7 @@ static int afs_deliver_cb_callback(struct afs_call *call, struct sk_buff *skb,
/* extract the FID array and its count in two steps */
case 1:
_debug("extract FID count");
ret = afs_extract_data(call, skb, last, &call->tmp, 4);
ret = afs_extract_data(call, &call->tmp, 4, true);
if (ret < 0)
return ret;

Expand All @@ -202,8 +198,8 @@ static int afs_deliver_cb_callback(struct afs_call *call, struct sk_buff *skb,

case 2:
_debug("extract FID array");
ret = afs_extract_data(call, skb, last, call->buffer,
call->count * 3 * 4);
ret = afs_extract_data(call, call->buffer,
call->count * 3 * 4, true);
if (ret < 0)
return ret;

Expand All @@ -229,7 +225,7 @@ static int afs_deliver_cb_callback(struct afs_call *call, struct sk_buff *skb,
/* extract the callback array and its count in two steps */
case 3:
_debug("extract CB count");
ret = afs_extract_data(call, skb, last, &call->tmp, 4);
ret = afs_extract_data(call, &call->tmp, 4, true);
if (ret < 0)
return ret;

Expand All @@ -239,13 +235,11 @@ static int afs_deliver_cb_callback(struct afs_call *call, struct sk_buff *skb,
return -EBADMSG;
call->offset = 0;
call->unmarshall++;
if (tmp == 0)
goto empty_cb_array;

case 4:
_debug("extract CB array");
ret = afs_extract_data(call, skb, last, call->request,
call->count * 3 * 4);
ret = afs_extract_data(call, call->buffer,
call->count * 3 * 4, false);
if (ret < 0)
return ret;

Expand All @@ -258,15 +252,9 @@ static int afs_deliver_cb_callback(struct afs_call *call, struct sk_buff *skb,
cb->type = ntohl(*bp++);
}

empty_cb_array:
call->offset = 0;
call->unmarshall++;

case 5:
ret = afs_data_complete(call, skb, last);
if (ret < 0)
return ret;

/* Record that the message was unmarshalled successfully so
* that the call destructor can know do the callback breaking
* work, even if the final ACK isn't received.
Expand All @@ -275,7 +263,7 @@ static int afs_deliver_cb_callback(struct afs_call *call, struct sk_buff *skb,
* updated also.
*/
call->unmarshall++;
case 6:
case 5:
break;
}

Expand Down Expand Up @@ -310,19 +298,17 @@ static void SRXAFSCB_InitCallBackState(struct work_struct *work)
/*
* deliver request data to a CB.InitCallBackState call
*/
static int afs_deliver_cb_init_call_back_state(struct afs_call *call,
struct sk_buff *skb,
bool last)
static int afs_deliver_cb_init_call_back_state(struct afs_call *call)
{
struct sockaddr_rxrpc srx;
struct afs_server *server;
int ret;

_enter(",{%u},%d", skb->len, last);
_enter("");

rxrpc_kernel_get_peer(afs_socket, call->rxcall, &srx);

ret = afs_data_complete(call, skb, last);
ret = afs_extract_data(call, NULL, 0, false);
if (ret < 0)
return ret;

Expand All @@ -344,21 +330,61 @@ static int afs_deliver_cb_init_call_back_state(struct afs_call *call,
/*
* deliver request data to a CB.InitCallBackState3 call
*/
static int afs_deliver_cb_init_call_back_state3(struct afs_call *call,
struct sk_buff *skb,
bool last)
static int afs_deliver_cb_init_call_back_state3(struct afs_call *call)
{
struct sockaddr_rxrpc srx;
struct afs_server *server;
struct afs_uuid *r;
unsigned loop;
__be32 *b;
int ret;

_enter(",{%u},%d", skb->len, last);
_enter("");

rxrpc_kernel_get_peer(afs_socket, call->rxcall, &srx);

/* There are some arguments that we ignore */
afs_data_consumed(call, skb);
if (!last)
return -EAGAIN;
_enter("{%u}", call->unmarshall);

switch (call->unmarshall) {
case 0:
call->offset = 0;
call->buffer = kmalloc(11 * sizeof(__be32), GFP_KERNEL);
if (!call->buffer)
return -ENOMEM;
call->unmarshall++;

case 1:
_debug("extract UUID");
ret = afs_extract_data(call, call->buffer,
11 * sizeof(__be32), false);
switch (ret) {
case 0: break;
case -EAGAIN: return 0;
default: return ret;
}

_debug("unmarshall UUID");
call->request = kmalloc(sizeof(struct afs_uuid), GFP_KERNEL);
if (!call->request)
return -ENOMEM;

b = call->buffer;
r = call->request;
r->time_low = ntohl(b[0]);
r->time_mid = ntohl(b[1]);
r->time_hi_and_version = ntohl(b[2]);
r->clock_seq_hi_and_reserved = ntohl(b[3]);
r->clock_seq_low = ntohl(b[4]);

for (loop = 0; loop < 6; loop++)
r->node[loop] = ntohl(b[loop + 5]);

call->offset = 0;
call->unmarshall++;

case 2:
break;
}

/* no unmarshalling required */
call->state = AFS_CALL_REPLYING;
Expand Down Expand Up @@ -390,14 +416,13 @@ static void SRXAFSCB_Probe(struct work_struct *work)
/*
* deliver request data to a CB.Probe call
*/
static int afs_deliver_cb_probe(struct afs_call *call, struct sk_buff *skb,
bool last)
static int afs_deliver_cb_probe(struct afs_call *call)
{
int ret;

_enter(",{%u},%d", skb->len, last);
_enter("");

ret = afs_data_complete(call, skb, last);
ret = afs_extract_data(call, NULL, 0, false);
if (ret < 0)
return ret;

Expand Down Expand Up @@ -435,19 +460,14 @@ static void SRXAFSCB_ProbeUuid(struct work_struct *work)
/*
* deliver request data to a CB.ProbeUuid call
*/
static int afs_deliver_cb_probe_uuid(struct afs_call *call, struct sk_buff *skb,
bool last)
static int afs_deliver_cb_probe_uuid(struct afs_call *call)
{
struct afs_uuid *r;
unsigned loop;
__be32 *b;
int ret;

_enter("{%u},{%u},%d", call->unmarshall, skb->len, last);

ret = afs_data_complete(call, skb, last);
if (ret < 0)
return ret;
_enter("{%u}", call->unmarshall);

switch (call->unmarshall) {
case 0:
Expand All @@ -459,8 +479,8 @@ static int afs_deliver_cb_probe_uuid(struct afs_call *call, struct sk_buff *skb,

case 1:
_debug("extract UUID");
ret = afs_extract_data(call, skb, last, call->buffer,
11 * sizeof(__be32));
ret = afs_extract_data(call, call->buffer,
11 * sizeof(__be32), false);
switch (ret) {
case 0: break;
case -EAGAIN: return 0;
Expand All @@ -487,16 +507,9 @@ static int afs_deliver_cb_probe_uuid(struct afs_call *call, struct sk_buff *skb,
call->unmarshall++;

case 2:
_debug("trailer");
if (skb->len != 0)
return -EBADMSG;
break;
}

ret = afs_data_complete(call, skb, last);
if (ret < 0)
return ret;

call->state = AFS_CALL_REPLYING;

INIT_WORK(&call->work, SRXAFSCB_ProbeUuid);
Expand Down Expand Up @@ -570,14 +583,13 @@ static void SRXAFSCB_TellMeAboutYourself(struct work_struct *work)
/*
* deliver request data to a CB.TellMeAboutYourself call
*/
static int afs_deliver_cb_tell_me_about_yourself(struct afs_call *call,
struct sk_buff *skb, bool last)
static int afs_deliver_cb_tell_me_about_yourself(struct afs_call *call)
{
int ret;

_enter(",{%u},%d", skb->len, last);
_enter("");

ret = afs_data_complete(call, skb, last);
ret = afs_extract_data(call, NULL, 0, false);
if (ret < 0)
return ret;

Expand Down
Loading

0 comments on commit d001648

Please sign in to comment.