On Thu, Aug 15, 2019 at 08:15:18PM +0800, Ming Lei wrote:
> It is reported that sysfs buffer overflow can be triggered in case
> of too many CPU cores(>841 on 4K PAGE_SIZE) when showing CPUs in
> one hctx.
> 
> So use snprintf for avoiding the potential buffer overflow.
> 
> Cc: sta...@vger.kernel.org
> Cc: Mark Ray <mark....@hpe.com>
> Fixes: 676141e48af7("blk-mq: don't dump CPU -> hw queue map on driver load")
> Signed-off-by: Ming Lei <ming....@redhat.com>
> ---
>  block/blk-mq-sysfs.c | 30 ++++++++++++++++++------------
>  1 file changed, 18 insertions(+), 12 deletions(-)
> 
> diff --git a/block/blk-mq-sysfs.c b/block/blk-mq-sysfs.c
> index d6e1a9bd7131..e75f41a98415 100644
> --- a/block/blk-mq-sysfs.c
> +++ b/block/blk-mq-sysfs.c
> @@ -164,22 +164,28 @@ static ssize_t 
> blk_mq_hw_sysfs_nr_reserved_tags_show(struct blk_mq_hw_ctx *hctx,
>       return sprintf(page, "%u\n", hctx->tags->nr_reserved_tags);
>  }
>  
> +/* avoid overflow by too many CPU cores */
>  static ssize_t blk_mq_hw_sysfs_cpus_show(struct blk_mq_hw_ctx *hctx, char 
> *page)
>  {
> -     unsigned int i, first = 1;
> -     ssize_t ret = 0;
> -
> -     for_each_cpu(i, hctx->cpumask) {
> -             if (first)
> -                     ret += sprintf(ret + page, "%u", i);
> -             else
> -                     ret += sprintf(ret + page, ", %u", i);
> -
> -             first = 0;
> +     unsigned int cpu = cpumask_first(hctx->cpumask);
> +     ssize_t len = snprintf(page, PAGE_SIZE - 1, "%u", cpu);
> +     int last_len = len;
> +
> +     while ((cpu = cpumask_next(cpu, hctx->cpumask)) < nr_cpu_ids) {
> +             int cur_len = snprintf(page + len, PAGE_SIZE - 1 - len,
> +                                    ", %u", cpu);
> +             if (cur_len >= PAGE_SIZE - 1 - len) {
> +                     len -= last_len;
> +                     len += snprintf(page + len, PAGE_SIZE - 1 - len,
> +                                     "...");
> +                     break;
> +             }
> +             len += cur_len;
> +             last_len = cur_len;
>       }
>  
> -     ret += sprintf(ret + page, "\n");
> -     return ret;
> +     len += snprintf(page + len, PAGE_SIZE - 1 - len, "\n");
> +     return len;
>  }
>

What????

sysfs is "one value per file".  You should NEVER have to care about the
size of the sysfs buffer.  If you do, you are doing something wrong.

What excatly are you trying to show in this sysfs file?  I can't seem to
find the Documenatation/ABI/ entry for it, am I just missing it because
I don't know the filename for it?

thanks,

greg k-h

Reply via email to