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]
Date:	Tue, 29 Sep 2015 19:30:24 +0100
From:	Mark Brown <broonie@...nel.org>
To:	Rasmus Villemoes <linux@...musvillemoes.dk>
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/2] regmap: debugfs: improve
 regmap_reg_ranges_read_file()

On Tue, Sep 29, 2015 at 12:29:02AM +0200, Rasmus Villemoes wrote:

This patch is an example of why SubmittingPatches recommends splitting
things up into one change per patch, it would be much easier to read and
review as a series (especially given that there's very few collisions).

> * A page is a bit much for two integers and a bit of punctuation. 64
>   bytes should suffice.

Right, the reason PAGE_SIZE was chosen is that it's a natural unit for
the allocator and is clearly absurdly large for the data.  For 64 bytes
I have to think for a moment if it's suitably large.  Please leave it at
PAGE_SIZE, it's not like this hangs around for any length of time.

> * Calling strlen() on entry no less than three times is silly,
>   especially when snprintf() has returned that value (which was just
>   thrown away).

I think we were expecting the compiler would figure out that strlen() is
a pure function and do the right thing here (though I do see it's
missing an annotation).

> * Transferring entry to the output buffer using snprintf is silly,
>   when we know the length. Use memcpy instead.

Right, that's a legacy of a previous version transferring the maximum
amount of data possible (which got abandoned due to complexity).
However with the changes to use the return value of snprinf() it seems
like the best thing to do here is to go back to this and just fill up as
much of the buffer as we can by using snprintf() to do the transfer.

That said I think memcpy() is going to be the best way of getting to
that since one of the issues there (which currently doesn't work) is
slicing things off the front and memcpy() handles that nicely.

Download attachment "signature.asc" of type "application/pgp-signature" (474 bytes)

Powered by blists - more mailing lists