Skip to content

Commit

Permalink
RDMA/siw: Fix 64/32bit pointer inconsistency
Browse files Browse the repository at this point in the history
Fixes improper casting between addresses and unsigned types.
Changes siw_pbl_get_buffer() function to return appropriate
dma_addr_t, and not u64.

Also fixes debug prints. Now any potentially kernel private
pointers are printed formatted as '%pK', to allow keeping that
information secret.

Fixes: d941bfe500be ("RDMA/siw: Change CQ flags from 64->32 bits")
Fixes: b0fff73 ("rdma/siw: completion queue methods")
Fixes: 8b6a361 ("rdma/siw: receive path")
Fixes: b9be6f1 ("rdma/siw: transmit path")
Fixes: f29dd55 ("rdma/siw: queue pair methods")
Fixes: 2251334 ("rdma/siw: application buffer management")
Fixes: 303ae1c ("rdma/siw: application interface")
Fixes: 6c52fdc ("rdma/siw: connection management")
Fixes: a531975 ("rdma/siw: main include file")

Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Reported-by: Jason Gunthorpe <jgg@ziepe.ca>
Reported-by: Leon Romanovsky <leon@kernel.org>
Signed-off-by: Bernard Metzler <bmt@zurich.ibm.com>
Link: https://lore.kernel.org/r/20190822173738.26817-1-bmt@zurich.ibm.com
Signed-off-by: Doug Ledford <dledford@redhat.com>
  • Loading branch information
Bernard Metzler authored and Doug Ledford committed Aug 23, 2019
1 parent fab4f97 commit c536277
Show file tree
Hide file tree
Showing 9 changed files with 108 additions and 109 deletions.
8 changes: 4 additions & 4 deletions drivers/infiniband/sw/siw/siw.h
Original file line number Diff line number Diff line change
Expand Up @@ -138,9 +138,9 @@ struct siw_umem {
};

struct siw_pble {
u64 addr; /* Address of assigned user buffer */
u64 size; /* Size of this entry */
u64 pbl_off; /* Total offset from start of PBL */
dma_addr_t addr; /* Address of assigned buffer */
unsigned int size; /* Size of this entry */
unsigned long pbl_off; /* Total offset from start of PBL */
};

struct siw_pbl {
Expand Down Expand Up @@ -734,7 +734,7 @@ static inline void siw_crc_skb(struct siw_rx_stream *srx, unsigned int len)
"MEM[0x%08x] %s: " fmt, mem->stag, __func__, ##__VA_ARGS__)

#define siw_dbg_cep(cep, fmt, ...) \
ibdev_dbg(&cep->sdev->base_dev, "CEP[0x%p] %s: " fmt, \
ibdev_dbg(&cep->sdev->base_dev, "CEP[0x%pK] %s: " fmt, \
cep, __func__, ##__VA_ARGS__)

void siw_cq_flush(struct siw_cq *cq);
Expand Down
77 changes: 36 additions & 41 deletions drivers/infiniband/sw/siw/siw_cm.c
Original file line number Diff line number Diff line change
Expand Up @@ -355,8 +355,8 @@ static int siw_cm_upcall(struct siw_cep *cep, enum iw_cm_event_type reason,
getname_local(cep->sock, &event.local_addr);
getname_peer(cep->sock, &event.remote_addr);
}
siw_dbg_cep(cep, "[QP %u]: id 0x%p, reason=%d, status=%d\n",
cep->qp ? qp_id(cep->qp) : -1, id, reason, status);
siw_dbg_cep(cep, "[QP %u]: reason=%d, status=%d\n",
cep->qp ? qp_id(cep->qp) : UINT_MAX, reason, status);

return id->event_handler(id, &event);
}
Expand Down Expand Up @@ -947,8 +947,6 @@ static void siw_accept_newconn(struct siw_cep *cep)
siw_cep_get(new_cep);
new_s->sk->sk_user_data = new_cep;

siw_dbg_cep(cep, "listen socket 0x%p, new 0x%p\n", s, new_s);

if (siw_tcp_nagle == false) {
int val = 1;

Expand Down Expand Up @@ -1011,7 +1009,8 @@ static void siw_cm_work_handler(struct work_struct *w)
cep = work->cep;

siw_dbg_cep(cep, "[QP %u]: work type: %d, state %d\n",
cep->qp ? qp_id(cep->qp) : -1, work->type, cep->state);
cep->qp ? qp_id(cep->qp) : UINT_MAX,
work->type, cep->state);

siw_cep_set_inuse(cep);

Expand Down Expand Up @@ -1145,9 +1144,9 @@ static void siw_cm_work_handler(struct work_struct *w)
}
if (release_cep) {
siw_dbg_cep(cep,
"release: timer=%s, QP[%u], id 0x%p\n",
"release: timer=%s, QP[%u]\n",
cep->mpa_timer ? "y" : "n",
cep->qp ? qp_id(cep->qp) : -1, cep->cm_id);
cep->qp ? qp_id(cep->qp) : UINT_MAX);

siw_cancel_mpatimer(cep);

Expand Down Expand Up @@ -1211,8 +1210,8 @@ int siw_cm_queue_work(struct siw_cep *cep, enum siw_work_type type)
else
delay = MPAREP_TIMEOUT;
}
siw_dbg_cep(cep, "[QP %u]: work type: %d, work 0x%p, timeout %lu\n",
cep->qp ? qp_id(cep->qp) : -1, type, work, delay);
siw_dbg_cep(cep, "[QP %u]: work type: %d, timeout %lu\n",
cep->qp ? qp_id(cep->qp) : -1, type, delay);

