linux-kernel - Re: [PATCH net-next v3 1/3] hexdump: Implement macro for converting large buffers

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: <20250220220050.61aa504d@pumpkin>
Date: Thu, 20 Feb 2025 22:00:50 +0000
From: David Laight <david.laight.linux@...il.com>
To: Nick Child <nnac123@...ux.ibm.com>
Cc: netdev@...r.kernel.org, linux-kernel@...r.kernel.org, horms@...nel.org,
 nick.child@....com, pmladek@...e.com, rostedt@...dmis.org,
 john.ogness@...utronix.de, senozhatsky@...omium.org
Subject: Re: [PATCH net-next v3 1/3] hexdump: Implement macro for converting
 large buffers

On Wed, 19 Feb 2025 15:11:00 -0600
Nick Child <nnac123@...ux.ibm.com> wrote:

> Define for_each_line_in_hex_dump which loops over a buffer and calls
> hex_dump_to_buffer for each segment in the buffer. This allows the
> caller to decide what to do with the resulting string and is not
> limited by a specific printing format like print_hex_dump.
> 
> Signed-off-by: Nick Child <nnac123@...ux.ibm.com>
> ---
>  include/linux/printk.h | 20 ++++++++++++++++++++
>  1 file changed, 20 insertions(+)
> 
> diff --git a/include/linux/printk.h b/include/linux/printk.h
> index 4217a9f412b2..12e51b1cdca5 100644
> --- a/include/linux/printk.h
> +++ b/include/linux/printk.h
> @@ -755,6 +755,26 @@ enum {
>  extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
>  			      int groupsize, char *linebuf, size_t linebuflen,
>  			      bool ascii);
> +/**
> + * for_each_line_in_hex_dump - iterate over buffer, converting into hex ASCII
> + * @i: offset in @buff
> + * @rowsize: number of bytes to print per line; must be 16 or 32
> + * @linebuf: where to put the converted data
> + * @linebuflen: total size of @linebuf, including space for terminating NUL
> + *		IOW >= (@rowsize * 2) + ((@rowsize - 1 / @groupsize)) + 1
> + * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1)
> + * @buf: data blob to dump
> + * @len: number of bytes in the @buf
> + */
> + #define for_each_line_in_hex_dump(i, rowsize, linebuf, linebuflen, groupsize, \
> +				   buf, len) \
> +	for ((i) = 0;							\
> +	     (i) < (len) &&						\
> +	     hex_dump_to_buffer((unsigned char *)(buf) + (i),		\
> +				(len) - (i), (rowsize), (groupsize),	\
> +				(linebuf), (linebuflen), false);	\

You can avoid the compiler actually checking the function result
it you try a bit harder - see below.

> +	     (i) += (rowsize) == 32 ? 32 : 16				\
> +	    )

If you are doing this as a #define you really shouldn't evaluate the
arguments more than once.
I'd also not add more code that relies on the perverse and pointless
code that enforces rowsize of 16 or 32.
Maybe document it, but there is no point changing the stride without
doing the equivalent change to the rowsize passed to hex_dump_to_buffer.

You could do:
#define for_each_line_in_hex_dump(buf_offset, rowsize, linebuf, linebuflen, groupsize, buf, len, ascii) \
for (unsigned int _offset = 0, _rowsize = (rowsize), _len = (len); \
	((offset) = _offset) < _len && (hex_dump_to_buffer((const char *)(buf) + _offset, _len - _offset, \
		_rowsize, (groupsize), (linebuf), (linebuflen), (ascii)), 1); \
	_offset += _rowsize )

(Assuming I've not mistyped it.)

As soon as 'ascii' gets replaced by 'flags' you'll need to pass it through.

	David