On Tue, Mar 20, 2018 at 07:28:19AM +0000, Mark Bloch wrote:
> Currently access to hardware stats buffer isn't protected, this can
> result in multiple writes and reads at the same time to the same
> memory location. This can lead to providing an incorrect value to
> the user. Add a mutex to protect against it.
>
> Fixes: b40f4757daa1 ("IB/core: Make device counter infrastructure dynamic")
> Signed-off-by: Mark Bloch <markb@xxxxxxxxxxxx>
> We might want to protect setting/reading lifespan as well, but it seems
> not needed as it shouldn't be changed too often, and even if it does not by
> multiple threads at the same time.
>
> v0 -> v1: Removed RFC tag and added a Fixes line.
> drivers/infiniband/core/sysfs.c | 10 ++++++++--
> include/rdma/ib_verbs.h | 1 +
> 2 files changed, 9 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c
> index 8ae1308e..b769d61 100644
> +++ b/drivers/infiniband/core/sysfs.c
> @@ -810,10 +810,15 @@ static ssize_t show_hw_stats(struct kobject *kobj, struct attribute *attr,
> dev = port->ibdev;
> stats = port->hw_stats;
> }
> + mutex_lock(&stats->lock);
> ret = update_hw_stats(dev, stats, hsa->port_num, hsa->index);
> if (ret)
> - return ret;
> - return print_hw_stat(stats, hsa->index, buf);
> + goto unlock;
> + ret = print_hw_stat(stats, hsa->index, buf);
> +unlock:
> + mutex_unlock(&stats->lock);
> +
> + return ret;
> }
I think we are going to add a lock, it should cover all readers
too. There is no real reason not too that I can see.
So switch stats->lock to a rwlock and hold it in
show_hw_stats (write)
show_stats_lifespan
set_stats_lifespan
Which is all the places that touch stats..
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
