2005-05-24 22:21:16

by Roland Dreier

[permalink] [raw]
Subject: [PATCH 0/2] IB: allow NULL sa_query callbacks

Check if a client passes a NULL callback into an SA query, and if so,
never call back. This fixes an oops if someone unloads ib_ipoib and
ib_sa in rapid succession. ib_ipoib does an MCMember delete with a
NULL callback and 0 timeout on unload, which is usually fine since the
delete completes successfully. However, if ib_sa is unloaded
immediately afterwards, the delete will be canceled and ib_sa will try
to call the (now already unloaded) ib_ipoib module back with the
cancel completion, which triggers the oops.

Signed-off-by: Roland Dreier <[email protected]>

---

drivers/infiniband/core/sa_query.c | 35 ++++++++++++++++++-----------------
1 files changed, 18 insertions(+), 17 deletions(-)



--- linux-git.orig/drivers/infiniband/core/sa_query.c 2005-05-24 15:17:29.409716468 -0700
+++ linux-git/drivers/infiniband/core/sa_query.c 2005-05-24 15:17:39.225578334 -0700
@@ -587,7 +587,7 @@

init_mad(query->sa_query.mad, agent);

- query->sa_query.callback = ib_sa_path_rec_callback;
+ query->sa_query.callback = callback ? ib_sa_path_rec_callback : NULL;
query->sa_query.release = ib_sa_path_rec_release;
query->sa_query.port = port;
query->sa_query.mad->mad_hdr.method = IB_MGMT_METHOD_GET;
@@ -663,7 +663,7 @@

init_mad(query->sa_query.mad, agent);

- query->sa_query.callback = ib_sa_mcmember_rec_callback;
+ query->sa_query.callback = callback ? ib_sa_mcmember_rec_callback : NULL;
query->sa_query.release = ib_sa_mcmember_rec_release;
query->sa_query.port = port;
query->sa_query.mad->mad_hdr.method = method;
@@ -698,20 +698,21 @@
if (!query)
return;

- switch (mad_send_wc->status) {
- case IB_WC_SUCCESS:
- /* No callback -- already got recv */
- break;
- case IB_WC_RESP_TIMEOUT_ERR:
- query->callback(query, -ETIMEDOUT, NULL);
- break;
- case IB_WC_WR_FLUSH_ERR:
- query->callback(query, -EINTR, NULL);
- break;
- default:
- query->callback(query, -EIO, NULL);
- break;
- }
+ if (query->callback)
+ switch (mad_send_wc->status) {
+ case IB_WC_SUCCESS:
+ /* No callback -- already got recv */
+ break;
+ case IB_WC_RESP_TIMEOUT_ERR:
+ query->callback(query, -ETIMEDOUT, NULL);
+ break;
+ case IB_WC_WR_FLUSH_ERR:
+ query->callback(query, -EINTR, NULL);
+ break;
+ default:
+ query->callback(query, -EIO, NULL);
+ break;
+ }

dma_unmap_single(agent->device->dma_device,
pci_unmap_addr(query, mapping),
@@ -736,7 +737,7 @@
query = idr_find(&query_idr, mad_recv_wc->wc->wr_id);
spin_unlock_irqrestore(&idr_lock, flags);

- if (query) {
+ if (query && query->callback) {
if (mad_recv_wc->wc->status == IB_WC_SUCCESS)
query->callback(query,
mad_recv_wc->recv_buf.mad->mad_hdr.status ?


2005-05-24 22:21:24

by Roland Dreier

[permalink] [raw]
Subject: [PATCH 1/2] IB: fix potential ib_umad leak

Free all unclaimed MAD receive buffers when userspace closes our file
so we don't leak memory.

Signed-off-by: Roland Dreier <[email protected]>

---

drivers/infiniband/core/user_mad.c | 4 ++++
1 files changed, 4 insertions(+)



--- linux-git.orig/drivers/infiniband/core/user_mad.c 2005-05-24 15:11:25.795151912 -0700
+++ linux-git/drivers/infiniband/core/user_mad.c 2005-05-24 15:17:39.800453121 -0700
@@ -499,6 +499,7 @@
static int ib_umad_close(struct inode *inode, struct file *filp)
{
struct ib_umad_file *file = filp->private_data;
+ struct ib_umad_packet *packet, *tmp;
int i;

for (i = 0; i < IB_UMAD_MAX_AGENTS; ++i)
@@ -507,6 +508,9 @@
ib_unregister_mad_agent(file->agent[i]);
}

+ list_for_each_entry_safe(packet, tmp, &file->recv_list, list)
+ kfree(packet);
+
kfree(file);

return 0;

2005-05-24 22:22:44

by Roland Dreier

[permalink] [raw]
Subject: [PATCH 2/2] IB: fix endianness of path record MTU field

Make MTU field in SA PathRecord and MCMemberRecord a u8 rather than an
enum to avoid complications with endianness.

Signed-off-by: Roland Dreier <[email protected]>

---

drivers/infiniband/include/ib_sa.h | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)



--- linux-git.orig/drivers/infiniband/include/ib_sa.h 2005-05-24 15:15:55.076280598 -0700
+++ linux-git/drivers/infiniband/include/ib_sa.h 2005-05-24 15:17:41.485086202 -0700
@@ -147,7 +147,7 @@
/* reserved */
u8 sl;
u8 mtu_selector;
- enum ib_mtu mtu;
+ u8 mtu;
u8 rate_selector;
u8 rate;
u8 packet_life_time_selector;
@@ -180,7 +180,7 @@
u32 qkey;
u16 mlid;
u8 mtu_selector;
- enum ib_mtu mtu;
+ u8 mtu;
u8 traffic_class;
u16 pkey;
u8 rate_selector;

2005-05-24 22:26:44

by Roland Dreier

[permalink] [raw]
Subject: Re: [openib-general] [PATCH 0/2] IB: allow NULL sa_query callbacks

err, sorry -- slight screwup in the subject lines due to a bug in my
patch scripts. These should obviously be 1-based rather than 0-based.
Anyway, the patches themselves should be fine.

Thanks,
Roland