2019-05-08 07:03:29

by Alastair D'Silva

[permalink] [raw]
Subject: [PATCH v2 0/7] Hexdump Enhancements

From: Alastair D'Silva <[email protected]>

Apologies for the large CC list, it's a heads up for those responsible
for subsystems where a prototype change in generic code causes a change
in those subsystems.

This series enhances hexdump.

These improve the readability of the dumped data in certain situations
(eg. wide terminals are available, many lines of empty bytes exist, etc).

The default behaviour of hexdump is unchanged, however, the prototype
for hex_dump_to_buffer() has changed, and print_hex_dump() has been
renamed to print_hex_dump_ext(), with a wrapper replacing it for
compatibility with existing code, which would have been too invasive to
change.

Hexdump selftests have be run & confirmed passed.

Changelog:
- Fix failing selftests
- Fix precedence bug in 'Replace ascii bool in hex_dump_to_buffer...'
- Remove hardcoded new lengths & instead relax the checks in
hex_dump_to_buffer, allocating the buffer from the heap instead of the
stack.
- Replace the skipping of lines of 0x00/0xff with skipping lines of
repeated characters, announcing what has been skipped.
- Add spaces as an optional N-group separator
- Allow byte ordering to be maintained when HEXDUMP_RETAIN_BYTE_ORDERING
is set.
- Updated selftests to cover 'Relax rowsize checks' &
'Optionally retain byte ordering'

Alastair D'Silva (7):
lib/hexdump.c: Fix selftests
lib/hexdump.c: Relax rowsize checks in hex_dump_to_buffer
lib/hexdump.c: Optionally suppress lines of repeated bytes
lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
lib/hexdump.c: Allow multiple groups to be separated by lines '|'
lib/hexdump.c: Allow multiple groups to be separated by spaces
lib/hexdump.c: Optionally retain byte ordering

drivers/gpu/drm/i915/intel_engine_cs.c | 2 +-
drivers/isdn/hardware/mISDN/mISDNisar.c | 6 +-
drivers/mailbox/mailbox-test.c | 2 +-
drivers/net/ethernet/amd/xgbe/xgbe-drv.c | 2 +-
.../net/ethernet/synopsys/dwc-xlgmac-common.c | 2 +-
drivers/net/wireless/ath/ath10k/debug.c | 3 +-
.../net/wireless/intel/iwlegacy/3945-mac.c | 2 +-
drivers/platform/chrome/wilco_ec/debugfs.c | 2 +-
drivers/scsi/scsi_logging.c | 8 +-
drivers/staging/fbtft/fbtft-core.c | 2 +-
fs/seq_file.c | 3 +-
include/linux/printk.h | 34 ++-
lib/hexdump.c | 260 +++++++++++++++---
lib/test_hexdump.c | 146 +++++++---
14 files changed, 372 insertions(+), 102 deletions(-)

--
2.21.0


2019-05-08 07:03:29

by Alastair D'Silva

[permalink] [raw]
Subject: [PATCH v2 3/7] lib/hexdump.c: Optionally suppress lines of repeated bytes

From: Alastair D'Silva <[email protected]>

Some buffers may only be partially filled with useful data, while the rest
is padded (typically with 0x00 or 0xff).

This patch introduces a flag to allow the supression of lines of repeated
bytes, which are replaced with '** Skipped %u bytes of value 0x%x **'

An inline wrapper function is provided for backwards compatibility with
existing code, which maintains the original behaviour.

Signed-off-by: Alastair D'Silva <[email protected]>
---
include/linux/printk.h | 25 +++++++++---
lib/hexdump.c | 91 ++++++++++++++++++++++++++++++++++++------
2 files changed, 99 insertions(+), 17 deletions(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index d7c77ed1a4cb..938a67580d78 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -479,13 +479,18 @@ enum {
DUMP_PREFIX_ADDRESS,
DUMP_PREFIX_OFFSET
};
+
extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
int groupsize, char *linebuf, size_t linebuflen,
bool ascii);
+
+#define HEXDUMP_ASCII (1 << 0)
+#define HEXDUMP_SUPPRESS_REPEATED (1 << 1)
+
#ifdef CONFIG_PRINTK
-extern void print_hex_dump(const char *level, const char *prefix_str,
+extern void print_hex_dump_ext(const char *level, const char *prefix_str,
int prefix_type, int rowsize, int groupsize,
- const void *buf, size_t len, bool ascii);
+ const void *buf, size_t len, u64 flags);
#if defined(CONFIG_DYNAMIC_DEBUG)
#define print_hex_dump_bytes(prefix_str, prefix_type, buf, len) \
dynamic_hex_dump(prefix_str, prefix_type, 16, 1, buf, len, true)
@@ -494,18 +499,28 @@ extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
const void *buf, size_t len);
#endif /* defined(CONFIG_DYNAMIC_DEBUG) */
#else
-static inline void print_hex_dump(const char *level, const char *prefix_str,
+static inline void print_hex_dump_ext(const char *level, const char *prefix_str,
int prefix_type, int rowsize, int groupsize,
- const void *buf, size_t len, bool ascii)
+ const void *buf, size_t len, u64 flags)
{
}
static inline void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
const void *buf, size_t len)
{
}
-
#endif

+static __always_inline void print_hex_dump(const char *level,
+ const char *prefix_str,
+ int prefix_type, int rowsize,
+ int groupsize, const void *buf,
+ size_t len, bool ascii)
+{
+ print_hex_dump_ext(level, prefix_str, prefix_type, rowsize, groupsize,
+ buf, len, ascii ? HEXDUMP_ASCII : 0);
+}
+
+
#if defined(CONFIG_DYNAMIC_DEBUG)
#define print_hex_dump_debug(prefix_str, prefix_type, rowsize, \
groupsize, buf, len, ascii) \
diff --git a/lib/hexdump.c b/lib/hexdump.c
index 3943507bc0e9..d61a1e4f19fa 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -212,8 +212,44 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
EXPORT_SYMBOL(hex_dump_to_buffer);

#ifdef CONFIG_PRINTK
+
+/**
+ * Check if a buffer contains only a single byte value
+ * @buf: pointer to the buffer
+ * @len: the size of the buffer in bytes
+ * @val: outputs the value if if the bytes are identical
+ */
+static bool buf_is_all(const u8 *buf, size_t len, u8 *val_out)
+{
+ size_t i;
+ u8 val;
+
+ if (len <= 1)
+ return false;
+
+ val = buf[0];
+
+ for (i = 1; i < len; i++) {
+ if (buf[i] != val)
+ return false;
+ }
+
+ *val_out = val;
+ return true;
+}
+
+static void announce_skipped(const char *level, const char *prefix_str,
+ u8 val, size_t count)
+{
+ if (count == 0)
+ return;
+
+ printk("%s%s ** Skipped %lu bytes of value 0x%x **\n",
+ level, prefix_str, count, val);
+}
+
/**
- * print_hex_dump - print a text hex dump to syslog for a binary blob of data
+ * print_hex_dump_ext: dump a binary blob of data to syslog in hexadecimal
* @level: kernel log level (e.g. KERN_DEBUG)
* @prefix_str: string to prefix each line with;
* caller supplies trailing spaces for alignment if desired
@@ -224,6 +260,10 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
* @buf: data blob to dump
* @len: number of bytes in the @buf
* @ascii: include ASCII after the hex output
+ * @flags: A bitwise OR of the following flags:
+ * HEXDUMP_ASCII: include ASCII after the hex output
+ * HEXDUMP_SUPPRESS_REPEATED: suppress repeated lines of identical
+ * bytes
*
* Given a buffer of u8 data, print_hex_dump() prints a hex + ASCII dump
* to the kernel log at the specified kernel log level, with an optional
@@ -234,22 +274,25 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
* (optionally) ASCII output.
*
* E.g.:
- * print_hex_dump(KERN_DEBUG, "raw data: ", DUMP_PREFIX_ADDRESS,
- * 16, 1, frame->data, frame->len, true);
+ * print_hex_dump_ext(KERN_DEBUG, "raw data: ", DUMP_PREFIX_ADDRESS,
+ * 16, 1, frame->data, frame->len, HEXDUMP_ASCII);
*
* Example output using %DUMP_PREFIX_OFFSET and 1-byte mode:
* 0009ab42: 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f @ABCDEFGHIJKLMNO
* Example output using %DUMP_PREFIX_ADDRESS and 4-byte mode:
* ffffffff88089af0: 73727170 77767574 7b7a7978 7f7e7d7c pqrstuvwxyz{|}~.
*/
-void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
- int rowsize, int groupsize,
- const void *buf, size_t len, bool ascii)
+void print_hex_dump_ext(const char *level, const char *prefix_str,
+ int prefix_type, int rowsize, int groupsize,
+ const void *buf, size_t len, u64 flags)
{
const u8 *ptr = buf;
- int i, linelen, remaining = len;
+ int i, remaining = len;
unsigned char *linebuf;
unsigned int linebuf_len;
+ u8 skipped_val = 0;
+ size_t skipped = 0;
+

if (rowsize % groupsize)
rowsize -= rowsize % groupsize;
@@ -266,11 +309,35 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
}

