Hi Dan, On Tue, May 19, 2020 at 2:02 PM Dan Carpenter <dan.carpenter@xxxxxxxxxx> 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