| Message ID | 20161009231936.GA24139@obsidianresearch.com (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On 10/10/2016 2:19 AM, Jason Gunthorpe wrote: > On Sun, Oct 09, 2016 at 04:31:17PM +0300, Yishai Hadas wrote: > >>> - if (write(connfd, msg, sizeof msg) != sizeof msg) { >>> - fprintf(stderr, "Couldn't send local address\n"); >>> + if (write(connfd, msg, sizeof msg) != sizeof msg || >>> + read(connfd, msg, sizeof msg) != sizeof msg) { >>> + fprintf(stderr, "Couldn't send/recv local address\n"); >> >> At that step the server expects to read a "done" response from its client, >> checking whether the read was done for sizeof msg which is much larger will >> fail. > > This OK? Yes, below patch solves the issue. > From db525af53140c3b7604ab45406ed8845cb6171e1 Mon Sep 17 00:00:00 2001 > From: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > Date: Sun, 9 Oct 2016 17:17:54 -0600 > Subject: [PATCH] verbs: Fix read error check in pingpong > > Turns out these reads rely on the short readed caused by EOS. > > Fixes: f3912df771db (Avoid gcc 5.4 warning -Wunused-result) > Signed-off-by: Jason Gunthorpe <jgunthorpe@obsidianresearch.com> > --- > libibverbs/examples/rc_pingpong.c | 2 +- > libibverbs/examples/srq_pingpong.c | 2 +- > libibverbs/examples/uc_pingpong.c | 2 +- > libibverbs/examples/ud_pingpong.c | 2 +- > 4 files changed, 4 insertions(+), 4 deletions(-) > > diff --git a/libibverbs/examples/rc_pingpong.c b/libibverbs/examples/rc_pingpong.c > index c92e551c00e6..aca7bac4491b 100644 > --- a/libibverbs/examples/rc_pingpong.c > +++ b/libibverbs/examples/rc_pingpong.c > @@ -309,7 +309,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx, > sprintf(msg, "%04x:%06x:%06x:%s", my_dest->lid, my_dest->qpn, > my_dest->psn, gid); > if (write(connfd, msg, sizeof msg) != sizeof msg || > - read(connfd, msg, sizeof msg) != sizeof msg) { > + read(connfd, msg, sizeof msg) != sizeof "done") { > fprintf(stderr, "Couldn't send/recv local address\n"); > free(rem_dest); > rem_dest = NULL; > diff --git a/libibverbs/examples/srq_pingpong.c b/libibverbs/examples/srq_pingpong.c > index 863ff10dd0f4..8f55d78e87d4 100644 > --- a/libibverbs/examples/srq_pingpong.c > +++ b/libibverbs/examples/srq_pingpong.c > @@ -333,7 +333,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx, > } > } > > - if (read(connfd, msg, sizeof msg) != sizeof msg) { > + if (read(connfd, msg, sizeof msg) != sizeof "done") { > perror("client write"); > free(rem_dest); > rem_dest = NULL; > diff --git a/libibverbs/examples/uc_pingpong.c b/libibverbs/examples/uc_pingpong.c > index 2b105b947cf3..b565bacaff2a 100644 > --- a/libibverbs/examples/uc_pingpong.c > +++ b/libibverbs/examples/uc_pingpong.c > @@ -283,7 +283,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx, > sprintf(msg, "%04x:%06x:%06x:%s", my_dest->lid, my_dest->qpn, > my_dest->psn, gid); > if (write(connfd, msg, sizeof msg) != sizeof msg || > - read(connfd, msg, sizeof msg) != sizeof msg) { > + read(connfd, msg, sizeof msg) != sizeof "done") { > fprintf(stderr, "Couldn't send/recv local address\n"); > free(rem_dest); > rem_dest = NULL; > diff --git a/libibverbs/examples/ud_pingpong.c b/libibverbs/examples/ud_pingpong.c > index d0cd73cc0fae..ddb68cf8624b 100644 > --- a/libibverbs/examples/ud_pingpong.c > +++ b/libibverbs/examples/ud_pingpong.c > @@ -280,7 +280,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx, > sprintf(msg, "%04x:%06x:%06x:%s", my_dest->lid, my_dest->qpn, > my_dest->psn, gid); > if (write(connfd, msg, sizeof msg) != sizeof msg || > - read(connfd, msg, sizeof msg) != sizeof msg) { > + read(connfd, msg, sizeof msg) != sizeof "done") { > fprintf(stderr, "Couldn't send/recv local address\n"); > free(rem_dest); > rem_dest = NULL; > -- To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 10/10/2016 6:06 AM, Yishai Hadas wrote: > On 10/10/2016 2:19 AM, Jason Gunthorpe wrote: >> On Sun, Oct 09, 2016 at 04:31:17PM +0300, Yishai Hadas wrote: >> >>>> - if (write(connfd, msg, sizeof msg) != sizeof msg) { >>>> - fprintf(stderr, "Couldn't send local address\n"); >>>> + if (write(connfd, msg, sizeof msg) != sizeof msg || >>>> + read(connfd, msg, sizeof msg) != sizeof msg) { >>>> + fprintf(stderr, "Couldn't send/recv local address\n"); >>> >>> At that step the server expects to read a "done" response from its >>> client, >>> checking whether the read was done for sizeof msg which is much >>> larger will >>> fail. >> >> This OK? > > Yes, below patch solves the issue. Merged, thanks.
diff --git a/libibverbs/examples/rc_pingpong.c b/libibverbs/examples/rc_pingpong.c index c92e551c00e6..aca7bac4491b 100644 --- a/libibverbs/examples/rc_pingpong.c +++ b/libibverbs/examples/rc_pingpong.c @@ -309,7 +309,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx, sprintf(msg, "%04x:%06x:%06x:%s", my_dest->lid, my_dest->qpn, my_dest->psn, gid); if (write(connfd, msg, sizeof msg) != sizeof msg || - read(connfd, msg, sizeof msg) != sizeof msg) { + read(connfd, msg, sizeof msg) != sizeof "done") { fprintf(stderr, "Couldn't send/recv local address\n"); free(rem_dest); rem_dest = NULL; diff --git a/libibverbs/examples/srq_pingpong.c b/libibverbs/examples/srq_pingpong.c index 863ff10dd0f4..8f55d78e87d4 100644 --- a/libibverbs/examples/srq_pingpong.c +++ b/libibverbs/examples/srq_pingpong.c @@ -333,7 +333,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx, } } - if (read(connfd, msg, sizeof msg) != sizeof msg) { + if (read(connfd, msg, sizeof msg) != sizeof "done") { perror("client write"); free(rem_dest); rem_dest = NULL; diff --git a/libibverbs/examples/uc_pingpong.c b/libibverbs/examples/uc_pingpong.c index 2b105b947cf3..b565bacaff2a 100644 --- a/libibverbs/examples/uc_pingpong.c +++ b/libibverbs/examples/uc_pingpong.c @@ -283,7 +283,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx, sprintf(msg, "%04x:%06x:%06x:%s", my_dest->lid, my_dest->qpn, my_dest->psn, gid); if (write(connfd, msg, sizeof msg) != sizeof msg || - read(connfd, msg, sizeof msg) != sizeof msg) { + read(connfd, msg, sizeof msg) != sizeof "done") { fprintf(stderr, "Couldn't send/recv local address\n"); free(rem_dest); rem_dest = NULL; diff --git a/libibverbs/examples/ud_pingpong.c b/libibverbs/examples/ud_pingpong.c index d0cd73cc0fae..ddb68cf8624b 100644 --- a/libibverbs/examples/ud_pingpong.c +++ b/libibverbs/examples/ud_pingpong.c @@ -280,7 +280,7 @@ static struct pingpong_dest *pp_server_exch_dest(struct pingpong_context *ctx, sprintf(msg, "%04x:%06x:%06x:%s", my_dest->lid, my_dest->qpn, my_dest->psn, gid); if (write(connfd, msg, sizeof msg) != sizeof msg || - read(connfd, msg, sizeof msg) != sizeof msg) { + read(connfd, msg, sizeof msg) != sizeof "done") { fprintf(stderr, "Couldn't send/recv local address\n"); free(rem_dest); rem_dest = NULL;