On 6/4/2016 8:15 AM, Leon Romanovsky wrote:
> From: Mark Bloch <markb@xxxxxxxxxxxx>
>
> In a attribute group struct, attrs should point to a NULL
> terminated list. Which means we need to allocate one more
> slot in the array.
This patch is both right and wrong. You're right with the intent (that
there should be a total of 2 extra entries in the array size, one for
the NULL termination and one for the lifespan entry), but wrong with the
mechanics (unless I've missed something). We already have two extra
entries because sizeof(*hsag) includes our first counter, so just
num_counters is actually enough to NULL terminate the list, and + 1
gives us lifespan plus a NULL terminate spot. The comment could be
cleaned up to make this more clear though, so I'll do that.
> Fixes: b40f4757daa1 ("IB/core: Make device counter infrastructure dynamic")
> Signed-off-by: Mark Bloch <markb@xxxxxxxxxxxx>
> Signed-off-by: Leon Romanovsky <leon@xxxxxxxxxx>
> ---
> drivers/infiniband/core/sysfs.c | 7 +++++--
> 1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/infiniband/core/sysfs.c b/drivers/infiniband/core/sysfs.c
> index 5e573bb..c68f132 100644
> --- a/drivers/infiniband/core/sysfs.c
> +++ b/drivers/infiniband/core/sysfs.c
> @@ -902,8 +902,11 @@ static void setup_hw_stats(struct ib_device *device, struct ib_port *port,
> goto err;
>
> hsag = kzalloc(sizeof(*hsag) +
> - // 1 extra for the lifespan config entry
> - sizeof(void *) * (stats->num_counters + 1),
> + /* 1 extra for the lifespan config entry,
> + * and 1 more because attrs should be
> + * NULL terminated.
> + */
> + sizeof(void *) * (stats->num_counters + 2),
> GFP_KERNEL);
> if (!hsag)
> return;
>
Attachment:
signature.asc
Description: OpenPGP digital signature