for (i = 0; i < len; i += rowsize) {
- linelen = min(remaining, rowsize);
+ int linelen = min(remaining, rowsize);
remaining -= rowsize;

+ if (flags & HEXDUMP_SUPPRESS_REPEATED) {
+ u8 new_val;
+
+ if (buf_is_all(ptr + i, linelen, &new_val)) {
+ if (new_val == skipped_val) {
+ skipped += linelen;
+ continue;
+ } else {
+ announce_skipped(level, prefix_str,
+ skipped_val, skipped);
+ skipped_val = new_val;
+ skipped = linelen;
+ continue;
+ }
+ }
+ }
+
+ if (skipped) {
+ announce_skipped(level, prefix_str, skipped_val,
+ skipped);
+ skipped = 0;
+ }
+
hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
- linebuf, linebuf_len, ascii);
+ linebuf, linebuf_len,
+ flags & HEXDUMP_ASCII);

switch (prefix_type) {
case DUMP_PREFIX_ADDRESS:
@@ -288,7 +355,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,

kfree(linebuf);
}
-EXPORT_SYMBOL(print_hex_dump);
+EXPORT_SYMBOL(print_hex_dump_ext);

#if !defined(CONFIG_DYNAMIC_DEBUG)
/**
@@ -306,8 +373,8 @@ EXPORT_SYMBOL(print_hex_dump);
void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
const void *buf, size_t len)
{
- print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, 16, 1,
- buf, len, true);
+ print_hex_dump_ext(KERN_DEBUG, prefix_str, prefix_type, 16, 1,
+ buf, len, HEXDUMP_ASCII);
}
EXPORT_SYMBOL(print_hex_dump_bytes);
#endif /* !defined(CONFIG_DYNAMIC_DEBUG) */
--
2.21.0

2019-05-08 07:03:29

by Alastair D'Silva

[permalink] [raw]
Subject: [PATCH v2 6/7] lib/hexdump.c: Allow multiple groups to be separated by spaces

From: Alastair D'Silva <[email protected]>

Similar to the previous patch, this patch separates groups by 2 spaces for
the hex fields, and 1 space for the ASCII field.

eg.
buf:00000000: 454d414e 43415053 4e495f45 00584544 NAMESPAC E_INDEX.
buf:00000010: 00000000 00000002 00000000 00000000 ........ ........

