All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] libceph: don't access invalid memory in keepalive2 path
@ 2015-09-14 13:50 Ilya Dryomov
  2015-09-16  6:33 ` Yan, Zheng
  0 siblings, 1 reply; 2+ messages in thread
From: Ilya Dryomov @ 2015-09-14 13:50 UTC (permalink / raw)
  To: ceph-devel

This

    struct ceph_timespec ceph_ts;
    ...
    con_out_kvec_add(con, sizeof(ceph_ts), &ceph_ts);

wraps ceph_ts into a kvec and adds it to con->out_kvec array, yet
ceph_ts becomes invalid on return from prepare_write_keepalive().  As
a result, we send out bogus keepalive2 stamps.  Fix this by encoding
into a ceph_timespec member, similar to how acks are read and written.

Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
---
 include/linux/ceph/messenger.h | 4 +++-
 net/ceph/messenger.c           | 9 +++++----
 2 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/include/linux/ceph/messenger.h b/include/linux/ceph/messenger.h
index 7e1252e97a30..b2371d9b51fa 100644
--- a/include/linux/ceph/messenger.h
+++ b/include/linux/ceph/messenger.h
@@ -238,6 +238,8 @@ struct ceph_connection {
 	bool out_kvec_is_msg; /* kvec refers to out_msg */
 	int out_more;        /* there is more data after the kvecs */
 	__le64 out_temp_ack; /* for writing an ack */
+	struct ceph_timespec out_temp_keepalive2; /* for writing keepalive2
+						     stamp */
 
 	/* message in temps */
 	struct ceph_msg_header in_hdr;
@@ -248,7 +250,7 @@ struct ceph_connection {
 	int in_base_pos;     /* bytes read */
 	__le64 in_temp_ack;  /* for reading an ack */
 
-	struct timespec last_keepalive_ack;
+	struct timespec last_keepalive_ack; /* keepalive2 ack stamp */
 
 	struct delayed_work work;	    /* send|recv work */
 	unsigned long       delay;          /* current delay interval */
diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
index 525f454f7531..b9b0e3b5da49 100644
--- a/net/ceph/messenger.c
+++ b/net/ceph/messenger.c
@@ -1353,11 +1353,12 @@ static void prepare_write_keepalive(struct ceph_connection *con)
 	dout("prepare_write_keepalive %p\n", con);
 	con_out_kvec_reset(con);
 	if (con->peer_features & CEPH_FEATURE_MSGR_KEEPALIVE2) {
-		struct timespec ts = CURRENT_TIME;
-		struct ceph_timespec ceph_ts;
-		ceph_encode_timespec(&ceph_ts, &ts);
+		struct timespec now = CURRENT_TIME;
+
 		con_out_kvec_add(con, sizeof(tag_keepalive2), &tag_keepalive2);
-		con_out_kvec_add(con, sizeof(ceph_ts), &ceph_ts);
+		ceph_encode_timespec(&con->out_temp_keepalive2, &now);
+		con_out_kvec_add(con, sizeof(con->out_temp_keepalive2),
+				 &con->out_temp_keepalive2);
 	} else {
 		con_out_kvec_add(con, sizeof(tag_keepalive), &tag_keepalive);
 	}
-- 
1.9.3


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH] libceph: don't access invalid memory in keepalive2 path
  2015-09-14 13:50 [PATCH] libceph: don't access invalid memory in keepalive2 path Ilya Dryomov
@ 2015-09-16  6:33 ` Yan, Zheng
  0 siblings, 0 replies; 2+ messages in thread
From: Yan, Zheng @ 2015-09-16  6:33 UTC (permalink / raw)
  To: Ilya Dryomov; +Cc: ceph-devel

On Mon, Sep 14, 2015 at 9:50 PM, Ilya Dryomov <idryomov@gmail.com> wrote:
> This
>
>     struct ceph_timespec ceph_ts;
>     ...
>     con_out_kvec_add(con, sizeof(ceph_ts), &ceph_ts);
>
> wraps ceph_ts into a kvec and adds it to con->out_kvec array, yet
> ceph_ts becomes invalid on return from prepare_write_keepalive().  As
> a result, we send out bogus keepalive2 stamps.  Fix this by encoding
> into a ceph_timespec member, similar to how acks are read and written.
>
> Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
> ---
>  include/linux/ceph/messenger.h | 4 +++-
>  net/ceph/messenger.c           | 9 +++++----
>  2 files changed, 8 insertions(+), 5 deletions(-)
>
> diff --git a/include/linux/ceph/messenger.h b/include/linux/ceph/messenger.h
> index 7e1252e97a30..b2371d9b51fa 100644
> --- a/include/linux/ceph/messenger.h
> +++ b/include/linux/ceph/messenger.h
> @@ -238,6 +238,8 @@ struct ceph_connection {
>         bool out_kvec_is_msg; /* kvec refers to out_msg */
>         int out_more;        /* there is more data after the kvecs */
>         __le64 out_temp_ack; /* for writing an ack */
> +       struct ceph_timespec out_temp_keepalive2; /* for writing keepalive2
> +                                                    stamp */
>
>         /* message in temps */
>         struct ceph_msg_header in_hdr;
> @@ -248,7 +250,7 @@ struct ceph_connection {
>         int in_base_pos;     /* bytes read */
>         __le64 in_temp_ack;  /* for reading an ack */
>
> -       struct timespec last_keepalive_ack;
> +       struct timespec last_keepalive_ack; /* keepalive2 ack stamp */
>
>         struct delayed_work work;           /* send|recv work */
>         unsigned long       delay;          /* current delay interval */
> diff --git a/net/ceph/messenger.c b/net/ceph/messenger.c
> index 525f454f7531..b9b0e3b5da49 100644
> --- a/net/ceph/messenger.c
> +++ b/net/ceph/messenger.c
> @@ -1353,11 +1353,12 @@ static void prepare_write_keepalive(struct ceph_connection *con)
>         dout("prepare_write_keepalive %p\n", con);
>         con_out_kvec_reset(con);
>         if (con->peer_features & CEPH_FEATURE_MSGR_KEEPALIVE2) {
> -               struct timespec ts = CURRENT_TIME;
> -               struct ceph_timespec ceph_ts;
> -               ceph_encode_timespec(&ceph_ts, &ts);
> +               struct timespec now = CURRENT_TIME;
> +
>                 con_out_kvec_add(con, sizeof(tag_keepalive2), &tag_keepalive2);
> -               con_out_kvec_add(con, sizeof(ceph_ts), &ceph_ts);
> +               ceph_encode_timespec(&con->out_temp_keepalive2, &now);
> +               con_out_kvec_add(con, sizeof(con->out_temp_keepalive2),
> +                                &con->out_temp_keepalive2);
>         } else {
>                 con_out_kvec_add(con, sizeof(tag_keepalive), &tag_keepalive);
>         }
> --
Sorry for introducing this bug


Reviewed-by: Yan, Zheng <zyan@redhat.com>

> 1.9.3
>
> --
> To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2015-09-16  6:33 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-09-14 13:50 [PATCH] libceph: don't access invalid memory in keepalive2 path Ilya Dryomov
2015-09-16  6:33 ` Yan, Zheng

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.