* [bug report] RDMA/rtrs: server: main functionality
@ 2020-05-19 12:02 Dan Carpenter
2020-05-19 15:07 ` Danil Kipnis
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2020-05-19 12:02 UTC (permalink / raw)
To: jinpu.wang; +Cc: linux-rdma
Hello Jack Wang,
The patch 9cb837480424: "RDMA/rtrs: server: main functionality" from
May 11, 2020, leads to the following static checker warning:
drivers/infiniband/ulp/rtrs/rtrs-srv.c:1224 rtrs_srv_rdma_done()
warn: array off by one? 'sess->mrs[msg_id]'
drivers/infiniband/ulp/rtrs/rtrs-srv.c
1207 }
1208 rtrs_from_imm(be32_to_cpu(wc->ex.imm_data),
1209 &imm_type, &imm_payload);
1210 if (likely(imm_type == RTRS_IO_REQ_IMM)) {
1211 u32 msg_id, off;
1212 void *data;
1213
1214 msg_id = imm_payload >> sess->mem_bits;
1215 off = imm_payload & ((1 << sess->mem_bits) - 1);
1216 if (unlikely(msg_id > srv->queue_depth ||
^
This should definitely be >=
1217 off > max_chunk_size)) {
^
My only question is should "off" be >=. I feel like probably it should
but I'm not sure.
1218 rtrs_err(s, "Wrong msg_id %u, off %u\n",
1219 msg_id, off);
1220 close_sess(sess);
1221 return;
1222 }
1223 if (always_invalidate) {
1224 struct rtrs_srv_mr *mr = &sess->mrs[msg_id];
^^^^^^^^^^^^^^^^^^
1225
1226 mr->msg_off = off;
1227 mr->msg_id = msg_id;
1228 err = rtrs_srv_inv_rkey(con, mr);
1229 if (unlikely(err)) {
1230 rtrs_err(s, "rtrs_post_recv(), err: %d\n",
1231 err);
1232 close_sess(sess);
1233 break;
1234 }
1235 } else {
1236 data = page_address(srv->chunks[msg_id]) + off;
1237 process_io_req(con, data, msg_id, off);
1238 }
1239 } else if (imm_type == RTRS_HB_MSG_IMM) {
1240 WARN_ON(con->c.cid);
1241 rtrs_send_hb_ack(&sess->s);
1242 } else if (imm_type == RTRS_HB_ACK_IMM) {
regards,
dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [bug report] RDMA/rtrs: server: main functionality
2020-05-19 12:02 [bug report] RDMA/rtrs: server: main functionality Dan Carpenter
@ 2020-05-19 15:07 ` Danil Kipnis
2020-05-19 15:45 ` [PATCH] RDMA/rtrs: Fix a couple off by one bugs in rtrs_srv_rdma_done() Dan Carpenter
0 siblings, 1 reply; 5+ messages in thread
From: Danil Kipnis @ 2020-05-19 15:07 UTC (permalink / raw)
To: Dan Carpenter; +Cc: Jinpu Wang, linux-rdma
Hi Dan,
On Tue, May 19, 2020 at 2:02 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> Hello Jack Wang,
>
> The patch 9cb837480424: "RDMA/rtrs: server: main functionality" from
> May 11, 2020, leads to the following static checker warning:
>
> drivers/infiniband/ulp/rtrs/rtrs-srv.c:1224 rtrs_srv_rdma_done()
> warn: array off by one? 'sess->mrs[msg_id]'
>
> drivers/infiniband/ulp/rtrs/rtrs-srv.c
> 1207 }
> 1208 rtrs_from_imm(be32_to_cpu(wc->ex.imm_data),
> 1209 &imm_type, &imm_payload);
> 1210 if (likely(imm_type == RTRS_IO_REQ_IMM)) {
> 1211 u32 msg_id, off;
> 1212 void *data;
> 1213
> 1214 msg_id = imm_payload >> sess->mem_bits;
> 1215 off = imm_payload & ((1 << sess->mem_bits) - 1);
> 1216 if (unlikely(msg_id > srv->queue_depth ||
> ^
> This should definitely be >=
Definitely, thank you.
>
> 1217 off > max_chunk_size)) {
> ^
> My only question is should "off" be >=. I feel like probably it should
> but I'm not sure.
Here also, yes.
>
> 1218 rtrs_err(s, "Wrong msg_id %u, off %u\n",
> 1219 msg_id, off);
> 1220 close_sess(sess);
> 1221 return;
> 1222 }
> 1223 if (always_invalidate) {
> 1224 struct rtrs_srv_mr *mr = &sess->mrs[msg_id];
> ^^^^^^^^^^^^^^^^^^
> 1225
> 1226 mr->msg_off = off;
> 1227 mr->msg_id = msg_id;
> 1228 err = rtrs_srv_inv_rkey(con, mr);
> 1229 if (unlikely(err)) {
> 1230 rtrs_err(s, "rtrs_post_recv(), err: %d\n",
> 1231 err);
> 1232 close_sess(sess);
> 1233 break;
> 1234 }
> 1235 } else {
> 1236 data = page_address(srv->chunks[msg_id]) + off;
> 1237 process_io_req(con, data, msg_id, off);
> 1238 }
> 1239 } else if (imm_type == RTRS_HB_MSG_IMM) {
> 1240 WARN_ON(con->c.cid);
> 1241 rtrs_send_hb_ack(&sess->s);
> 1242 } else if (imm_type == RTRS_HB_ACK_IMM) {
>
> regards,
> dan carpenter
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] RDMA/rtrs: Fix a couple off by one bugs in rtrs_srv_rdma_done()
2020-05-19 15:07 ` Danil Kipnis
@ 2020-05-19 15:45 ` Dan Carpenter
2020-05-19 16:00 ` Danil Kipnis
2020-05-19 23:46 ` Jason Gunthorpe
0 siblings, 2 replies; 5+ messages in thread
From: Dan Carpenter @ 2020-05-19 15:45 UTC (permalink / raw)
To: Danil Kipnis
Cc: Jack Wang, Doug Ledford, Jason Gunthorpe, linux-rdma, kernel-janitors
These > comparisons should be >= to prevent accessing one element
beyond the end of the buffer.
Fixes: 9cb837480424 ("RDMA/rtrs: server: main functionality")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
---
drivers/infiniband/ulp/rtrs/rtrs-srv.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
index 658c8999cb0d..0b53b79b0e27 100644
--- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
+++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
@@ -1213,8 +1213,8 @@ static void rtrs_srv_rdma_done(struct ib_cq *cq, struct ib_wc *wc)
msg_id = imm_payload >> sess->mem_bits;
off = imm_payload & ((1 << sess->mem_bits) - 1);
- if (unlikely(msg_id > srv->queue_depth ||
- off > max_chunk_size)) {
+ if (unlikely(msg_id >= srv->queue_depth ||
+ off >= max_chunk_size)) {
rtrs_err(s, "Wrong msg_id %u, off %u\n",
msg_id, off);
close_sess(sess);
--
2.26.2
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] RDMA/rtrs: Fix a couple off by one bugs in rtrs_srv_rdma_done()
2020-05-19 15:45 ` [PATCH] RDMA/rtrs: Fix a couple off by one bugs in rtrs_srv_rdma_done() Dan Carpenter
@ 2020-05-19 16:00 ` Danil Kipnis
2020-05-19 23:46 ` Jason Gunthorpe
1 sibling, 0 replies; 5+ messages in thread
From: Danil Kipnis @ 2020-05-19 16:00 UTC (permalink / raw)
To: Dan Carpenter
Cc: Jack Wang, Doug Ledford, Jason Gunthorpe, linux-rdma, kernel-janitors
On Tue, May 19, 2020 at 5:45 PM Dan Carpenter <dan.carpenter@oracle.com> wrote:
>
> These > comparisons should be >= to prevent accessing one element
> beyond the end of the buffer.
>
> Fixes: 9cb837480424 ("RDMA/rtrs: server: main functionality")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> ---
> drivers/infiniband/ulp/rtrs/rtrs-srv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/ulp/rtrs/rtrs-srv.c b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> index 658c8999cb0d..0b53b79b0e27 100644
> --- a/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> +++ b/drivers/infiniband/ulp/rtrs/rtrs-srv.c
> @@ -1213,8 +1213,8 @@ static void rtrs_srv_rdma_done(struct ib_cq *cq, struct ib_wc *wc)
>
> msg_id = imm_payload >> sess->mem_bits;
> off = imm_payload & ((1 << sess->mem_bits) - 1);
> - if (unlikely(msg_id > srv->queue_depth ||
> - off > max_chunk_size)) {
> + if (unlikely(msg_id >= srv->queue_depth ||
> + off >= max_chunk_size)) {
> rtrs_err(s, "Wrong msg_id %u, off %u\n",
> msg_id, off);
> close_sess(sess);
> --
> 2.26.2
>
Thanks a lot, Dan!
Acked-by: Danil Kipnis <danil.kipnis@cloud.ionos.com>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] RDMA/rtrs: Fix a couple off by one bugs in rtrs_srv_rdma_done()
2020-05-19 15:45 ` [PATCH] RDMA/rtrs: Fix a couple off by one bugs in rtrs_srv_rdma_done() Dan Carpenter
2020-05-19 16:00 ` Danil Kipnis
@ 2020-05-19 23:46 ` Jason Gunthorpe
1 sibling, 0 replies; 5+ messages in thread
From: Jason Gunthorpe @ 2020-05-19 23:46 UTC (permalink / raw)
To: Dan Carpenter
Cc: Danil Kipnis, Jack Wang, Doug Ledford, linux-rdma, kernel-janitors
On Tue, May 19, 2020 at 06:45:25PM +0300, Dan Carpenter wrote:
> These > comparisons should be >= to prevent accessing one element
> beyond the end of the buffer.
>
> Fixes: 9cb837480424 ("RDMA/rtrs: server: main functionality")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
> Acked-by: Danil Kipnis <danil.kipnis@cloud.ionos.com>
> ---
> drivers/infiniband/ulp/rtrs/rtrs-srv.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
Applied to for-next, thanks
Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2020-05-19 23:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-19 12:02 [bug report] RDMA/rtrs: server: main functionality Dan Carpenter
2020-05-19 15:07 ` Danil Kipnis
2020-05-19 15:45 ` [PATCH] RDMA/rtrs: Fix a couple off by one bugs in rtrs_srv_rdma_done() Dan Carpenter
2020-05-19 16:00 ` Danil Kipnis
2020-05-19 23:46 ` Jason Gunthorpe
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).