linux-kernel - Re: [PATCH v7 1/4] cpumask: introduce cpumap_print_to

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <YPBUJmqk4akuX8cv@yury-ThinkPad>
Date:   Thu, 15 Jul 2021 08:28:38 -0700
From:   Yury Norov <yury.norov@...il.com>
To:     Barry Song <song.bao.hua@...ilicon.com>
Cc:     gregkh@...uxfoundation.org, akpm@...ux-foundation.org,
        andriy.shevchenko@...ux.intel.com, linux-kernel@...r.kernel.org,
        dave.hansen@...el.com, linux@...musvillemoes.dk, rafael@...nel.org,
        rdunlap@...radead.org, agordeev@...ux.ibm.com, sbrivio@...hat.com,
        jianpeng.ma@...el.com, valentin.schneider@....com,
        peterz@...radead.org, bristot@...hat.com, guodong.xu@...aro.org,
        tangchengchang@...wei.com, prime.zeng@...ilicon.com,
        yangyicong@...wei.com, tim.c.chen@...ux.intel.com,
        linuxarm@...wei.com, Tian Tao <tiantao6@...ilicon.com>
Subject: Re: [PATCH v7 1/4] cpumask: introduce cpumap_print_to_buf to support
 large bitmask and list

On Thu, Jul 15, 2021 at 11:58:53PM +1200, Barry Song wrote:
> (10.1.198.147)
> X-CFilter-Loop: Reflected
> Status: O
> Content-Length: 10263
> Lines: 252
> 
> From: Tian Tao <tiantao6@...ilicon.com>

[...]

> +int bitmap_print_to_buf(bool list, char *buf, const unsigned long *maskp,
> +		int nmaskbits, loff_t off, size_t count)
> +{
> +	const char *fmt = list ? "%*pbl\n" : "%*pb\n";
> +	ssize_t size;
> +	void *data;
> +
> +	data = kasprintf(GFP_KERNEL, fmt, nmaskbits, maskp);
> +	if (!data)
> +		return -ENOMEM;
> +
> +	size = memory_read_from_buffer(buf, count, &off, data, strlen(data) + 1);
> +	kfree(data);
> +
> +	return size;
> +}
> +EXPORT_SYMBOL(bitmap_print_to_buf);

In discussion to v4 of this series I've pointed out inefficiency of
this approach. Now it's v7, but the problem is still there.

1. You make user of your API guess aboout proper @count without any
   hint. This is worse than how it works now with pure vsnprintf().
2. For big bitmaps and small @count, your code will make enormous
   amount of unneeded work. For example, if the full length of string
   representation of bitmap is 1M, and length of the output buffer is
   1k, one will have to call bitmap_print_to_buf() 1000 times.  With
   current design it assumes that you allocate the full amount of memory
   1000 times, free it 1000 times and print huge bitmap 1000 times to
   just return small part of it.

NAK for this, and please stop submitting wrong approach again and
again.