| Message ID | 1303759728-15256-2-git-send-email-brad.figg@canonical.com |
|---|---|
| State | New |
| Headers | show |
On 04/25/2011 01:28 PM, Brad Figg wrote: > From: Sean Hefty<sean.hefty@intel.com> > > CVE-2011-0695 > > BugLink: http://bugs.launchpad.net/bugs/770369 > > Doug Ledford and Red Hat reported a crash when running the rdma_cm on > a real-time OS. The crash has the following call trace: > > cm_process_work > cma_req_handler > cma_disable_callback > rdma_create_id > kzalloc > init_completion > cma_get_net_info > cma_save_net_info > cma_any_addr > cma_zero_addr > rdma_translate_ip > rdma_copy_addr > cma_acquire_dev > rdma_addr_get_sgid > ib_find_cached_gid > cma_attach_to_dev > ucma_event_handler > kzalloc > ib_copy_ah_attr_to_user > cma_comp > > [ preempted ] > > cma_write > copy_from_user > ucma_destroy_id > copy_from_user > _ucma_find_context > ucma_put_ctx > ucma_free_ctx > rdma_destroy_id > cma_exch > cma_cancel_operation > rdma_node_get_transport > > rt_mutex_slowunlock > bad_area_nosemaphore > oops_enter > > They were able to reproduce the crash multiple times with the > following details: > > Crash seems to always happen on the: > mutex_unlock(&conn_id->handler_mutex); > as conn_id looks to have been freed during this code path. > > An examination of the code shows that a race exists in the request > handlers. When a new connection request is received, the rdma_cm > allocates a new connection identifier. This identifier has a single > reference count on it. If a user calls rdma_destroy_id() from another > thread after receiving a callback, rdma_destroy_id will proceed to > destroy the id and free the associated memory. However, the request > handlers may still be in the process of running. When control returns > to the request handlers, they can attempt to access the newly created > identifiers. > > Fix this by holding a reference on the newly created rdma_cm_id until > the request handler is through accessing it. > > Signed-off-by: Sean Hefty<sean.hefty@intel.com> > Acked-by: Doug Ledford<dledford@redhat.com> > Cc:<stable@kernel.org> > Signed-off-by: Roland Dreier<roland@purestorage.com> > > (backported from commit 25ae21a10112875763c18b385624df713a288a05) > Signed-off-by: Brad Figg<brad.figg@canonical.com> > --- > drivers/infiniband/core/cma.c | 19 ++++++++++++++++++- > 1 files changed, 18 insertions(+), 1 deletions(-) > > diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c > index 0751697..d4636ab 100644 > --- a/drivers/infiniband/core/cma.c > +++ b/drivers/infiniband/core/cma.c > @@ -1121,9 +1121,17 @@ static int cma_req_handler(struct ib_cm_id *cm_id, struct ib_cm_event *ib_event) > cm_id->context = conn_id; > cm_id->cm_handler = cma_ib_handler; > > + /* > + * Protect against the user destroying conn_id from another thread > + * until we're done accessing it. > + */ > + atomic_inc(&conn_id->refcount); > ret = conn_id->id.event_handler(&conn_id->id,&event); > - if (!ret) > + if (!ret) { > + cma_deref_id(conn_id); > goto out; > + } > + cma_deref_id(conn_id); > > /* Destroy the CM ID by returning a non-zero value. */ > conn_id->cm_id.ib = NULL; > @@ -1315,14 +1323,23 @@ static int iw_conn_req_handler(struct iw_cm_id *cm_id, > event.event = RDMA_CM_EVENT_CONNECT_REQUEST; > event.param.conn.private_data = iw_event->private_data; > event.param.conn.private_data_len = iw_event->private_data_len; > + > + /* > + * Protect against the user destroying conn_id from another thread > + * until we're done accessing it. > + */ > + atomic_inc(&conn_id->refcount); > ret = conn_id->id.event_handler(&conn_id->id,&event); > if (ret) { > /* User wants to destroy the CM ID */ > conn_id->cm_id.iw = NULL; > cma_exch(conn_id, CMA_DESTROYING); > cma_enable_remove(conn_id); > + cma_deref_id(conn_id); > rdma_destroy_id(&conn_id->id); > + goto out; > } > + cma_deref_id(conn_id); > > out: > if (dev) Acked-by: Tim Gardner <tim.gardner@canonical.com>
On Mon, 2011-04-25 at 12:28 -0700, Brad Figg wrote: > From: Sean Hefty <sean.hefty@intel.com> > > CVE-2011-0695 > > BugLink: http://bugs.launchpad.net/bugs/770369 > > Doug Ledford and Red Hat reported a crash when running the rdma_cm on > a real-time OS. The crash has the following call trace: > > cm_process_work > cma_req_handler > cma_disable_callback > rdma_create_id > kzalloc > init_completion > cma_get_net_info > cma_save_net_info > cma_any_addr > cma_zero_addr > rdma_translate_ip > rdma_copy_addr > cma_acquire_dev > rdma_addr_get_sgid > ib_find_cached_gid > cma_attach_to_dev > ucma_event_handler > kzalloc > ib_copy_ah_attr_to_user > cma_comp > > [ preempted ] > > cma_write > copy_from_user > ucma_destroy_id > copy_from_user > _ucma_find_context > ucma_put_ctx > ucma_free_ctx > rdma_destroy_id > cma_exch > cma_cancel_operation > rdma_node_get_transport > > rt_mutex_slowunlock > bad_area_nosemaphore > oops_enter > > They were able to reproduce the crash multiple times with the > following details: > > Crash seems to always happen on the: > mutex_unlock(&conn_id->handler_mutex); > as conn_id looks to have been freed during this code path. > > An examination of the code shows that a race exists in the request > handlers. When a new connection request is received, the rdma_cm > allocates a new connection identifier. This identifier has a single > reference count on it. If a user calls rdma_destroy_id() from another > thread after receiving a callback, rdma_destroy_id will proceed to > destroy the id and free the associated memory. However, the request > handlers may still be in the process of running. When control returns > to the request handlers, they can attempt to access the newly created > identifiers. > > Fix this by holding a reference on the newly created rdma_cm_id until > the request handler is through accessing it. > > Signed-off-by: Sean Hefty <sean.hefty@intel.com> > Acked-by: Doug Ledford <dledford@redhat.com> > Cc: <stable@kernel.org> > Signed-off-by: Roland Dreier <roland@purestorage.com> > > (backported from commit 25ae21a10112875763c18b385624df713a288a05) > Signed-off-by: Brad Figg <brad.figg@canonical.com> Acked-by: Leann Ogasawara <leann.ogasawara@canonical.com> > --- > drivers/infiniband/core/cma.c | 19 ++++++++++++++++++- > 1 files changed, 18 insertions(+), 1 deletions(-) > > diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c > index 0751697..d4636ab 100644 > --- a/drivers/infiniband/core/cma.c > +++ b/drivers/infiniband/core/cma.c > @@ -1121,9 +1121,17 @@ static int cma_req_handler(struct ib_cm_id *cm_id, struct ib_cm_event *ib_event) > cm_id->context = conn_id; > cm_id->cm_handler = cma_ib_handler; > > + /* > + * Protect against the user destroying conn_id from another thread > + * until we're done accessing it. > + */ > + atomic_inc(&conn_id->refcount); > ret = conn_id->id.event_handler(&conn_id->id, &event); > - if (!ret) > + if (!ret) { > + cma_deref_id(conn_id); > goto out; > + } > + cma_deref_id(conn_id); > > /* Destroy the CM ID by returning a non-zero value. */ > conn_id->cm_id.ib = NULL; > @@ -1315,14 +1323,23 @@ static int iw_conn_req_handler(struct iw_cm_id *cm_id, > event.event = RDMA_CM_EVENT_CONNECT_REQUEST; > event.param.conn.private_data = iw_event->private_data; > event.param.conn.private_data_len = iw_event->private_data_len; > + > + /* > + * Protect against the user destroying conn_id from another thread > + * until we're done accessing it. > + */ > + atomic_inc(&conn_id->refcount); > ret = conn_id->id.event_handler(&conn_id->id, &event); > if (ret) { > /* User wants to destroy the CM ID */ > conn_id->cm_id.iw = NULL; > cma_exch(conn_id, CMA_DESTROYING); > cma_enable_remove(conn_id); > + cma_deref_id(conn_id); > rdma_destroy_id(&conn_id->id); > + goto out; > } > + cma_deref_id(conn_id); > > out: > if (dev) > -- > 1.7.0.4 > >
On 04/25/2011 01:28 PM, Brad Figg wrote: > From: Sean Hefty<sean.hefty@intel.com> > > CVE-2011-0695 > > BugLink: http://bugs.launchpad.net/bugs/770369 > > Doug Ledford and Red Hat reported a crash when running the rdma_cm on > a real-time OS. The crash has the following call trace: > > cm_process_work > cma_req_handler > cma_disable_callback > rdma_create_id > kzalloc > init_completion > cma_get_net_info > cma_save_net_info > cma_any_addr > cma_zero_addr > rdma_translate_ip > rdma_copy_addr > cma_acquire_dev > rdma_addr_get_sgid > ib_find_cached_gid > cma_attach_to_dev > ucma_event_handler > kzalloc > ib_copy_ah_attr_to_user > cma_comp > > [ preempted ] > > cma_write > copy_from_user > ucma_destroy_id > copy_from_user > _ucma_find_context > ucma_put_ctx > ucma_free_ctx > rdma_destroy_id > cma_exch > cma_cancel_operation > rdma_node_get_transport > > rt_mutex_slowunlock > bad_area_nosemaphore > oops_enter > > They were able to reproduce the crash multiple times with the > following details: > > Crash seems to always happen on the: > mutex_unlock(&conn_id->handler_mutex); > as conn_id looks to have been freed during this code path. > > An examination of the code shows that a race exists in the request > handlers. When a new connection request is received, the rdma_cm > allocates a new connection identifier. This identifier has a single > reference count on it. If a user calls rdma_destroy_id() from another > thread after receiving a callback, rdma_destroy_id will proceed to > destroy the id and free the associated memory. However, the request > handlers may still be in the process of running. When control returns > to the request handlers, they can attempt to access the newly created > identifiers. > > Fix this by holding a reference on the newly created rdma_cm_id until > the request handler is through accessing it. > > Signed-off-by: Sean Hefty<sean.hefty@intel.com> > Acked-by: Doug Ledford<dledford@redhat.com> > Cc:<stable@kernel.org> > Signed-off-by: Roland Dreier<roland@purestorage.com> > > (backported from commit 25ae21a10112875763c18b385624df713a288a05) > Signed-off-by: Brad Figg<brad.figg@canonical.com> > --- > drivers/infiniband/core/cma.c | 19 ++++++++++++++++++- > 1 files changed, 18 insertions(+), 1 deletions(-) > > diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c > index 0751697..d4636ab 100644 > --- a/drivers/infiniband/core/cma.c > +++ b/drivers/infiniband/core/cma.c > @@ -1121,9 +1121,17 @@ static int cma_req_handler(struct ib_cm_id *cm_id, struct ib_cm_event *ib_event) > cm_id->context = conn_id; > cm_id->cm_handler = cma_ib_handler; > > + /* > + * Protect against the user destroying conn_id from another thread > + * until we're done accessing it. > + */ > + atomic_inc(&conn_id->refcount); > ret = conn_id->id.event_handler(&conn_id->id,&event); > - if (!ret) > + if (!ret) { > + cma_deref_id(conn_id); > goto out; > + } > + cma_deref_id(conn_id); > > /* Destroy the CM ID by returning a non-zero value. */ > conn_id->cm_id.ib = NULL; > @@ -1315,14 +1323,23 @@ static int iw_conn_req_handler(struct iw_cm_id *cm_id, > event.event = RDMA_CM_EVENT_CONNECT_REQUEST; > event.param.conn.private_data = iw_event->private_data; > event.param.conn.private_data_len = iw_event->private_data_len; > + > + /* > + * Protect against the user destroying conn_id from another thread > + * until we're done accessing it. > + */ > + atomic_inc(&conn_id->refcount); > ret = conn_id->id.event_handler(&conn_id->id,&event); > if (ret) { > /* User wants to destroy the CM ID */ > conn_id->cm_id.iw = NULL; > cma_exch(conn_id, CMA_DESTROYING); > cma_enable_remove(conn_id); > + cma_deref_id(conn_id); > rdma_destroy_id(&conn_id->id); > + goto out; > } > + cma_deref_id(conn_id); > > out: > if (dev) applied
diff --git a/drivers/infiniband/core/cma.c b/drivers/infiniband/core/cma.c index 0751697..d4636ab 100644 --- a/drivers/infiniband/core/cma.c +++ b/drivers/infiniband/core/cma.c @@ -1121,9 +1121,17 @@ static int cma_req_handler(struct ib_cm_id *cm_id, struct ib_cm_event *ib_event) cm_id->context = conn_id; cm_id->cm_handler = cma_ib_handler; + /* + * Protect against the user destroying conn_id from another thread + * until we're done accessing it. + */ + atomic_inc(&conn_id->refcount); ret = conn_id->id.event_handler(&conn_id->id, &event); - if (!ret) + if (!ret) { + cma_deref_id(conn_id); goto out; + } + cma_deref_id(conn_id); /* Destroy the CM ID by returning a non-zero value. */ conn_id->cm_id.ib = NULL; @@ -1315,14 +1323,23 @@ static int iw_conn_req_handler(struct iw_cm_id *cm_id, event.event = RDMA_CM_EVENT_CONNECT_REQUEST; event.param.conn.private_data = iw_event->private_data; event.param.conn.private_data_len = iw_event->private_data_len; + + /* + * Protect against the user destroying conn_id from another thread + * until we're done accessing it. + */ + atomic_inc(&conn_id->refcount); ret = conn_id->id.event_handler(&conn_id->id, &event); if (ret) { /* User wants to destroy the CM ID */ conn_id->cm_id.iw = NULL; cma_exch(conn_id, CMA_DESTROYING); cma_enable_remove(conn_id); + cma_deref_id(conn_id); rdma_destroy_id(&conn_id->id); + goto out; } + cma_deref_id(conn_id); out: if (dev)