linux-kernel - Re: [PATCH v1] platform/x86: wmi: Replace kmalloc + sprintf() with kasprintf()

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] [day] [month] [year] [list]

Message-ID: <20180216195410.GF5786@fury>
Date:   Fri, 16 Feb 2018 11:54:10 -0800
From:   Darren Hart <dvhart@...radead.org>
To:     David Laight <David.Laight@...LAB.COM>
Cc:     'Andy Shevchenko' <andriy.shevchenko@...ux.intel.com>,
        "platform-driver-x86@...r.kernel.org" 
        <platform-driver-x86@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1] platform/x86: wmi: Replace kmalloc + sprintf() with
 kasprintf()

On Fri, Feb 16, 2018 at 03:55:24PM +0000, David Laight wrote:
> From: Andy Shevchenko
> > Sent: 16 February 2018 15:40
> >
> > kasprintf() does the job of two: kmalloc() and sprintf().
> > Replace two calls with one.
> ...
> > -		buf = kmalloc(strlen(wdriver->driver.name) + 5, GFP_KERNEL);
> > +		buf = kasprintf(GFP_KERNEL, "wmi/%s", wdriver->driver.name);
> ...
> 
> Except that kasprintf() has no idea how long a buffer is needed.
> It might even do the printf twice just to get the length.

Sure, but this is one-time and non-critical path. Unfortunately there
isn't any guidance in Documentation here. Of the 520 or so instances of
kasprintf usages in the kernel, device name or similar is a very common
pattern. Eliminating manual counting of characters for buffer allocation
seems like a good plan to me.

So unless we want to argue that all those use cases are wrong, this
would appear to be at least common practice, if not best practice for
this particular pattern.

Reviewed-by: Darren Hart (VMware) <dvhart@...radead.org>

-- 
Darren Hart
VMware Open Source Technology Center