On Mon, Jan 22, 2018 at 02:51:14PM +0200, Leon Romanovsky wrote:
> + mutex_lock(&dev->res.mutex);
> + hash_del_rcu(&res->node);
> + mutex_unlock(&dev->res.mutex);
> +
> + res->valid = false;
> +
> + if (res->task)
> + put_task_struct(res->task);
> + synchronize_srcu(&dev->res.srcu);
> +}
This locking is wrong..
Nothing can invalidate internal elements of 'res' until
synchronize_srcu() returns, otherwise it creates races.
Eg here:
+ key = srcu_read_lock(&device->res.srcu);
+ hash_for_each_possible_rcu(device->res.hash, res, node, RDMA_RESTRACK_QP) {
+ if (idx < start) {
+ idx++;
+ continue;
+ }
+
+ if ((rdma_is_kernel_res(res) &&
+ task_active_pid_ns(current) != &init_pid_ns) ||
+ (!rdma_is_kernel_res(res) &&
+ task_active_pid_ns(current) != task_active_pid_ns(res->task)))
Will use-after-put res->task
Didn't audit closely enough to tell if valid is OK or not. Why isn't
the hash_del sufficient? Why does valid exist?
Jason
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
