[PATCH] libceph: skip message if too big to receive

[PATCH] libceph: skip message if too big to receive

[Alex Elder]

We know the length of our message buffers.  If we get a message
that's too long, just dump it and ignore it.

This resolves:
    http://tracker.ceph.com/issues/4664

Signed-off-by: Alex Elder <elder@inktank.com>
---
 net/ceph/messenger.c |    6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index 994192b..ae825e44 100644
--- a/net/ceph/messenger.c
+++ b/net/ceph/messenger.c
@@ -2207,6 +2207,12 @@ static int read_partial_message(struct
ceph_connection *con)
 		ret = ceph_con_in_msg_alloc(con, &skip);
 		if (ret < 0)
 			return ret;
+
+		if (data_len > con->in_msg->data_length) {
+			pr_warning("%s skipping long message (%u > %zd)\n",
+				__func__, data_len, con->in_msg->data_length);
+			skip = 1;
+		}
 		if (skip) {
 			/* skip this message */
 			dout("alloc_msg said skip message\n");
-- 
1.7.9.5

[PATCH, v2] libceph: skip message if too big to receive

[Alex Elder]

I found a bug in this and am posting the following
update.  If a connection's alloc_msg() method sets
the skip flag, it will return with con->in_msg being
a null pointer.  The original version of this would
dereference that pointer without checking, which
causes a crash.  This version checks first.

(This and the updated patches that follow it are
available in the "review/wip-3761-4" branch of the
ceph-client git repository.)

					-Alex

We know the length of our message buffers.  If we get a message
that's too long, just dump it and ignore it.  If skip was set
then con->in_msg won't be valid, so be careful not to dereference
a null pointer in the process.

This resolves:
    http://tracker.ceph.com/issues/4664

Signed-off-by: Alex Elder <elder@inktank.com>
---
v2: make sure con->in_msg is valid before dereferencing it

 net/ceph/messenger.c |   10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index 994192b..cb5b4e6 100644
--- a/net/ceph/messenger.c
+++ b/net/ceph/messenger.c
@@ -2207,10 +2207,18 @@ static int read_partial_message(struct
ceph_connection *con)
 		ret = ceph_con_in_msg_alloc(con, &skip);
 		if (ret < 0)
 			return ret;
+
+		BUG_ON(!con->in_msg ^ skip);
+		if (con->in_msg && data_len > con->in_msg->data_length) {
+			pr_warning("%s skipping long message (%u > %zd)\n",
+				__func__, data_len, con->in_msg->data_length);
+			ceph_msg_put(con->in_msg);
+			con->in_msg = NULL;
+			skip = 1;
+		}
 		if (skip) {
 			/* skip this message */
 			dout("alloc_msg said skip message\n");
-			BUG_ON(con->in_msg);
 			con->in_base_pos = -front_len - middle_len - data_len -
 				sizeof(m->footer);
 			con->in_tag = CEPH_MSGR_TAG_READY;
-- 
1.7.9.5

Re: [PATCH, v2] libceph: skip message if too big to receive

[Josh Durgin]

Assuming we'll come back and clean this up soon:

Reviewed-by: Josh Durgin <josh.durgin@inktank.com>

On 04/06/2013 01:42 PM, Alex Elder wrote:
> I found a bug in this and am posting the following
> update.  If a connection's alloc_msg() method sets
> the skip flag, it will return with con->in_msg being
> a null pointer.  The original version of this would
> dereference that pointer without checking, which
> causes a crash.  This version checks first.
>
> (This and the updated patches that follow it are
> available in the "review/wip-3761-4" branch of the
> ceph-client git repository.)
>
> 					-Alex
>
> We know the length of our message buffers.  If we get a message
> that's too long, just dump it and ignore it.  If skip was set
> then con->in_msg won't be valid, so be careful not to dereference
> a null pointer in the process.
>
> This resolves:
>      http://tracker.ceph.com/issues/4664
>
> Signed-off-by: Alex Elder <elder@inktank.com>
> ---
> v2: make sure con->in_msg is valid before dereferencing it
>
>   net/ceph/messenger.c |   10 +++++++++-
>   1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
> index 994192b..cb5b4e6 100644
> --- a/net/ceph/messenger.c
> +++ b/net/ceph/messenger.c
> @@ -2207,10 +2207,18 @@ static int read_partial_message(struct
> ceph_connection *con)
>   		ret = ceph_con_in_msg_alloc(con, &skip);
>   		if (ret < 0)
>   			return ret;
> +
> +		BUG_ON(!con->in_msg ^ skip);
> +		if (con->in_msg && data_len > con->in_msg->data_length) {
> +			pr_warning("%s skipping long message (%u > %zd)\n",
> +				__func__, data_len, con->in_msg->data_length);
> +			ceph_msg_put(con->in_msg);
> +			con->in_msg = NULL;
> +			skip = 1;
> +		}
>   		if (skip) {
>   			/* skip this message */
>   			dout("alloc_msg said skip message\n");
> -			BUG_ON(con->in_msg);
>   			con->in_base_pos = -front_len - middle_len - data_len -
>   				sizeof(m->footer);
>   			con->in_tag = CEPH_MSGR_TAG_READY;
>