queue_delayed_work(siw_cm_wq, &work->work, delay);

Expand Down Expand Up @@ -1376,16 +1375,16 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
}
if (v4)
siw_dbg_qp(qp,
"id 0x%p, pd_len %d, laddr %pI4 %d, raddr %pI4 %d\n",
id, pd_len,
"pd_len %d, laddr %pI4 %d, raddr %pI4 %d\n",
pd_len,
&((struct sockaddr_in *)(laddr))->sin_addr,
ntohs(((struct sockaddr_in *)(laddr))->sin_port),
&((struct sockaddr_in *)(raddr))->sin_addr,
ntohs(((struct sockaddr_in *)(raddr))->sin_port));
else
siw_dbg_qp(qp,
"id 0x%p, pd_len %d, laddr %pI6 %d, raddr %pI6 %d\n",
id, pd_len,
"pd_len %d, laddr %pI6 %d, raddr %pI6 %d\n",
pd_len,
&((struct sockaddr_in6 *)(laddr))->sin6_addr,
ntohs(((struct sockaddr_in6 *)(laddr))->sin6_port),
&((struct sockaddr_in6 *)(raddr))->sin6_addr,
Expand Down Expand Up @@ -1508,8 +1507,7 @@ int siw_connect(struct iw_cm_id *id, struct iw_cm_conn_param *params)
if (rv >= 0) {
rv = siw_cm_queue_work(cep, SIW_CM_WORK_MPATIMEOUT);
if (!rv) {
siw_dbg_cep(cep, "id 0x%p, [QP %u]: exit\n", id,
qp_id(qp));
siw_dbg_cep(cep, "[QP %u]: exit\n", qp_id(qp));
siw_cep_set_free(cep);
return 0;
}
Expand Down Expand Up @@ -1581,7 +1579,7 @@ int siw_accept(struct iw_cm_id *id, struct iw_cm_conn_param *params)
siw_cancel_mpatimer(cep);

if (cep->state != SIW_EPSTATE_RECVD_MPAREQ) {
siw_dbg_cep(cep, "id 0x%p: out of state\n", id);
siw_dbg_cep(cep, "out of state\n");

siw_cep_set_free(cep);
siw_cep_put(cep);
Expand All @@ -1602,7 +1600,7 @@ int siw_accept(struct iw_cm_id *id, struct iw_cm_conn_param *params)
up_write(&qp->state_lock);
goto error;
}
siw_dbg_cep(cep, "id 0x%p\n", id);
siw_dbg_cep(cep, "[QP %d]\n", params->qpn);

if (try_gso && cep->mpa.hdr.params.bits & MPA_RR_FLAG_GSO_EXP) {
siw_dbg_cep(cep, "peer allows GSO on TX\n");
Expand All @@ -1612,8 +1610,8 @@ int siw_accept(struct iw_cm_id *id, struct iw_cm_conn_param *params)
params->ird > sdev->attrs.max_ird) {
siw_dbg_cep(
cep,
"id 0x%p, [QP %u]: ord %d (max %d), ird %d (max %d)\n",
id, qp_id(qp), params->ord, sdev->attrs.max_ord,
"[QP %u]: ord %d (max %d), ird %d (max %d)\n",
qp_id(qp), params->ord, sdev->attrs.max_ord,
params->ird, sdev->attrs.max_ird);
rv = -EINVAL;
up_write(&qp->state_lock);
Expand All @@ -1625,8 +1623,8 @@ int siw_accept(struct iw_cm_id *id, struct iw_cm_conn_param *params)
if (params->private_data_len > max_priv_data) {
siw_dbg_cep(
cep,
"id 0x%p, [QP %u]: private data length: %d (max %d)\n",
id, qp_id(qp), params->private_data_len, max_priv_data);
"[QP %u]: private data length: %d (max %d)\n",
qp_id(qp), params->private_data_len, max_priv_data);
rv = -EINVAL;
up_write(&qp->state_lock);
goto error;
Expand Down Expand Up @@ -1680,7 +1678,7 @@ int siw_accept(struct iw_cm_id *id, struct iw_cm_conn_param *params)
qp_attrs.flags = SIW_MPA_CRC;
qp_attrs.state = SIW_QP_STATE_RTS;

siw_dbg_cep(cep, "id 0x%p, [QP%u]: moving to rts\n", id, qp_id(qp));
siw_dbg_cep(cep, "[QP%u]: moving to rts\n", qp_id(qp));

/* Associate QP with CEP */
siw_cep_get(cep);
Expand All @@ -1701,8 +1699,8 @@ int siw_accept(struct iw_cm_id *id, struct iw_cm_conn_param *params)
if (rv)
goto error;

siw_dbg_cep(cep, "id 0x%p, [QP %u]: send mpa reply, %d byte pdata\n",
id, qp_id(qp), params->private_data_len);
siw_dbg_cep(cep, "[QP %u]: send mpa reply, %d byte pdata\n",
qp_id(qp), params->private_data_len);

rv = siw_send_mpareqrep(cep, params->private_data,
params->private_data_len);
Expand Down Expand Up @@ -1760,14 +1758,14 @@ int siw_reject(struct iw_cm_id *id, const void *pdata, u8 pd_len)
siw_cancel_mpatimer(cep);

if (cep->state != SIW_EPSTATE_RECVD_MPAREQ) {
siw_dbg_cep(cep, "id 0x%p: out of state\n", id);
siw_dbg_cep(cep, "out of state\n");

siw_cep_set_free(cep);
siw_cep_put(cep); /* put last reference */

return -ECONNRESET;
}
siw_dbg_cep(cep, "id 0x%p, cep->state %d, pd_len %d\n", id, cep->state,
siw_dbg_cep(cep, "cep->state %d, pd_len %d\n", cep->state,
pd_len);

if (__mpa_rr_revision(cep->mpa.hdr.params.bits) >= MPA_REVISION_1) {
Expand Down Expand Up @@ -1805,14 +1803,14 @@ static int siw_listen_address(struct iw_cm_id *id, int backlog,
rv = kernel_setsockopt(s, SOL_SOCKET, SO_REUSEADDR, (char *)&s_val,
sizeof(s_val));
if (rv) {
siw_dbg(id->device, "id 0x%p: setsockopt error: %d\n", id, rv);
siw_dbg(id->device, "setsockopt error: %d\n", rv);
goto error;
}
rv = s->ops->bind(s, laddr, addr_family == AF_INET ?
sizeof(struct sockaddr_in) :
sizeof(struct sockaddr_in6));
if (rv) {
siw_dbg(id->device, "id 0x%p: socket bind error: %d\n", id, rv);
siw_dbg(id->device, "socket bind error: %d\n", rv);
goto error;
}
cep = siw_cep_alloc(sdev);
Expand All @@ -1825,13 +1823,13 @@ static int siw_listen_address(struct iw_cm_id *id, int backlog,
rv = siw_cm_alloc_work(cep, backlog);
if (rv) {
siw_dbg(id->device,
"id 0x%p: alloc_work error %d, backlog %d\n", id,
"alloc_work error %d, backlog %d\n",
rv, backlog);
goto error;
}
rv = s->ops->listen(s, backlog);
if (rv) {
siw_dbg(id->device, "id 0x%p: listen error %d\n", id, rv);
siw_dbg(id->device, "listen error %d\n", rv);
goto error;
}
cep->cm_id = id;
Expand Down Expand Up @@ -1915,8 +1913,7 @@ static void siw_drop_listeners(struct iw_cm_id *id)

list_del(p);

siw_dbg_cep(cep, "id 0x%p: drop cep, state %d\n", id,
cep->state);
siw_dbg_cep(cep, "drop cep, state %d\n", cep->state);

siw_cep_set_inuse(cep);

Expand Down Expand Up @@ -1953,7 +1950,7 @@ int siw_create_listen(struct iw_cm_id *id, int backlog)
struct net_device *dev = to_siw_dev(id->device)->netdev;
int rv = 0, listeners = 0;

siw_dbg(id->device, "id 0x%p: backlog %d\n", id, backlog);
siw_dbg(id->device, "backlog %d\n", backlog);

/*
* For each attached address of the interface, create a
Expand All @@ -1969,8 +1966,8 @@ int siw_create_listen(struct iw_cm_id *id, int backlog)
s_raddr = (struct sockaddr_in *)&id->remote_addr;

siw_dbg(id->device,
"id 0x%p: laddr %pI4:%d, raddr %pI4:%d\n",
id, &s_laddr.sin_addr, ntohs(s_laddr.sin_port),
"laddr %pI4:%d, raddr %pI4:%d\n",
&s_laddr.sin_addr, ntohs(s_laddr.sin_port),
&s_raddr->sin_addr, ntohs(s_raddr->sin_port));

rtnl_lock();
Expand All @@ -1995,8 +1992,8 @@ int siw_create_listen(struct iw_cm_id *id, int backlog)
*s_raddr = &to_sockaddr_in6(id->remote_addr);

siw_dbg(id->device,
"id 0x%p: laddr %pI6:%d, raddr %pI6:%d\n",
id, &s_laddr->sin6_addr, ntohs(s_laddr->sin6_port),
"laddr %pI6:%d, raddr %pI6:%d\n",
&s_laddr->sin6_addr, ntohs(s_laddr->sin6_port),
&s_raddr->sin6_addr, ntohs(s_raddr->sin6_port));

read_lock_bh(&in6_dev->lock);
Expand Down Expand Up @@ -2029,17 +2026,15 @@ int siw_create_listen(struct iw_cm_id *id, int backlog)
else if (!rv)
rv = -EINVAL;

siw_dbg(id->device, "id 0x%p: %s\n", id, rv ? "FAIL" : "OK");
siw_dbg(id->device, "%s\n", rv ? "FAIL" : "OK");

return rv;
}

int siw_destroy_listen(struct iw_cm_id *id)
{
siw_dbg(id->device, "id 0x%p\n", id);

if (!id->provider_data) {
siw_dbg(id->device, "id 0x%p: no cep(s)\n", id);
siw_dbg(id->device, "no cep(s)\n");
return 0;
}
siw_drop_listeners(id);
Expand Down
5 changes: 3 additions & 2 deletions drivers/infiniband/sw/siw/siw_cq.c
Original file line number Diff line number Diff line change
Expand Up @@ -71,9 +71,10 @@ int siw_reap_cqe(struct siw_cq *cq, struct ib_wc *wc)
wc->wc_flags = IB_WC_WITH_INVALIDATE;
}
wc->qp = cqe->base_qp;
siw_dbg_cq(cq, "idx %u, type %d, flags %2x, id 0x%p\n",
siw_dbg_cq(cq,
"idx %u, type %d, flags %2x, id 0x%pK\n",
cq->cq_get % cq->num_cqe, cqe->opcode,
cqe->flags, (void *)cqe->id);
cqe->flags, (void *)(uintptr_t)cqe->id);
}
WRITE_ONCE(cqe->flags, 0);
cq->cq_get++;
Expand Down
14 changes: 7 additions & 7 deletions drivers/infiniband/sw/siw/siw_mem.c
Original file line number Diff line number Diff line change
Expand Up @@ -197,12 +197,12 @@ int siw_check_mem(struct ib_pd *pd, struct siw_mem *mem, u64 addr,
*/
if (addr < mem->va || addr + len > mem->va + mem->len) {
siw_dbg_pd(pd, "MEM interval len %d\n", len);
siw_dbg_pd(pd, "[0x%016llx, 0x%016llx] out of bounds\n",
(unsigned long long)addr,
(unsigned long long)(addr + len));
siw_dbg_pd(pd, "[0x%016llx, 0x%016llx] STag=0x%08x\n",
(unsigned long long)mem->va,
(unsigned long long)(mem->va + mem->len),
siw_dbg_pd(pd, "[0x%pK, 0x%pK] out of bounds\n",
(void *)(uintptr_t)addr,
(void *)(uintptr_t)(addr + len));
siw_dbg_pd(pd, "[0x%pK, 0x%pK] STag=0x%08x\n",
(void *)(uintptr_t)mem->va,
(void *)(uintptr_t)(mem->va + mem->len),
mem->stag);

return -E_BASE_BOUNDS;
Expand Down Expand Up @@ -330,7 +330,7 @@ int siw_invalidate_stag(struct ib_pd *pd, u32 stag)
* Optionally, provides remaining len within current element, and
* current PBL index for later resume at same element.
*/
u64 siw_pbl_get_buffer(struct siw_pbl *pbl, u64 off, int *len, int *idx)
dma_addr_t siw_pbl_get_buffer(struct siw_pbl *pbl, u64 off, int *len, int *idx)
{
int i = idx ? *idx : 0;

Expand Down
2 changes: 1 addition & 1 deletion drivers/infiniband/sw/siw/siw_mem.h
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
struct siw_umem *siw_umem_get(u64 start, u64 len, bool writable);
void siw_umem_release(struct siw_umem *umem, bool dirty);
struct siw_pbl *siw_pbl_alloc(u32 num_buf);
u64 siw_pbl_get_buffer(struct siw_pbl *pbl, u64 off, int *len, int *idx);
dma_addr_t siw_pbl_get_buffer(struct siw_pbl *pbl, u64 off, int *len, int *idx);
struct siw_mem *siw_mem_id2obj(struct siw_device *sdev, int stag_index);
int siw_mem_add(struct siw_device *sdev, struct siw_mem *m);
int siw_invalidate_stag(struct ib_pd *pd, u32 stag);
Expand Down
2 changes: 1 addition & 1 deletion drivers/infiniband/sw/siw/siw_qp.c
Original file line number Diff line number Diff line change
Expand Up @@ -949,7 +949,7 @@ int siw_activate_tx(struct siw_qp *qp)
rv = -EINVAL;
goto out;
}
wqe->sqe.sge[0].laddr = (u64)&wqe->sqe.sge[1];
wqe->sqe.sge[0].laddr = (uintptr_t)&wqe->sqe.sge[1];
wqe->sqe.sge[0].lkey = 0;
wqe->sqe.num_sge = 1;
}
Expand Down
Loading

0 comments on commit c536277

Please sign in to comment.