Signed-off-by: Alastair D'Silva <[email protected]>
---
include/linux/printk.h | 3 ++
lib/hexdump.c | 65 +++++++++++++++++++++++++++++++-----------
2 files changed, 52 insertions(+), 16 deletions(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index dc693aec394c..5231a14e4593 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -485,6 +485,9 @@ enum {
#define HEXDUMP_2_GRP_LINES (1 << 2)
#define HEXDUMP_4_GRP_LINES (1 << 3)
#define HEXDUMP_8_GRP_LINES (1 << 4)
+#define HEXDUMP_2_GRP_SPACES (1 << 5)
+#define HEXDUMP_4_GRP_SPACES (1 << 6)
+#define HEXDUMP_8_GRP_SPACES (1 << 7)

extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
int groupsize, char *linebuf, size_t linebuflen,
diff --git a/lib/hexdump.c b/lib/hexdump.c
index 6f4d1176c332..febd614406d1 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -91,9 +91,37 @@ static const char *group_separator(int group, u64 flags)
if ((flags & HEXDUMP_2_GRP_LINES) && !((group) % 2))
return "|";

+ if ((flags & HEXDUMP_8_GRP_SPACES) && !((group) % 8))
+ return " ";
+
+ if ((flags & HEXDUMP_4_GRP_SPACES) && !((group) % 4))
+ return " ";
+
+ if ((flags & HEXDUMP_2_GRP_SPACES) && !((group) % 2))
+ return " ";
+
return " ";
}

+static void separator_parameters(u64 flags, int groupsize, int *sep_chars,
+ char *sep)
+{
+ if (flags & (HEXDUMP_2_GRP_LINES | HEXDUMP_2_GRP_SPACES))
+ *sep_chars = groupsize * 2;
+ if (flags & (HEXDUMP_4_GRP_LINES | HEXDUMP_4_GRP_SPACES))
+ *sep_chars = groupsize * 4;
+ if (flags & (HEXDUMP_8_GRP_LINES | HEXDUMP_8_GRP_SPACES))
+ *sep_chars = groupsize * 8;
+
+ if (flags & (HEXDUMP_2_GRP_LINES | HEXDUMP_4_GRP_LINES |
+ HEXDUMP_8_GRP_LINES))
+ *sep = '|';
+
+ if (flags & (HEXDUMP_2_GRP_SPACES | HEXDUMP_4_GRP_SPACES |
+ HEXDUMP_8_GRP_SPACES))
+ *sep = ' ';
+}
+
/**
* hex_dump_to_buffer - convert a blob of data to "hex ASCII" in memory
* @buf: data blob to dump
@@ -107,6 +135,9 @@ static const char *group_separator(int group, u64 flags)
* HEXDUMP_2_GRP_LINES: insert a '|' after every 2 groups
* HEXDUMP_4_GRP_LINES: insert a '|' after every 4 groups
* HEXDUMP_8_GRP_LINES: insert a '|' after every 8 groups
+ * HEXDUMP_2_GRP_SPACES: insert a ' ' after every 2 groups
+ * HEXDUMP_4_GRP_SPACES: insert a ' ' after every 4 groups
+ * HEXDUMP_8_GRP_SPACES: insert a ' ' after every 8 groups
*
* hex_dump_to_buffer() works on one "line" of output at a time, converting
* <groupsize> bytes of input to hexadecimal (and optionally printable ASCII)
@@ -138,7 +169,8 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
int j, lx = 0;
int ascii_column;
int ret;
- int line_chars = 0;
+ int sep_chars = 0;
+ char sep = 0;

if (!is_power_of_2(groupsize) || groupsize > 8)
groupsize = 1;
@@ -152,8 +184,14 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
len = rowsize;

ngroups = len / groupsize;
+
ascii_column = rowsize * 2 + rowsize / groupsize + 1;

+ // space separators use 2 spaces in the hex output
+ separator_parameters(flags, groupsize, &sep_chars, &sep);
+ if (sep == ' ')
+ ascii_column += rowsize / sep_chars;
+
if (!linebuflen)
goto overflow1;

@@ -221,24 +259,17 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
linebuf[lx++] = ' ';
}

- if (flags & HEXDUMP_2_GRP_LINES)
- line_chars = groupsize * 2;
- if (flags & HEXDUMP_4_GRP_LINES)
- line_chars = groupsize * 4;
- if (flags & HEXDUMP_8_GRP_LINES)
- line_chars = groupsize * 8;
-
for (j = 0; j < len; j++) {
if (linebuflen < lx + 2)
goto overflow2;
ch = ptr[j];
linebuf[lx++] = (isascii(ch) && isprint(ch)) ? ch : '.';

- if (line_chars && ((j + 1) < len) &&
- ((j + 1) % line_chars == 0)) {
+ if (sep_chars && ((j + 1) < len) &&
+ ((j + 1) % sep_chars == 0)) {
if (linebuflen < lx + 2)
goto overflow2;
- linebuf[lx++] = '|';
+ linebuf[lx++] = sep;
}
}
nil:
@@ -247,9 +278,11 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
overflow2:
linebuf[lx++] = '\0';
overflow1:
- return (flags & HEXDUMP_ASCII) ? ascii_column + len +
- (len - 1) / line_chars :
- (groupsize * 2 + 1) * ngroups - 1;
+ if (flags & HEXDUMP_ASCII)
+ return ascii_column + len + (len - 1) / sep_chars;
+
+ return groupsize * 2 * ngroups +
+ ((sep == ' ') ? 2 : 1) * (ngroups - 1);
}
EXPORT_SYMBOL(hex_dump_to_buffer);

@@ -343,9 +376,9 @@ void print_hex_dump_ext(const char *level, const char *prefix_str,

/* Worst case line length:
* 2 hex chars + space per byte in, 2 spaces, 1 char per byte in,
- * 1 char per N groups, NULL
+ * 2 char per N groups, NULL
*/
- linebuf_len = rowsize * 3 + 2 + rowsize + rowsize / groupsize + 1;
+ linebuf_len = rowsize * 3 + 2 + rowsize + 2 * rowsize / groupsize + 1;
linebuf = kzalloc(linebuf_len, GFP_KERNEL);
if (!linebuf) {
printk("%s%shexdump: Could not alloc %u bytes for buffer\n",
--
2.21.0

2019-05-08 07:03:30

by Alastair D'Silva

[permalink] [raw]
Subject: [PATCH v2 5/7] lib/hexdump.c: Allow multiple groups to be separated by lines '|'

From: Alastair D'Silva <[email protected]>

With the wider display format, it can become hard to identify how many
bytes into the line you are looking at.

The patch adds new flags to hex_dump_to_buffer() and print_hex_dump() to
print vertical lines to separate every N groups of bytes.

eg.
buf:00000000: 454d414e 43415053|4e495f45 00584544 NAMESPAC|E_INDEX.
buf:00000010: 00000000 00000002|00000000 00000000 ........|........

Signed-off-by: Alastair D'Silva <[email protected]>
---
include/linux/printk.h | 3 +++
lib/hexdump.c | 59 ++++++++++++++++++++++++++++++++++++------
2 files changed, 54 insertions(+), 8 deletions(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index 00a82e468643..dc693aec394c 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -482,6 +482,9 @@ enum {

#define HEXDUMP_ASCII (1 << 0)
#define HEXDUMP_SUPPRESS_REPEATED (1 << 1)
+#define HEXDUMP_2_GRP_LINES (1 << 2)
+#define HEXDUMP_4_GRP_LINES (1 << 3)
+#define HEXDUMP_8_GRP_LINES (1 << 4)

extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
int groupsize, char *linebuf, size_t linebuflen,
diff --git a/lib/hexdump.c b/lib/hexdump.c
index ddd1697e5f9b..6f4d1176c332 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -77,6 +77,23 @@ char *bin2hex(char *dst, const void *src, size_t count)
}
EXPORT_SYMBOL(bin2hex);

+static const char *group_separator(int group, u64 flags)
+{
+ if (group == 0)
+ return " ";
+
+ if ((flags & HEXDUMP_8_GRP_LINES) && !((group) % 8))
+ return "|";
+
+ if ((flags & HEXDUMP_4_GRP_LINES) && !((group) % 4))
+ return "|";
+
+ if ((flags & HEXDUMP_2_GRP_LINES) && !((group) % 2))
+ return "|";
+
+ return " ";
+}
+
/**
* hex_dump_to_buffer - convert a blob of data to "hex ASCII" in memory
* @buf: data blob to dump
@@ -87,6 +104,9 @@ EXPORT_SYMBOL(bin2hex);
* @linebuflen: total size of @linebuf, including space for terminating NUL
* @flags: A bitwise OR of the following flags:
* HEXDUMP_ASCII: include ASCII after the hex output
+ * HEXDUMP_2_GRP_LINES: insert a '|' after every 2 groups
+ * HEXDUMP_4_GRP_LINES: insert a '|' after every 4 groups
+ * HEXDUMP_8_GRP_LINES: insert a '|' after every 8 groups
*
* hex_dump_to_buffer() works on one "line" of output at a time, converting
* <groupsize> bytes of input to hexadecimal (and optionally printable ASCII)
@@ -118,6 +138,7 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
int j, lx = 0;
int ascii_column;
int ret;
+ int line_chars = 0;

if (!is_power_of_2(groupsize) || groupsize > 8)
groupsize = 1;
@@ -144,7 +165,8 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,

for (j = 0; j < ngroups; j++) {
ret = snprintf(linebuf + lx, linebuflen - lx,
- "%s%16.16llx", j ? " " : "",
+ "%s%16.16llx",
+ j ? group_separator(j, flags) : "",
get_unaligned(ptr8 + j));
if (ret >= linebuflen - lx)
goto overflow1;
@@ -155,7 +177,8 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,

for (j = 0; j < ngroups; j++) {
ret = snprintf(linebuf + lx, linebuflen - lx,
- "%s%8.8x", j ? " " : "",
+ "%s%8.8x",
+ j ? group_separator(j, flags) : "",
get_unaligned(ptr4 + j));
if (ret >= linebuflen - lx)
goto overflow1;
@@ -166,7 +189,8 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,

for (j = 0; j < ngroups; j++) {
ret = snprintf(linebuf + lx, linebuflen - lx,
- "%s%4.4x", j ? " " : "",
+ "%s%4.4x",
+ j ? group_separator(j, flags) : "",
get_unaligned(ptr2 + j));
if (ret >= linebuflen - lx)
goto overflow1;
@@ -196,11 +220,26 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
goto overflow2;
linebuf[lx++] = ' ';
}
+
+ if (flags & HEXDUMP_2_GRP_LINES)
+ line_chars = groupsize * 2;
+ if (flags & HEXDUMP_4_GRP_LINES)
+ line_chars = groupsize * 4;
+ if (flags & HEXDUMP_8_GRP_LINES)
+ line_chars = groupsize * 8;
+
for (j = 0; j < len; j++) {
if (linebuflen < lx + 2)
goto overflow2;
ch = ptr[j];
linebuf[lx++] = (isascii(ch) && isprint(ch)) ? ch : '.';
+
+ if (line_chars && ((j + 1) < len) &&
+ ((j + 1) % line_chars == 0)) {
+ if (linebuflen < lx + 2)
+ goto overflow2;
+ linebuf[lx++] = '|';
+ }
}
nil:
linebuf[lx] = '\0';
@@ -208,7 +247,8 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
overflow2:
linebuf[lx++] = '\0';
overflow1:
- return (flags & HEXDUMP_ASCII) ? ascii_column + len :
+ return (flags & HEXDUMP_ASCII) ? ascii_column + len +
+ (len - 1) / line_chars :
(groupsize * 2 + 1) * ngroups - 1;
}
EXPORT_SYMBOL(hex_dump_to_buffer);
@@ -246,7 +286,7 @@ static void announce_skipped(const char *level, const char *prefix_str,
if (count == 0)
return;

- printk("%s%s ** Skipped %lu bytes of value 0x%x **\n",
+ printk("%s%s ** Skipped %lu bytes of value 0x%02x **\n",
level, prefix_str, count, val);
}

@@ -266,6 +306,9 @@ static void announce_skipped(const char *level, const char *prefix_str,
* HEXDUMP_ASCII: include ASCII after the hex output
* HEXDUMP_SUPPRESS_REPEATED: suppress repeated lines of identical
* bytes
+ * HEXDUMP_2_GRP_LINES: insert a '|' after every 2 groups
+ * HEXDUMP_4_GRP_LINES: insert a '|' after every 4 groups
+ * HEXDUMP_8_GRP_LINES: insert a '|' after every 8 groups
*
* Given a buffer of u8 data, print_hex_dump() prints a hex + ASCII dump
* to the kernel log at the specified kernel log level, with an optional
@@ -295,14 +338,14 @@ void print_hex_dump_ext(const char *level, const char *prefix_str,
u8 skipped_val = 0;
size_t skipped = 0;

-
if (rowsize % groupsize)
rowsize -= rowsize % groupsize;

/* Worst case line length:
- * 2 hex chars + space per byte in, 2 spaces, 1 char per byte in, NULL
+ * 2 hex chars + space per byte in, 2 spaces, 1 char per byte in,
+ * 1 char per N groups, NULL
*/
- linebuf_len = rowsize * 3 + 2 + rowsize + 1;
+ linebuf_len = rowsize * 3 + 2 + rowsize + rowsize / groupsize + 1;
linebuf = kzalloc(linebuf_len, GFP_KERNEL);
if (!linebuf) {
printk("%s%shexdump: Could not alloc %u bytes for buffer\n",
--
2.21.0

2019-05-08 07:03:45

by Alastair D'Silva

[permalink] [raw]
Subject: [PATCH v2 2/7] lib/hexdump.c: Relax rowsize checks in hex_dump_to_buffer

From: Alastair D'Silva <[email protected]>

This patch removes the hardcoded row limits and allows for
other lengths. These lengths must still be a multiple of
groupsize.

This allows structs that are not 16/32 bytes to display on
a single line.

This patch also expands the self-tests to test row sizes
up to 64 bytes (though they can now be arbitrarily long).

Signed-off-by: Alastair D'Silva <[email protected]>
---
lib/hexdump.c | 48 ++++++++++++++++++++++++++++--------------
lib/test_hexdump.c | 52 ++++++++++++++++++++++++++++++++++++++--------
2 files changed, 75 insertions(+), 25 deletions(-)

diff --git a/lib/hexdump.c b/lib/hexdump.c
index 81b70ed37209..3943507bc0e9 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -12,6 +12,7 @@
#include <linux/errno.h>
#include <linux/kernel.h>
#include <linux/export.h>
+#include <linux/slab.h>
#include <asm/unaligned.h>

const char hex_asc[] = "0123456789abcdef";
@@ -80,14 +81,15 @@ EXPORT_SYMBOL(bin2hex);
* hex_dump_to_buffer - convert a blob of data to "hex ASCII" in memory
* @buf: data blob to dump
* @len: number of bytes in the @buf
- * @rowsize: number of bytes to print per line; must be 16 or 32
+ * @rowsize: number of bytes to print per line; must be a multiple of groupsize
* @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1)
* @linebuf: where to put the converted data
* @linebuflen: total size of @linebuf, including space for terminating NUL
* @ascii: include ASCII after the hex output
*
- * hex_dump_to_buffer() works on one "line" of output at a time, i.e.,
- * 16 or 32 bytes of input data converted to hex + ASCII output.
+ * hex_dump_to_buffer() works on one "line" of output at a time, converting
+ * <groupsize> bytes of input to hexadecimal (and optionally printable ASCII)
+ * until <rowsize> bytes have been emitted.
*
* Given a buffer of u8 data, hex_dump_to_buffer() converts the input data
* to a hex + ASCII dump at the supplied memory location.
@@ -116,16 +118,17 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
int ascii_column;
int ret;

- if (rowsize != 16 && rowsize != 32)
- rowsize = 16;
-
- if (len > rowsize) /* limit to one line at a time */
- len = rowsize;
if (!is_power_of_2(groupsize) || groupsize > 8)
groupsize = 1;
if ((len % groupsize) != 0) /* no mixed size output */
groupsize = 1;

+ if (rowsize % groupsize)
+ rowsize -= rowsize % groupsize;
+
+ if (len > rowsize) /* limit to one line at a time */
+ len = rowsize;
+
ngroups = len / groupsize;
ascii_column = rowsize * 2 + rowsize / groupsize + 1;

@@ -216,7 +219,7 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
* caller supplies trailing spaces for alignment if desired
* @prefix_type: controls whether prefix of an offset, address, or none
* is printed (%DUMP_PREFIX_OFFSET, %DUMP_PREFIX_ADDRESS, %DUMP_PREFIX_NONE)
- * @rowsize: number of bytes to print per line; must be 16 or 32
+ * @rowsize: number of bytes to print per line; must be a multiple of groupsize
* @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
@@ -226,10 +229,9 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
* to the kernel log at the specified kernel log level, with an optional
* leading prefix.
*
- * print_hex_dump() works on one "line" of output at a time, i.e.,
- * 16 or 32 bytes of input data converted to hex + ASCII output.
* print_hex_dump() iterates over the entire input @buf, breaking it into
- * "line size" chunks to format and print.
+ * lines of rowsize/groupsize groups of input data converted to hex +
+ * (optionally) ASCII output.
*
* E.g.:
* print_hex_dump(KERN_DEBUG, "raw data: ", DUMP_PREFIX_ADDRESS,
@@ -246,17 +248,29 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
{
const u8 *ptr = buf;
int i, linelen, remaining = len;
- unsigned char linebuf[32 * 3 + 2 + 32 + 1];
+ unsigned char *linebuf;
+ unsigned int linebuf_len;

- if (rowsize != 16 && rowsize != 32)
- rowsize = 16;
+ if (rowsize % groupsize)
+ rowsize -= rowsize % groupsize;
+
+ /* Worst case line length:
+ * 2 hex chars + space per byte in, 2 spaces, 1 char per byte in, NULL
+ */
+ linebuf_len = rowsize * 3 + 2 + rowsize + 1;
+ linebuf = kzalloc(linebuf_len, GFP_KERNEL);
+ if (!linebuf) {
+ printk("%s%shexdump: Could not alloc %u bytes for buffer\n",
+ level, prefix_str, linebuf_len);
+ return;
+ }

for (i = 0; i < len; i += rowsize) {
linelen = min(remaining, rowsize);
remaining -= rowsize;

hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
- linebuf, sizeof(linebuf), ascii);
+ linebuf, linebuf_len, ascii);

switch (prefix_type) {
case DUMP_PREFIX_ADDRESS:
@@ -271,6 +285,8 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
break;
}
}
+
+ kfree(linebuf);
}
EXPORT_SYMBOL(print_hex_dump);

diff --git a/lib/test_hexdump.c b/lib/test_hexdump.c
index d78ddd62ffd0..6ab75a209b43 100644
--- a/lib/test_hexdump.c
+++ b/lib/test_hexdump.c
@@ -14,15 +14,25 @@ static const unsigned char data_b[] = {
'\x70', '\xba', '\xc4', '\x24', '\x7d', '\x83', '\x34', '\x9b', /* 08 - 0f */
'\xa6', '\x9c', '\x31', '\xad', '\x9c', '\x0f', '\xac', '\xe9', /* 10 - 17 */
'\x4c', '\xd1', '\x19', '\x99', '\x43', '\xb1', '\xaf', '\x0c', /* 18 - 1f */
+ '\x00', '\x01', '\x02', '\x03', '\x04', '\x05', '\x06', '\x07', /* 20 - 27 */
+ '\x0f', '\x0e', '\x0d', '\x0c', '\x0b', '\x0a', '\x09', '\x08', /* 28 - 2f */
+ '\x10', '\x11', '\x12', '\x13', '\x14', '\x15', '\x16', '\x17', /* 30 - 37 */
+ '\x1f', '\x1e', '\x1d', '\x1c', '\x1b', '\x1a', '\x19', '\x18', /* 38 - 3f */
};

-static const unsigned char data_a[] = ".2.{....p..$}.4...1.....L...C...";
+static const unsigned char data_a[] = ".2.{....p..$}.4...1.....L...C..."
+ "................................";

static const char * const test_data_1[] __initconst = {
"be", "32", "db", "7b", "0a", "18", "93", "b2",
"70", "ba", "c4", "24", "7d", "83", "34", "9b",
"a6", "9c", "31", "ad", "9c", "0f", "ac", "e9",
"4c", "d1", "19", "99", "43", "b1", "af", "0c",
+ "00", "01", "02", "03", "04", "05", "06", "07",
+ "0f", "0e", "0d", "0c", "0b", "0a", "09", "08",
+ "10", "11", "12", "13", "14", "15", "16", "17",
+ "1f", "1e", "1d", "1c", "1b", "1a", "19", "18",
+ NULL
};

static const char * const test_data_2_le[] __initconst = {
@@ -30,6 +40,11 @@ static const char * const test_data_2_le[] __initconst = {
"ba70", "24c4", "837d", "9b34",
"9ca6", "ad31", "0f9c", "e9ac",
"d14c", "9919", "b143", "0caf",
+ "0100", "0302", "0504", "0706",
+ "0e0f", "0c0d", "0a0b", "0809",
+ "1110", "1312", "1514", "1716",
+ "1e1f", "1c1d", "1a1b", "1819",
+ NULL
};

static const char * const test_data_2_be[] __initconst = {
@@ -37,26 +52,43 @@ static const char * const test_data_2_be[] __initconst = {
"70ba", "c424", "7d83", "349b",
"a69c", "31ad", "9c0f", "ace9",
"4cd1", "1999", "43b1", "af0c",
+ "0001", "0203", "0405", "0607",
+ "0f0e", "0d0c", "0b0a", "0908",
+ "1011", "1213", "1415", "1617",
+ "1f1e", "1d1c", "1b1a", "1918",
+ NULL
};

static const char * const test_data_4_le[] __initconst = {
"7bdb32be", "b293180a", "24c4ba70", "9b34837d",
"ad319ca6", "e9ac0f9c", "9919d14c", "0cafb143",
+ "03020100", "07060504", "0c0d0e0f", "08090a0b",
+ "13121110", "17161514", "1c1d1e1f", "18191a1b",
+ NULL
};

static const char * const test_data_4_be[] __initconst = {
"be32db7b", "0a1893b2", "70bac424", "7d83349b",
"a69c31ad", "9c0face9", "4cd11999", "43b1af0c",
+ "00010203", "04050607", "0f0e0d0c", "0b0a0908",
+ "10111213", "14151617", "1f1e1d1c", "1b1a1918",
+ NULL
};

static const char * const test_data_8_le[] __initconst = {
"b293180a7bdb32be", "9b34837d24c4ba70",
"e9ac0f9cad319ca6", "0cafb1439919d14c",
+ "0706050403020100", "08090a0b0c0d0e0f",
+ "1716151413121110", "18191a1b1c1d1e1f",
+ NULL
};

static const char * const test_data_8_be[] __initconst = {
"be32db7b0a1893b2", "70bac4247d83349b",
"a69c31ad9c0face9", "4cd1199943b1af0c",
+ "0001020304050607", "0f0e0d0c0b0a0908",
+ "1011121314151617", "1f1e1d1c1b1a1918",
+ NULL
};

#define FILL_CHAR '#'
@@ -75,9 +107,6 @@ static void __init test_hexdump_prepare_test(size_t len, int rowsize,
unsigned int i;
const bool is_be = IS_ENABLED(CONFIG_CPU_BIG_ENDIAN);

- if (rs != 16 && rs != 32)
- rs = 16;
-
if (l > rs)
l = rs;

@@ -97,7 +126,12 @@ static void __init test_hexdump_prepare_test(size_t len, int rowsize,
p = test;
for (i = 0; i < l / gs; i++) {
const char *q = *result++;
- size_t amount = strlen(q);
+ size_t amount;
+
+ if (!q)
+ break;
+
+ amount = strlen(q);

memcpy(p, q, amount);
p += amount;
@@ -120,7 +154,7 @@ static void __init test_hexdump_prepare_test(size_t len, int rowsize,
*p = '\0';
}

-#define TEST_HEXDUMP_BUF_SIZE (32 * 3 + 2 + 32 + 1)
+#define TEST_HEXDUMP_BUF_SIZE (64 * 3 + 2 + 64 + 1)

static void __init test_hexdump(size_t len, int rowsize, int groupsize,
bool ascii)
@@ -215,7 +249,7 @@ static void __init test_hexdump_overflow(size_t buflen, size_t len,
static void __init test_hexdump_overflow_set(size_t buflen, bool ascii)
{
unsigned int i = 0;
- int rs = (get_random_int() % 2 + 1) * 16;
+ int rs = (get_random_int() % 4 + 1) * 16;

do {
int gs = 1 << i;
@@ -230,11 +264,11 @@ static int __init test_hexdump_init(void)
unsigned int i;
int rowsize;

- rowsize = (get_random_int() % 2 + 1) * 16;
+ rowsize = (get_random_int() % 4 + 1) * 16;
for (i = 0; i < 16; i++)
test_hexdump_set(rowsize, false);

- rowsize = (get_random_int() % 2 + 1) * 16;
+ rowsize = (get_random_int() % 4 + 1) * 16;
for (i = 0; i < 16; i++)
test_hexdump_set(rowsize, true);

--
2.21.0

2019-05-08 07:03:50

by Alastair D'Silva

[permalink] [raw]
Subject: [PATCH v2 7/7] lib/hexdump.c: Optionally retain byte ordering

From: Alastair D'Silva <[email protected]>

The behaviour of hexdump groups is to print the data out as if
it was a native-endian number.

This patch tweaks the documentation to make this clear, and also
adds the HEXDUMP_RETAIN_BYTE_ORDER flag to allow groups of
multiple bytes to be printed without affecting the ordering
of the printed bytes.

Signed-off-by: Alastair D'Silva <[email protected]>
---
include/linux/printk.h | 1 +
lib/hexdump.c | 30 +++++++++++++++++----
lib/test_hexdump.c | 60 +++++++++++++++++++++++++++++-------------
3 files changed, 68 insertions(+), 23 deletions(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index 5231a14e4593..15277d50159c 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -488,6 +488,7 @@ enum {
#define HEXDUMP_2_GRP_SPACES (1 << 5)
#define HEXDUMP_4_GRP_SPACES (1 << 6)
#define HEXDUMP_8_GRP_SPACES (1 << 7)
+#define HEXDUMP_RETAIN_BYTE_ORDER (1 << 8)

extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
int groupsize, char *linebuf, size_t linebuflen,
diff --git a/lib/hexdump.c b/lib/hexdump.c
index febd614406d1..bfc9800630ae 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -127,7 +127,8 @@ static void separator_parameters(u64 flags, int groupsize, int *sep_chars,
* @buf: data blob to dump
* @len: number of bytes in the @buf
* @rowsize: number of bytes to print per line; must be a multiple of groupsize
- * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1)
+ * @groupsize: number of bytes to convert to a native endian number and print:
+ * 1, 2, 4, 8; default = 1
* @linebuf: where to put the converted data
* @linebuflen: total size of @linebuf, including space for terminating NUL
* @flags: A bitwise OR of the following flags:
@@ -138,6 +139,9 @@ static void separator_parameters(u64 flags, int groupsize, int *sep_chars,
* HEXDUMP_2_GRP_SPACES: insert a ' ' after every 2 groups
* HEXDUMP_4_GRP_SPACES: insert a ' ' after every 4 groups
* HEXDUMP_8_GRP_SPACES: insert a ' ' after every 8 groups
+ * HEXDUMP_RETAIN_BYTE_ORDER: Retain the byte ordering of groups
+ * instead of treating each group as a
+ * native-endian number
*
* hex_dump_to_buffer() works on one "line" of output at a time, converting
* <groupsize> bytes of input to hexadecimal (and optionally printable ASCII)
@@ -171,6 +175,7 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
int ret;
int sep_chars = 0;
char sep = 0;
+ bool big_endian = (flags & HEXDUMP_RETAIN_BYTE_ORDER) ? 1 : 0;

if (!is_power_of_2(groupsize) || groupsize > 8)
groupsize = 1;
@@ -202,10 +207,13 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
const u64 *ptr8 = buf;

for (j = 0; j < ngroups; j++) {
+ u64 val = big_endian ?
+ be64_to_cpu(get_unaligned(ptr8 + j)) :
+ get_unaligned(ptr8 + j);
ret = snprintf(linebuf + lx, linebuflen - lx,
"%s%16.16llx",
j ? group_separator(j, flags) : "",
- get_unaligned(ptr8 + j));
+ val);
if (ret >= linebuflen - lx)
goto overflow1;
lx += ret;
@@ -214,10 +222,14 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
const u32 *ptr4 = buf;

for (j = 0; j < ngroups; j++) {
+ u32 val = big_endian ?
+ be32_to_cpu(get_unaligned(ptr4 + j)) :
+ get_unaligned(ptr4 + j);
+
ret = snprintf(linebuf + lx, linebuflen - lx,
"%s%8.8x",
j ? group_separator(j, flags) : "",
- get_unaligned(ptr4 + j));
+ val);
if (ret >= linebuflen - lx)
goto overflow1;
lx += ret;
@@ -226,10 +238,14 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
const u16 *ptr2 = buf;

for (j = 0; j < ngroups; j++) {
+ u16 val = big_endian ?
+ be16_to_cpu(get_unaligned(ptr2 + j)) :
+ get_unaligned(ptr2 + j);
+
ret = snprintf(linebuf + lx, linebuflen - lx,
"%s%4.4x",
j ? group_separator(j, flags) : "",
- get_unaligned(ptr2 + j));
+ val);
if (ret >= linebuflen - lx)
goto overflow1;
lx += ret;
@@ -331,7 +347,8 @@ static void announce_skipped(const char *level, const char *prefix_str,
* @prefix_type: controls whether prefix of an offset, address, or none
* is printed (%DUMP_PREFIX_OFFSET, %DUMP_PREFIX_ADDRESS, %DUMP_PREFIX_NONE)
* @rowsize: number of bytes to print per line; must be a multiple of groupsize
- * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1)
+ * @groupsize: number of bytes to convert to a native endian number and print:
+ * 1, 2, 4, 8; default = 1
* @buf: data blob to dump
* @len: number of bytes in the @buf
* @ascii: include ASCII after the hex output
@@ -342,6 +359,9 @@ static void announce_skipped(const char *level, const char *prefix_str,
* HEXDUMP_2_GRP_LINES: insert a '|' after every 2 groups
* HEXDUMP_4_GRP_LINES: insert a '|' after every 4 groups
* HEXDUMP_8_GRP_LINES: insert a '|' after every 8 groups
+ * HEXDUMP_RETAIN_BYTE_ORDER: Retain the byte ordering of groups
+ * instead of treating each group as a
+ * native-endian number
*
* Given a buffer of u8 data, print_hex_dump() prints a hex + ASCII dump
* to the kernel log at the specified kernel log level, with an optional
diff --git a/lib/test_hexdump.c b/lib/test_hexdump.c
index ae340c5c1c6f..1e510e934568 100644
--- a/lib/test_hexdump.c
+++ b/lib/test_hexdump.c
@@ -98,14 +98,15 @@ static unsigned failed_tests __initdata;

static void __init test_hexdump_prepare_test(size_t len, int rowsize,
int groupsize, char *test,
- size_t testlen, bool ascii)
+ size_t testlen, u64 flags)
{
char *p;
const char * const *result;
size_t l = len;
int gs = groupsize, rs = rowsize;
unsigned int i;
- const bool is_be = IS_ENABLED(CONFIG_CPU_BIG_ENDIAN);
+ const bool is_be = IS_ENABLED(CONFIG_CPU_BIG_ENDIAN) ||
+ (flags & HEXDUMP_RETAIN_BYTE_ORDER);

if (l > rs)
l = rs;
@@ -142,7 +143,7 @@ static void __init test_hexdump_prepare_test(size_t len, int rowsize,
p--;

/* ASCII part */
- if (ascii) {
+ if (flags & HEXDUMP_ASCII) {
do {
*p++ = ' ';
} while (p < test + rs * 2 + rs / gs + 1);
@@ -157,7 +158,7 @@ static void __init test_hexdump_prepare_test(size_t len, int rowsize,
#define TEST_HEXDUMP_BUF_SIZE (64 * 3 + 2 + 64 + 1)

static void __init test_hexdump(size_t len, int rowsize, int groupsize,
- bool ascii)
+ u64 flags)
{
char test[TEST_HEXDUMP_BUF_SIZE];
char real[TEST_HEXDUMP_BUF_SIZE];
@@ -166,11 +167,11 @@ static void __init test_hexdump(size_t len, int rowsize, int groupsize,

memset(real, FILL_CHAR, sizeof(real));
hex_dump_to_buffer(data_b, len, rowsize, groupsize, real, sizeof(real),
- ascii ? HEXDUMP_ASCII : 0);
+ flags);

memset(test, FILL_CHAR, sizeof(test));
test_hexdump_prepare_test(len, rowsize, groupsize, test, sizeof(test),
- ascii);
+ flags);

if (memcmp(test, real, TEST_HEXDUMP_BUF_SIZE)) {
pr_err("Len: %zu row: %d group: %d\n", len, rowsize, groupsize);
@@ -193,7 +194,7 @@ static void __init test_hexdump_set(int rowsize, bool ascii)

static void __init test_hexdump_overflow(size_t buflen, size_t len,
int rowsize, int groupsize,
- bool ascii)
+ u64 flags)
{
char test[TEST_HEXDUMP_BUF_SIZE];
char buf[TEST_HEXDUMP_BUF_SIZE];
@@ -205,7 +206,7 @@ static void __init test_hexdump_overflow(size_t buflen, size_t len,
memset(buf, FILL_CHAR, sizeof(buf));

rc = hex_dump_to_buffer(data_b, len, rowsize, groupsize, buf, buflen,
- ascii ? HEXDUMP_ASCII : 0);
+ flags);

/*
* Caller must provide the data length multiple of groupsize. The
@@ -222,12 +223,12 @@ static void __init test_hexdump_overflow(size_t buflen, size_t len,
- 1 /* no trailing space */;
}

- expected_len = (ascii) ? ascii_len : hex_len;
+ expected_len = (flags & HEXDUMP_ASCII) ? ascii_len : hex_len;

fill_point = min_t(int, expected_len + 1, buflen);
if (buflen) {
test_hexdump_prepare_test(len, rowsize, groupsize, test,
- sizeof(test), ascii);
+ sizeof(test), flags);
test[fill_point - 1] = '\0';
}
memset(test + fill_point, FILL_CHAR, sizeof(test) - fill_point);
@@ -237,8 +238,8 @@ static void __init test_hexdump_overflow(size_t buflen, size_t len,
buf[sizeof(buf) - 1] = '\0';

if (!match) {
- pr_err("rowsize: %u groupsize: %u ascii: %d Len: %zu buflen: %zu strlen: %zu\n",
- rowsize, groupsize, ascii, len, buflen,
+ pr_err("rowsize: %u groupsize: %u flags: %llx Len: %zu buflen: %zu strlen: %zu\n",
+ rowsize, groupsize, flags, len, buflen,
strnlen(buf, sizeof(buf)));
pr_err("Result: %d '%-.*s'\n", rc, (int)buflen, buf);
pr_err("Expect: %d '%-.*s'\n", expected_len, (int)buflen, test);
@@ -247,7 +248,7 @@ static void __init test_hexdump_overflow(size_t buflen, size_t len,
}
}

-static void __init test_hexdump_overflow_set(size_t buflen, bool ascii)
+static void __init test_hexdump_overflow_set(size_t buflen, u64 flags)
{
unsigned int i = 0;
int rs = (get_random_int() % 4 + 1) * 16;
@@ -256,7 +257,7 @@ static void __init test_hexdump_overflow_set(size_t buflen, bool ascii)
int gs = 1 << i;
size_t len = get_random_int() % rs + gs;

- test_hexdump_overflow(buflen, rounddown(len, gs), rs, gs, ascii);
+ test_hexdump_overflow(buflen, rounddown(len, gs), rs, gs, flags);
} while (i++ < 3);
}

@@ -264,20 +265,43 @@ static int __init test_hexdump_init(void)
{
unsigned int i;
int rowsize;
+ u64 flags;

+ flags = 0;
rowsize = (get_random_int() % 4 + 1) * 16;
for (i = 0; i < 16; i++)
- test_hexdump_set(rowsize, false);
+ test_hexdump_set(rowsize, flags);

+ flags = HEXDUMP_ASCII;
rowsize = (get_random_int() % 4 + 1) * 16;
for (i = 0; i < 16; i++)
- test_hexdump_set(rowsize, true);
+ test_hexdump_set(rowsize, flags);

+ flags = HEXDUMP_RETAIN_BYTE_ORDER;
+ rowsize = (get_random_int() % 2 + 1) * 16;
+ for (i = 0; i < 16; i++)
+ test_hexdump_set(rowsize, flags);
+
+ flags = HEXDUMP_ASCII | HEXDUMP_RETAIN_BYTE_ORDER;
+ rowsize = (get_random_int() % 2 + 1) * 16;
+ for (i = 0; i < 16; i++)
+ test_hexdump_set(rowsize, flags);
+
+ flags = 0;
+ for (i = 0; i <= TEST_HEXDUMP_BUF_SIZE; i++)
+ test_hexdump_overflow_set(i, flags);
+
+ flags = HEXDUMP_ASCII;
+ for (i = 0; i <= TEST_HEXDUMP_BUF_SIZE; i++)
+ test_hexdump_overflow_set(i, flags);
+
+ flags = HEXDUMP_RETAIN_BYTE_ORDER;
for (i = 0; i <= TEST_HEXDUMP_BUF_SIZE; i++)
- test_hexdump_overflow_set(i, false);
+ test_hexdump_overflow_set(i, flags);

+ flags = HEXDUMP_ASCII | HEXDUMP_RETAIN_BYTE_ORDER;
for (i = 0; i <= TEST_HEXDUMP_BUF_SIZE; i++)
- test_hexdump_overflow_set(i, true);
+ test_hexdump_overflow_set(i, flags);

if (failed_tests == 0)
pr_info("all %u tests passed\n", total_tests);
--
2.21.0

2019-05-08 07:04:56

by Alastair D'Silva

[permalink] [raw]
Subject: [PATCH v2 1/7] lib/hexdump.c: Fix selftests

From: Alastair D'Silva <[email protected]>

The overflow tests did not account for the situation where no
overflow occurs and len < rowsize.

This patch renames the cryptic variables and accounts for the
above case.

The selftests now pass.

Signed-off-by: Alastair D'Silva <[email protected]>
---
lib/test_hexdump.c | 47 ++++++++++++++++++++++++++--------------------
1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/lib/test_hexdump.c b/lib/test_hexdump.c
index 5144899d3c6b..d78ddd62ffd0 100644
--- a/lib/test_hexdump.c
+++ b/lib/test_hexdump.c
@@ -163,45 +163,52 @@ static void __init test_hexdump_overflow(size_t buflen, size_t len,
{
char test[TEST_HEXDUMP_BUF_SIZE];
char buf[TEST_HEXDUMP_BUF_SIZE];
- int rs = rowsize, gs = groupsize;
- int ae, he, e, f, r;
- bool a;
+ int ascii_len, hex_len, expected_len, fill_point, ngroups, rc;
+ bool match;

total_tests++;

memset(buf, FILL_CHAR, sizeof(buf));

- r = hex_dump_to_buffer(data_b, len, rs, gs, buf, buflen, ascii);
+ rc = hex_dump_to_buffer(data_b, len, rowsize, groupsize, buf, buflen, ascii);

/*
* Caller must provide the data length multiple of groupsize. The
* calculations below are made with that assumption in mind.
*/
- ae = rs * 2 /* hex */ + rs / gs /* spaces */ + 1 /* space */ + len /* ascii */;
- he = (gs * 2 /* hex */ + 1 /* space */) * len / gs - 1 /* no trailing space */;
+ ngroups = rowsize / groupsize;
+ hex_len = (groupsize * 2 /* hex */ + 1 /* spaces */) * ngroups
+ - 1 /* no trailing space */;
+ ascii_len = hex_len + 2 /* space */ + len /* ascii */;
+
+ if (len < rowsize) {
+ ngroups = len / groupsize;
+ hex_len = (groupsize * 2 /* hex */ + 1 /* spaces */) * ngroups
+ - 1 /* no trailing space */;
+ }

- if (ascii)
- e = ae;
- else
- e = he;
+ expected_len = (ascii) ? ascii_len : hex_len;

- f = min_t(int, e + 1, buflen);
+ fill_point = min_t(int, expected_len + 1, buflen);
if (buflen) {
- test_hexdump_prepare_test(len, rs, gs, test, sizeof(test), ascii);
- test[f - 1] = '\0';
+ test_hexdump_prepare_test(len, rowsize, groupsize, test,
+ sizeof(test), ascii);
+ test[fill_point - 1] = '\0';
}
- memset(test + f, FILL_CHAR, sizeof(test) - f);
+ memset(test + fill_point, FILL_CHAR, sizeof(test) - fill_point);

- a = r == e && !memcmp(test, buf, TEST_HEXDUMP_BUF_SIZE);
+ match = rc == expected_len && !memcmp(test, buf, TEST_HEXDUMP_BUF_SIZE);

buf[sizeof(buf) - 1] = '\0';

- if (!a) {
- pr_err("Len: %zu buflen: %zu strlen: %zu\n",
- len, buflen, strnlen(buf, sizeof(buf)));
- pr_err("Result: %d '%s'\n", r, buf);
- pr_err("Expect: %d '%s'\n", e, test);
+ if (!match) {
+ pr_err("rowsize: %u groupsize: %u ascii: %d Len: %zu buflen: %zu strlen: %zu\n",
+ rowsize, groupsize, ascii, len, buflen,
+ strnlen(buf, sizeof(buf)));
+ pr_err("Result: %d '%-.*s'\n", rc, (int)buflen, buf);
+ pr_err("Expect: %d '%-.*s'\n", expected_len, (int)buflen, test);
failed_tests++;
+
}
}

--
2.21.0

2019-05-09 01:00:14

by Randy Dunlap

[permalink] [raw]
Subject: Re: [PATCH v2 3/7] lib/hexdump.c: Optionally suppress lines of repeated bytes

On 5/8/19 12:01 AM, Alastair D'Silva wrote:
> From: Alastair D'Silva <[email protected]>
>
> Some buffers may only be partially filled with useful data, while the rest
> is padded (typically with 0x00 or 0xff).
>
> This patch introduces a flag to allow the supression of lines of repeated
> bytes, which are replaced with '** Skipped %u bytes of value 0x%x **'
>
> An inline wrapper function is provided for backwards compatibility with
> existing code, which maintains the original behaviour.
>
> Signed-off-by: Alastair D'Silva <[email protected]>
> ---
> include/linux/printk.h | 25 +++++++++---
> lib/hexdump.c | 91 ++++++++++++++++++++++++++++++++++++------
> 2 files changed, 99 insertions(+), 17 deletions(-)
>

Hi,
Did you do "make htmldocs" or something similar on this?

> diff --git a/lib/hexdump.c b/lib/hexdump.c
> index 3943507bc0e9..d61a1e4f19fa 100644
> --- a/lib/hexdump.c
> +++ b/lib/hexdump.c
> @@ -212,8 +212,44 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
> EXPORT_SYMBOL(hex_dump_to_buffer);
>
> #ifdef CONFIG_PRINTK
> +
> +/**
> + * Check if a buffer contains only a single byte value
> + * @buf: pointer to the buffer
> + * @len: the size of the buffer in bytes
> + * @val: outputs the value if if the bytes are identical

Does this work without a function name?
Documentation/doc-guide/kernel-doc.rst says the general format is:

/**
* function_name() - Brief description of function.
* @arg1: Describe the first argument.
* @arg2: Describe the second argument.
* One can provide multiple line descriptions
* for arguments.
*

> + */

> /**
> - * print_hex_dump - print a text hex dump to syslog for a binary blob of data
> + * print_hex_dump_ext: dump a binary blob of data to syslog in hexadecimal

Also not in the general documented format.

> * @level: kernel log level (e.g. KERN_DEBUG)
> * @prefix_str: string to prefix each line with;
> * caller supplies trailing spaces for alignment if desired


--
~Randy

2019-05-10 00:18:07

by Alastair D'Silva

[permalink] [raw]
Subject: Re: [PATCH v2 3/7] lib/hexdump.c: Optionally suppress lines of repeated bytes

On Wed, 2019-05-08 at 17:58 -0700, Randy Dunlap wrote:
> On 5/8/19 12:01 AM, Alastair D'Silva wrote:
> > From: Alastair D'Silva <[email protected]>
> >
> > Some buffers may only be partially filled with useful data, while
> > the rest
> > is padded (typically with 0x00 or 0xff).
> >
> > This patch introduces a flag to allow the supression of lines of
> > repeated
> > bytes, which are replaced with '** Skipped %u bytes of value 0x%x
> > **'
> >
> > An inline wrapper function is provided for backwards compatibility
> > with
> > existing code, which maintains the original behaviour.
> >
> > Signed-off-by: Alastair D'Silva <[email protected]>
> > ---
> > include/linux/printk.h | 25 +++++++++---
> > lib/hexdump.c | 91 ++++++++++++++++++++++++++++++++++++
> > ------
> > 2 files changed, 99 insertions(+), 17 deletions(-)
> >
>
> Hi,
> Did you do "make htmldocs" or something similar on this?
>
> > diff --git a/lib/hexdump.c b/lib/hexdump.c
> > index 3943507bc0e9..d61a1e4f19fa 100644
> > --- a/lib/hexdump.c
> > +++ b/lib/hexdump.c
> > @@ -212,8 +212,44 @@ int hex_dump_to_buffer(const void *buf, size_t
> > len, int rowsize, int groupsize,
> > EXPORT_SYMBOL(hex_dump_to_buffer);
> >
> > #ifdef CONFIG_PRINTK
> > +
> > +/**
> > + * Check if a buffer contains only a single byte value
> > + * @buf: pointer to the buffer
> > + * @len: the size of the buffer in bytes
> > + * @val: outputs the value if if the bytes are identical
>
> Does this work without a function name?
> Documentation/doc-guide/kernel-doc.rst says the general format is:
>
> /**
> * function_name() - Brief description of function.
> * @arg1: Describe the first argument.
> * @arg2: Describe the second argument.
> * One can provide multiple line descriptions
> * for arguments.
> *
>
> > + */
> > /**
> > - * print_hex_dump - print a text hex dump to syslog for a binary
> > blob of data
> > + * print_hex_dump_ext: dump a binary blob of data to syslog in
> > hexadecimal
>
> Also not in the general documented format.
>

Thanks Randy, I'll address these.

--
Alastair D'Silva mob: 0423 762 819
skype: alastair_dsilva
Twitter: @EvilDeece
blog: http://alastair.d-silva.org


2019-05-13 07:05:23

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH v2 3/7] lib/hexdump.c: Optionally suppress lines of repeated bytes

Hi Alastair,

Thanks for your patch!

On Wed, May 8, 2019 at 9:04 AM Alastair D'Silva <[email protected]> wrote:
> From: Alastair D'Silva <[email protected]>
>
> Some buffers may only be partially filled with useful data, while the rest
> is padded (typically with 0x00 or 0xff).
>
> This patch introduces a flag to allow the supression of lines of repeated
> bytes,

Given print_hex_dump() operates on entities of groupsize (1, 2, 4, or 8)
bytes, wouldn't it make more sense to consider repeated groups instead
of repeated bytes?

> which are replaced with '** Skipped %u bytes of value 0x%x **'

Using a custom message instead of just "*", like "hexdump" uses, will
require preprocessing the output when recovering the original binary
data by feeding it to e.g. "xxd".
This may sound worse than it is, though, as I never got "xxd" to work
without preprocessing anyway ;-)

$ cat $(type -p unhexdump)
#!/bin/sh
sed 's/^[0-9a-f]*//' $1 | xxd -r -p | dd conv=swab

Gr{oetje,eeting}s,

Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- [email protected]

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds

2019-05-13 08:10:01

by Alastair D'Silva

[permalink] [raw]
Subject: RE: [PATCH v2 3/7] lib/hexdump.c: Optionally suppress lines of repeated bytes

> -----Original Message-----
> From: Geert Uytterhoeven <[email protected]>
> Sent: Monday, 13 May 2019 5:01 PM
> To: Alastair D'Silva <[email protected]>
> Cc: [email protected]; Jani Nikula <[email protected]>; Joonas
> Lahtinen <[email protected]>; Rodrigo Vivi
> <[email protected]>; David Airlie <[email protected]>; Daniel Vetter
> <[email protected]>; Dan Carpenter <[email protected]>; Karsten
> Keil <[email protected]>; Jassi Brar <[email protected]>; Tom
> Lendacky <[email protected]>; David S. Miller
> <[email protected]>; Jose Abreu <[email protected]>; Kalle
> Valo <[email protected]>; Stanislaw Gruszka <[email protected]>;
> Benson Leung <[email protected]>; Enric Balletbo i Serra
> <[email protected]>; James E.J. Bottomley
> <[email protected]>; Martin K. Petersen <[email protected]>;
> Greg Kroah-Hartman <[email protected]>; Alexander Viro
> <[email protected]>; Petr Mladek <[email protected]>; Sergey
> Senozhatsky <[email protected]>; Steven Rostedt
> <[email protected]>; David Laight <[email protected]>; Andrew
> Morton <[email protected]>; Intel Graphics Development <intel-
> [email protected]>; DRI Development <dri-
> [email protected]>; Linux Kernel Mailing List <linux-
> [email protected]>; netdev <[email protected]>;
> [email protected]; linux-wireless <[email protected]>;
> scsi <[email protected]>; Linux Fbdev development list <linux-
> [email protected]>; driverdevel <[email protected]>; Linux
> FS Devel <[email protected]>
> Subject: Re: [PATCH v2 3/7] lib/hexdump.c: Optionally suppress lines of
> repeated bytes
>
> Hi Alastair,
>
> Thanks for your patch!

And thanks for your politeness :)

>
> On Wed, May 8, 2019 at 9:04 AM Alastair D'Silva <[email protected]>
> wrote:
> > From: Alastair D'Silva <[email protected]>
> >
> > Some buffers may only be partially filled with useful data, while the
> > rest is padded (typically with 0x00 or 0xff).
> >
> > This patch introduces a flag to allow the supression of lines of
> > repeated bytes,
>
> Given print_hex_dump() operates on entities of groupsize (1, 2, 4, or 8)
> bytes, wouldn't it make more sense to consider repeated groups instead of
> repeated bytes?

Maybe, it would mean that subsequent addresses may not be a multiple of rowsize though, which is useful.

> > which are replaced with '** Skipped %u bytes of value 0x%x **'
>
> Using a custom message instead of just "*", like "hexdump" uses, will require
> preprocessing the output when recovering the original binary data by
> feeding it to e.g. "xxd".
> This may sound worse than it is, though, as I never got "xxd" to work without
> preprocessing anyway ;-)

I think showing the details of the skipped values is useful when reading the output directly. In situations where binary extracts are desired, the feature can always be disabled.

--
Alastair D'Silva mob: 0423 762 819
skype: alastair_dsilva msn: [email protected]
blog: http://alastair.d-silva.org Twitter: @EvilDeece