* [PATCH v3 6/7] lib/hexdump.c: Allow multiple groups to be separated by spaces
From: Alastair D'Silva @ 2019-06-17 2:04 UTC (permalink / raw)
To: alastair
Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
Daniel Vetter, Dan Carpenter, Karsten Keil, Jassi Brar,
Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
Alexander Viro, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
David Laight, Andrew Morton, intel-gfx, dri-devel, linux-kernel,
netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
linux-fsdevel
In-Reply-To: <20190617020430.8708-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
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 <alastair@d-silva.org>
---
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 c6b748f66a82..04416e788802 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -487,6 +487,9 @@ enum {
#define HEXDUMP_2_GRP_LINES BIT(2)
#define HEXDUMP_4_GRP_LINES BIT(3)
#define HEXDUMP_8_GRP_LINES BIT(4)
+#define HEXDUMP_2_GRP_SPACES BIT(5)
+#define HEXDUMP_4_GRP_SPACES BIT(6)
+#define HEXDUMP_8_GRP_SPACES BIT(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 4da7d24826fb..dc85ef0dbb0a 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
^ permalink raw reply related
* [PATCH v3 7/7] lib/hexdump.c: Optionally retain byte ordering
From: Alastair D'Silva @ 2019-06-17 2:04 UTC (permalink / raw)
To: alastair
Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
Daniel Vetter, Dan Carpenter, Karsten Keil, Jassi Brar,
Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
Alexander Viro, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
David Laight, Andrew Morton, intel-gfx, dri-devel, linux-kernel,
netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
linux-fsdevel
In-Reply-To: <20190617020430.8708-1-alastair@au1.ibm.com>
From: Alastair D'Silva <alastair@d-silva.org>
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 <alastair@d-silva.org>
---
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 04416e788802..ffc94bedd737 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -490,6 +490,7 @@ enum {
#define HEXDUMP_2_GRP_SPACES BIT(5)
#define HEXDUMP_4_GRP_SPACES BIT(6)
#define HEXDUMP_8_GRP_SPACES BIT(7)
+#define HEXDUMP_RETAIN_BYTE_ORDER BIT(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 dc85ef0dbb0a..ce14abc7701f 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
^ permalink raw reply related
* [PATCH v3 0/7] Hexdump Enhancements
From: Alastair D'Silva @ 2019-06-17 2:04 UTC (permalink / raw)
To: alastair
Cc: Jani Nikula, Joonas Lahtinen, Rodrigo Vivi, David Airlie,
Daniel Vetter, Dan Carpenter, Karsten Keil, Jassi Brar,
Tom Lendacky, David S. Miller, Jose Abreu, Kalle Valo,
Stanislaw Gruszka, Benson Leung, Enric Balletbo i Serra,
James E.J. Bottomley, Martin K. Petersen, Greg Kroah-Hartman,
Alexander Viro, Petr Mladek, Sergey Senozhatsky, Steven Rostedt,
David Laight, Andrew Morton, intel-gfx, dri-devel, linux-kernel,
netdev, ath10k, linux-wireless, linux-scsi, linux-fbdev, devel,
linux-fsdevel
From: Alastair D'Silva <alastair@d-silva.org>
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:
V3:
- Fix inline documention
- use BIT macros
- use u32 rather than u64 for flags
V2:
- 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
^ permalink raw reply
* Re: [PATCH] wireless: airo: switch to skcipher interface
From: Johannes Berg @ 2019-06-16 20:08 UTC (permalink / raw)
To: Eric Biggers, Ard Biesheuvel
Cc: linux-wireless, kvalo, linux-crypto, herbert, Ondrej Zary
In-Reply-To: <20190616071206.GB698@sol.localdomain>
On Sun, 2019-06-16 at 00:12 -0700, Eric Biggers wrote:
>
> The actual crypto in this driver, on the other hand, looks very outdated and
> broken. Apparently it's implementing some Cisco proprietary extension to WEP
> that uses a universal hashing based MAC, where the hash key is generated from
> AES-CTR. But the MAC is only 32 bits, and the universal hash (MMH) is
> implemented incorrectly: there's an off-by-one error in emmh32_final() in the
> code that is supposed to be an optimized version of 'sum % ((1ULL << 32) + 15)'.
>
> Do we know whether anyone is actually using this, or is this just another old
> driver that's sitting around unused?
I'd guess the latter, though as recent as 2015 Ondrej (CC'ed now)
actually changed something in the driver that wasn't just cleanups ...
Remove at least this weird Cisco WEP-with-AES-key-generation mode? That
must pre-date even TKIP?
johannes
^ permalink raw reply
* Re: [PATCH] lib80211: use crypto API ccm(aes) transform for CCMP processing
From: Eric Biggers @ 2019-06-16 19:47 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: <linux-wireless@vger.kernel.org>, Johannes Berg,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE, Herbert Xu
In-Reply-To: <CAKv+Gu_WiUcdKmDhtNf69P=uVxSLqV+RvscN9BhFg62mQTB=vA@mail.gmail.com>
On Sun, Jun 16, 2019 at 09:07:28PM +0200, Ard Biesheuvel wrote:
> On Sun, 16 Jun 2019 at 21:01, Eric Biggers <ebiggers@kernel.org> wrote:
> >
> > Hi Ard,
> >
> > On Fri, Jun 14, 2019 at 11:29:22AM +0200, Ard Biesheuvel wrote:
> > > -static void ccmp_init_blocks(struct crypto_cipher *tfm,
> > > - struct ieee80211_hdr *hdr,
> > > - u8 * pn, size_t dlen, u8 * b0, u8 * auth, u8 * s0)
> > > +static void ccmp_init_blocks(struct ieee80211_hdr *hdr,
> > > + u8 * pn, size_t dlen, u8 * b0, u8 * aad)
> > > {
> > > u8 *pos, qc = 0;
> > > size_t aad_len;
> > > int a4_included, qc_included;
> > > - u8 aad[2 * AES_BLOCK_LEN];
> > >
> > > a4_included = ieee80211_has_a4(hdr->frame_control);
> > > qc_included = ieee80211_is_data_qos(hdr->frame_control);
> > > @@ -131,17 +123,19 @@ static void ccmp_init_blocks(struct crypto_cipher *tfm,
> > > aad_len += 2;
> > > }
> > >
> > > - /* CCM Initial Block:
> > > - * Flag (Include authentication header, M=3 (8-octet MIC),
> > > - * L=1 (2-octet Dlen))
> > > - * Nonce: 0x00 | A2 | PN
> > > - * Dlen */
> > > - b0[0] = 0x59;
> > > + /* In CCM, the initial vectors (IV) used for CTR mode encryption and CBC
> > > + * mode authentication are not allowed to collide, yet both are derived
> > > + * from this vector b0. We only set L := 1 here to indicate that the
> > > + * data size can be represented in (L+1) bytes. The CCM layer will take
> > > + * care of storing the data length in the top (L+1) bytes and setting
> > > + * and clearing the other bits as is required to derive the two IVs.
> > > + */
> > > + b0[0] = 0x1;
> > > +
> > > + /* Nonce: QC | A2 | PN */
> > > b0[1] = qc;
> > > memcpy(b0 + 2, hdr->addr2, ETH_ALEN);
> > > memcpy(b0 + 8, pn, CCMP_PN_LEN);
> > > - b0[14] = (dlen >> 8) & 0xff;
> > > - b0[15] = dlen & 0xff;
> > >
> > > /* AAD:
> > > * FC with bits 4..6 and 11..13 masked to zero; 14 is always one
> > > @@ -166,16 +160,6 @@ static void ccmp_init_blocks(struct crypto_cipher *tfm,
> > > aad[a4_included ? 30 : 24] = qc;
> > > /* rest of QC masked */
> > > }
> > > -
> > > - /* Start with the first block and AAD */
> > > - lib80211_ccmp_aes_encrypt(tfm, b0, auth);
> > > - xor_block(auth, aad, AES_BLOCK_LEN);
> > > - lib80211_ccmp_aes_encrypt(tfm, auth, auth);
> > > - xor_block(auth, &aad[AES_BLOCK_LEN], AES_BLOCK_LEN);
> > > - lib80211_ccmp_aes_encrypt(tfm, auth, auth);
> > > - b0[0] &= 0x07;
> > > - b0[14] = b0[15] = 0;
> > > - lib80211_ccmp_aes_encrypt(tfm, b0, s0);
> > > }
> >
> > How about shifting the contents of aad over by 2 bytes and returning the AAD
> > length from this function instead? It's confusing to still manually format the
> > AAD length for CCM mode, when actually it's ignored now.
> >
> > Also I suggest fixing up the naming:
> >
> > ccmp_init_blocks() => ccmp_init_iv_and_aad()
> > b0 => iv
> >
> > >
> > > static int lib80211_ccmp_hdr(struct sk_buff *skb, int hdr_len,
> > > @@ -218,13 +202,13 @@ static int lib80211_ccmp_hdr(struct sk_buff *skb, int hdr_len,
> > > static int lib80211_ccmp_encrypt(struct sk_buff *skb, int hdr_len, void *priv)
> > > {
> > > struct lib80211_ccmp_data *key = priv;
> > > - int data_len, i, blocks, last, len;
> > > - u8 *pos, *mic;
> > > struct ieee80211_hdr *hdr;
> > > - u8 *b0 = key->tx_b0;
> > > - u8 *b = key->tx_b;
> > > - u8 *e = key->tx_e;
> > > - u8 *s0 = key->tx_s0;
> > > + struct aead_request *req;
> > > + struct scatterlist sg[2];
> > > + u8 *aad = key->tx_aad;
> > > + u8 b0[AES_BLOCK_LEN];
> > > + int len, data_len;
> > > + int ret;
> > >
> > > if (skb_tailroom(skb) < CCMP_MIC_LEN || skb->len < hdr_len)
> > > return -1;
> > > @@ -234,31 +218,29 @@ static int lib80211_ccmp_encrypt(struct sk_buff *skb, int hdr_len, void *priv)
> > > if (len < 0)
> > > return -1;
> > >
> > > - pos = skb->data + hdr_len + CCMP_HDR_LEN;
> > > + req = kzalloc(sizeof(*req) + crypto_aead_reqsize(key->tfm), GFP_ATOMIC);
> > > + if (!req)
> > > + return -ENOMEM;
> > > +
> >
> > Why not kzalloc() and kzfree() instead of aead_request_alloc() and
> > aead_request_free()? Same in lib80211_ccmp_decrypt().
> >
>
> Why kzalloc, right? (i.e., without the 'not'). Good question, I'll change that.
>
Yes ignore the "not", sorry.
- Eric
^ permalink raw reply
* Re: [PATCH] wireless: airo: switch to skcipher interface
From: Eric Biggers @ 2019-06-16 19:43 UTC (permalink / raw)
To: Ard Biesheuvel
Cc: <linux-wireless@vger.kernel.org>, Kalle Valo,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE, Herbert Xu
In-Reply-To: <CAKv+Gu97xkz5qxycyjqmukFhWAD6p=eYbTqoPWt-ZNbBFDbNAw@mail.gmail.com>
On Sun, Jun 16, 2019 at 09:03:58PM +0200, Ard Biesheuvel wrote:
> >
> > Otherwise this patch looks correct to me.
> >
> > The actual crypto in this driver, on the other hand, looks very outdated and
> > broken. Apparently it's implementing some Cisco proprietary extension to WEP
> > that uses a universal hashing based MAC, where the hash key is generated from
> > AES-CTR. But the MAC is only 32 bits, and the universal hash (MMH) is
> > implemented incorrectly: there's an off-by-one error in emmh32_final() in the
> > code that is supposed to be an optimized version of 'sum % ((1ULL << 32) + 15)'.
> >
>
> I stared at that code for a bit, and I don't see the problem.
>
I'm fairly certain that the line:
if (utmp > 0x10000000fLL)
is supposed to be:
if (utmp >= 0x10000000fLL)
Since it's doing mod 0x10000000f. It's supposed to be an optimized
implementation of 'val = (u32)(context->accum % 0x10000000f)' where 0x10000000f
is the prime number 2^32 + 15. It's meant to be the MMH algorithm: Section 3.2
of https://link.springer.com/content/pdf/10.1007/BFb0052345.pdf. But there are
values of 'accum' where it gives the wrong result, e.g. 14137323879880455377.
Possibly this is a bug in the Cisco MIC protocol itself so can't be fixed.
> > Do we know whether anyone is actually using this, or is this just another old
> > driver that's sitting around unused?
> >
>
> Excellent question. I take it this is pre-802.11b hardware, and so
> even the OpenWRT people are unlikely to still be using this.
- Eric
^ permalink raw reply
* Re: [PATCH] lib80211: use crypto API ccm(aes) transform for CCMP processing
From: Eric Biggers @ 2019-06-16 19:11 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-wireless, johannes, linux-crypto, herbert
In-Reply-To: <20190616190136.GA923@sol.localdomain>
On Sun, Jun 16, 2019 at 12:01:38PM -0700, Eric Biggers wrote:
> Hi Ard,
>
> On Fri, Jun 14, 2019 at 11:29:22AM +0200, Ard Biesheuvel wrote:
> > -static void ccmp_init_blocks(struct crypto_cipher *tfm,
> > - struct ieee80211_hdr *hdr,
> > - u8 * pn, size_t dlen, u8 * b0, u8 * auth, u8 * s0)
> > +static void ccmp_init_blocks(struct ieee80211_hdr *hdr,
> > + u8 * pn, size_t dlen, u8 * b0, u8 * aad)
> > {
> > u8 *pos, qc = 0;
> > size_t aad_len;
> > int a4_included, qc_included;
> > - u8 aad[2 * AES_BLOCK_LEN];
> >
> > a4_included = ieee80211_has_a4(hdr->frame_control);
> > qc_included = ieee80211_is_data_qos(hdr->frame_control);
> > @@ -131,17 +123,19 @@ static void ccmp_init_blocks(struct crypto_cipher *tfm,
> > aad_len += 2;
> > }
> >
> > - /* CCM Initial Block:
> > - * Flag (Include authentication header, M=3 (8-octet MIC),
> > - * L=1 (2-octet Dlen))
> > - * Nonce: 0x00 | A2 | PN
> > - * Dlen */
> > - b0[0] = 0x59;
> > + /* In CCM, the initial vectors (IV) used for CTR mode encryption and CBC
> > + * mode authentication are not allowed to collide, yet both are derived
> > + * from this vector b0. We only set L := 1 here to indicate that the
> > + * data size can be represented in (L+1) bytes. The CCM layer will take
> > + * care of storing the data length in the top (L+1) bytes and setting
> > + * and clearing the other bits as is required to derive the two IVs.
> > + */
> > + b0[0] = 0x1;
> > +
> > + /* Nonce: QC | A2 | PN */
> > b0[1] = qc;
> > memcpy(b0 + 2, hdr->addr2, ETH_ALEN);
> > memcpy(b0 + 8, pn, CCMP_PN_LEN);
> > - b0[14] = (dlen >> 8) & 0xff;
> > - b0[15] = dlen & 0xff;
> >
> > /* AAD:
> > * FC with bits 4..6 and 11..13 masked to zero; 14 is always one
> > @@ -166,16 +160,6 @@ static void ccmp_init_blocks(struct crypto_cipher *tfm,
> > aad[a4_included ? 30 : 24] = qc;
> > /* rest of QC masked */
> > }
> > -
> > - /* Start with the first block and AAD */
> > - lib80211_ccmp_aes_encrypt(tfm, b0, auth);
> > - xor_block(auth, aad, AES_BLOCK_LEN);
> > - lib80211_ccmp_aes_encrypt(tfm, auth, auth);
> > - xor_block(auth, &aad[AES_BLOCK_LEN], AES_BLOCK_LEN);
> > - lib80211_ccmp_aes_encrypt(tfm, auth, auth);
> > - b0[0] &= 0x07;
> > - b0[14] = b0[15] = 0;
> > - lib80211_ccmp_aes_encrypt(tfm, b0, s0);
> > }
>
> How about shifting the contents of aad over by 2 bytes and returning the AAD
> length from this function instead? It's confusing to still manually format the
> AAD length for CCM mode, when actually it's ignored now.
>
> Also I suggest fixing up the naming:
>
> ccmp_init_blocks() => ccmp_init_iv_and_aad()
> b0 => iv
>
Okay, couple more things. The 'dlen' parameter is no longer used so should be
removed. Also consider constifying 'hdr' and 'pn' to make it clear what's input
vs. output.
Also, xor_block() is no longer used so should be removed.
- Eric
^ permalink raw reply
* Re: [PATCH] lib80211: use crypto API ccm(aes) transform for CCMP processing
From: Ard Biesheuvel @ 2019-06-16 19:07 UTC (permalink / raw)
To: Eric Biggers
Cc: <linux-wireless@vger.kernel.org>, Johannes Berg,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE, Herbert Xu
In-Reply-To: <20190616190136.GA923@sol.localdomain>
On Sun, 16 Jun 2019 at 21:01, Eric Biggers <ebiggers@kernel.org> wrote:
>
> Hi Ard,
>
> On Fri, Jun 14, 2019 at 11:29:22AM +0200, Ard Biesheuvel wrote:
> > -static void ccmp_init_blocks(struct crypto_cipher *tfm,
> > - struct ieee80211_hdr *hdr,
> > - u8 * pn, size_t dlen, u8 * b0, u8 * auth, u8 * s0)
> > +static void ccmp_init_blocks(struct ieee80211_hdr *hdr,
> > + u8 * pn, size_t dlen, u8 * b0, u8 * aad)
> > {
> > u8 *pos, qc = 0;
> > size_t aad_len;
> > int a4_included, qc_included;
> > - u8 aad[2 * AES_BLOCK_LEN];
> >
> > a4_included = ieee80211_has_a4(hdr->frame_control);
> > qc_included = ieee80211_is_data_qos(hdr->frame_control);
> > @@ -131,17 +123,19 @@ static void ccmp_init_blocks(struct crypto_cipher *tfm,
> > aad_len += 2;
> > }
> >
> > - /* CCM Initial Block:
> > - * Flag (Include authentication header, M=3 (8-octet MIC),
> > - * L=1 (2-octet Dlen))
> > - * Nonce: 0x00 | A2 | PN
> > - * Dlen */
> > - b0[0] = 0x59;
> > + /* In CCM, the initial vectors (IV) used for CTR mode encryption and CBC
> > + * mode authentication are not allowed to collide, yet both are derived
> > + * from this vector b0. We only set L := 1 here to indicate that the
> > + * data size can be represented in (L+1) bytes. The CCM layer will take
> > + * care of storing the data length in the top (L+1) bytes and setting
> > + * and clearing the other bits as is required to derive the two IVs.
> > + */
> > + b0[0] = 0x1;
> > +
> > + /* Nonce: QC | A2 | PN */
> > b0[1] = qc;
> > memcpy(b0 + 2, hdr->addr2, ETH_ALEN);
> > memcpy(b0 + 8, pn, CCMP_PN_LEN);
> > - b0[14] = (dlen >> 8) & 0xff;
> > - b0[15] = dlen & 0xff;
> >
> > /* AAD:
> > * FC with bits 4..6 and 11..13 masked to zero; 14 is always one
> > @@ -166,16 +160,6 @@ static void ccmp_init_blocks(struct crypto_cipher *tfm,
> > aad[a4_included ? 30 : 24] = qc;
> > /* rest of QC masked */
> > }
> > -
> > - /* Start with the first block and AAD */
> > - lib80211_ccmp_aes_encrypt(tfm, b0, auth);
> > - xor_block(auth, aad, AES_BLOCK_LEN);
> > - lib80211_ccmp_aes_encrypt(tfm, auth, auth);
> > - xor_block(auth, &aad[AES_BLOCK_LEN], AES_BLOCK_LEN);
> > - lib80211_ccmp_aes_encrypt(tfm, auth, auth);
> > - b0[0] &= 0x07;
> > - b0[14] = b0[15] = 0;
> > - lib80211_ccmp_aes_encrypt(tfm, b0, s0);
> > }
>
> How about shifting the contents of aad over by 2 bytes and returning the AAD
> length from this function instead? It's confusing to still manually format the
> AAD length for CCM mode, when actually it's ignored now.
>
> Also I suggest fixing up the naming:
>
> ccmp_init_blocks() => ccmp_init_iv_and_aad()
> b0 => iv
>
> >
> > static int lib80211_ccmp_hdr(struct sk_buff *skb, int hdr_len,
> > @@ -218,13 +202,13 @@ static int lib80211_ccmp_hdr(struct sk_buff *skb, int hdr_len,
> > static int lib80211_ccmp_encrypt(struct sk_buff *skb, int hdr_len, void *priv)
> > {
> > struct lib80211_ccmp_data *key = priv;
> > - int data_len, i, blocks, last, len;
> > - u8 *pos, *mic;
> > struct ieee80211_hdr *hdr;
> > - u8 *b0 = key->tx_b0;
> > - u8 *b = key->tx_b;
> > - u8 *e = key->tx_e;
> > - u8 *s0 = key->tx_s0;
> > + struct aead_request *req;
> > + struct scatterlist sg[2];
> > + u8 *aad = key->tx_aad;
> > + u8 b0[AES_BLOCK_LEN];
> > + int len, data_len;
> > + int ret;
> >
> > if (skb_tailroom(skb) < CCMP_MIC_LEN || skb->len < hdr_len)
> > return -1;
> > @@ -234,31 +218,29 @@ static int lib80211_ccmp_encrypt(struct sk_buff *skb, int hdr_len, void *priv)
> > if (len < 0)
> > return -1;
> >
> > - pos = skb->data + hdr_len + CCMP_HDR_LEN;
> > + req = kzalloc(sizeof(*req) + crypto_aead_reqsize(key->tfm), GFP_ATOMIC);
> > + if (!req)
> > + return -ENOMEM;
> > +
>
> Why not kzalloc() and kzfree() instead of aead_request_alloc() and
> aead_request_free()? Same in lib80211_ccmp_decrypt().
>
Why kzalloc, right? (i.e., without the 'not'). Good question, I'll change that.
> Otherwise this patch looks good, though I'd like for someone to test it.
>
> Thanks for doing this!
>
As you know, I want to get rid of all the crypto cobbled together
using the cipher interface. I guess it's my turn to clean up some of
this mess :-)
^ permalink raw reply
* Re: [PATCH] wireless: airo: switch to skcipher interface
From: Ard Biesheuvel @ 2019-06-16 19:03 UTC (permalink / raw)
To: Eric Biggers
Cc: <linux-wireless@vger.kernel.org>, Kalle Valo,
open list:HARDWARE RANDOM NUMBER GENERATOR CORE, Herbert Xu
In-Reply-To: <20190616071206.GB698@sol.localdomain>
On Sun, 16 Jun 2019 at 09:12, Eric Biggers <ebiggers@kernel.org> wrote:
>
> On Fri, Jun 14, 2019 at 11:36:03AM +0200, Ard Biesheuvel wrote:
> > The AIRO driver applies a ctr(aes) on a buffer of considerable size
> > (2400 bytes), and instead of invoking the crypto API to handle this
> > in its entirety, it open codes the counter manipulation and invokes
> > the AES block cipher directly.
> >
> > Let's fix this, by switching to the sync skcipher API instead.
> >
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> > ---
> > NOTE: build tested only, since I don't have the hardware
> >
> > drivers/net/wireless/cisco/airo.c | 57 ++++++++++----------
> > 1 file changed, 27 insertions(+), 30 deletions(-)
> >
>
> Need to also select CRYPTO_CTR in drivers/net/wireless/cisco/Kconfig under
> AIRO_CS, and I think also CRYPTO_BLKCIPHER under AIRO.
>
OK
> > diff --git a/drivers/net/wireless/cisco/airo.c b/drivers/net/wireless/cisco/airo.c
> > index 3f5a14112c6b..2d29ad10505b 100644
> > --- a/drivers/net/wireless/cisco/airo.c
> > +++ b/drivers/net/wireless/cisco/airo.c
> > @@ -49,6 +49,9 @@
> > #include <linux/kthread.h>
> > #include <linux/freezer.h>
> >
> > +#include <crypto/aes.h>
> > +#include <crypto/skcipher.h>
> > +
> > #include <net/cfg80211.h>
> > #include <net/iw_handler.h>
> >
> > @@ -951,7 +954,7 @@ typedef struct {
> > } mic_statistics;
> >
> > typedef struct {
> > - u32 coeff[((EMMH32_MSGLEN_MAX)+3)>>2];
> > + __be32 coeff[((EMMH32_MSGLEN_MAX)+3)>>2];
> > u64 accum; // accumulated mic, reduced to u32 in final()
> > int position; // current position (byte offset) in message
> > union {
> > @@ -1216,7 +1219,7 @@ struct airo_info {
> > struct iw_spy_data spy_data;
> > struct iw_public_data wireless_data;
> > /* MIC stuff */
> > - struct crypto_cipher *tfm;
> > + struct crypto_sync_skcipher *tfm;
> > mic_module mod[2];
> > mic_statistics micstats;
> > HostRxDesc rxfids[MPI_MAX_FIDS]; // rx/tx/config MPI350 descriptors
> > @@ -1291,14 +1294,14 @@ static int flashrestart(struct airo_info *ai,struct net_device *dev);
> > static int RxSeqValid (struct airo_info *ai,miccntx *context,int mcast,u32 micSeq);
> > static void MoveWindow(miccntx *context, u32 micSeq);
> > static void emmh32_setseed(emmh32_context *context, u8 *pkey, int keylen,
> > - struct crypto_cipher *tfm);
> > + struct crypto_sync_skcipher *tfm);
> > static void emmh32_init(emmh32_context *context);
> > static void emmh32_update(emmh32_context *context, u8 *pOctets, int len);
> > static void emmh32_final(emmh32_context *context, u8 digest[4]);
> > static int flashpchar(struct airo_info *ai,int byte,int dwelltime);
> >
> > static void age_mic_context(miccntx *cur, miccntx *old, u8 *key, int key_len,
> > - struct crypto_cipher *tfm)
> > + struct crypto_sync_skcipher *tfm)
> > {
> > /* If the current MIC context is valid and its key is the same as
> > * the MIC register, there's nothing to do.
> > @@ -1359,7 +1362,7 @@ static int micsetup(struct airo_info *ai) {
> > int i;
> >
> > if (ai->tfm == NULL)
> > - ai->tfm = crypto_alloc_cipher("aes", 0, 0);
> > + ai->tfm = crypto_alloc_sync_skcipher("ctr(aes)", 0, 0);
> >
> > if (IS_ERR(ai->tfm)) {
> > airo_print_err(ai->dev->name, "failed to load transform for AES");
> > @@ -1624,37 +1627,31 @@ static void MoveWindow(miccntx *context, u32 micSeq)
> >
> > /* mic accumulate */
> > #define MIC_ACCUM(val) \
> > - context->accum += (u64)(val) * context->coeff[coeff_position++];
> > -
> > -static unsigned char aes_counter[16];
> > + context->accum += (u64)(val) * be32_to_cpu(context->coeff[coeff_position++]);
>
> You could alternatively call be32_to_cpu_array() after the AES encryption.
> But this works too.
>
> >
> > /* expand the key to fill the MMH coefficient array */
> > static void emmh32_setseed(emmh32_context *context, u8 *pkey, int keylen,
> > - struct crypto_cipher *tfm)
> > + struct crypto_sync_skcipher *tfm)
> > {
> > /* take the keying material, expand if necessary, truncate at 16-bytes */
> > /* run through AES counter mode to generate context->coeff[] */
> >
> > - int i,j;
> > - u32 counter;
> > - u8 *cipher, plain[16];
> > -
> > - crypto_cipher_setkey(tfm, pkey, 16);
> > - counter = 0;
> > - for (i = 0; i < ARRAY_SIZE(context->coeff); ) {
> > - aes_counter[15] = (u8)(counter >> 0);
> > - aes_counter[14] = (u8)(counter >> 8);
> > - aes_counter[13] = (u8)(counter >> 16);
> > - aes_counter[12] = (u8)(counter >> 24);
> > - counter++;
> > - memcpy (plain, aes_counter, 16);
> > - crypto_cipher_encrypt_one(tfm, plain, plain);
> > - cipher = plain;
> > - for (j = 0; (j < 16) && (i < ARRAY_SIZE(context->coeff)); ) {
> > - context->coeff[i++] = ntohl(*(__be32 *)&cipher[j]);
> > - j += 4;
> > - }
> > - }
> > + SYNC_SKCIPHER_REQUEST_ON_STACK(req, tfm);
> > + struct scatterlist dst, src;
> > + u8 iv[AES_BLOCK_SIZE] = {};
> > + int ret;
> > +
> > + crypto_sync_skcipher_setkey(tfm, pkey, 16);
> > +
> > + sg_init_one(&dst, context->coeff, sizeof(context->coeff));
> > + sg_init_one(&src, page_address(ZERO_PAGE(0)), sizeof(context->coeff));
>
> Should add:
>
> BUILD_BUG_ON(sizeof(context->coeff) > PAGE_SIZE);
>
> Or alternatively, instead of using ZERO_PAGE, just memset() the buffer to zero
> and encrypt it in-place. That would be less fragile.
>
I agree, I will change that.
> > +
> > + skcipher_request_set_sync_tfm(req, tfm);
> > + skcipher_request_set_callback(req, 0, NULL, NULL);
> > + skcipher_request_set_crypt(req, &src, &dst, sizeof(context->coeff), iv);
> > +
> > + ret = crypto_skcipher_encrypt(req);
> > + WARN_ON_ONCE(ret);
> > }
> >
> > /* prepare for calculation of a new mic */
> > @@ -2415,7 +2412,7 @@ void stop_airo_card( struct net_device *dev, int freeres )
> > ai->shared, ai->shared_dma);
> > }
> > }
> > - crypto_free_cipher(ai->tfm);
> > + crypto_free_sync_skcipher(ai->tfm);
> > del_airo_dev(ai);
> > free_netdev( dev );
> > }
> > --
> > 2.20.1
> >
>
> Otherwise this patch looks correct to me.
>
> The actual crypto in this driver, on the other hand, looks very outdated and
> broken. Apparently it's implementing some Cisco proprietary extension to WEP
> that uses a universal hashing based MAC, where the hash key is generated from
> AES-CTR. But the MAC is only 32 bits, and the universal hash (MMH) is
> implemented incorrectly: there's an off-by-one error in emmh32_final() in the
> code that is supposed to be an optimized version of 'sum % ((1ULL << 32) + 15)'.
>
I stared at that code for a bit, and I don't see the problem.
> Do we know whether anyone is actually using this, or is this just another old
> driver that's sitting around unused?
>
Excellent question. I take it this is pre-802.11b hardware, and so
even the OpenWRT people are unlikely to still be using this.
^ permalink raw reply
* Re: [PATCH] lib80211: use crypto API ccm(aes) transform for CCMP processing
From: Eric Biggers @ 2019-06-16 19:01 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-wireless, johannes, linux-crypto, herbert
In-Reply-To: <20190614092922.22517-1-ard.biesheuvel@linaro.org>
Hi Ard,
On Fri, Jun 14, 2019 at 11:29:22AM +0200, Ard Biesheuvel wrote:
> -static void ccmp_init_blocks(struct crypto_cipher *tfm,
> - struct ieee80211_hdr *hdr,
> - u8 * pn, size_t dlen, u8 * b0, u8 * auth, u8 * s0)
> +static void ccmp_init_blocks(struct ieee80211_hdr *hdr,
> + u8 * pn, size_t dlen, u8 * b0, u8 * aad)
> {
> u8 *pos, qc = 0;
> size_t aad_len;
> int a4_included, qc_included;
> - u8 aad[2 * AES_BLOCK_LEN];
>
> a4_included = ieee80211_has_a4(hdr->frame_control);
> qc_included = ieee80211_is_data_qos(hdr->frame_control);
> @@ -131,17 +123,19 @@ static void ccmp_init_blocks(struct crypto_cipher *tfm,
> aad_len += 2;
> }
>
> - /* CCM Initial Block:
> - * Flag (Include authentication header, M=3 (8-octet MIC),
> - * L=1 (2-octet Dlen))
> - * Nonce: 0x00 | A2 | PN
> - * Dlen */
> - b0[0] = 0x59;
> + /* In CCM, the initial vectors (IV) used for CTR mode encryption and CBC
> + * mode authentication are not allowed to collide, yet both are derived
> + * from this vector b0. We only set L := 1 here to indicate that the
> + * data size can be represented in (L+1) bytes. The CCM layer will take
> + * care of storing the data length in the top (L+1) bytes and setting
> + * and clearing the other bits as is required to derive the two IVs.
> + */
> + b0[0] = 0x1;
> +
> + /* Nonce: QC | A2 | PN */
> b0[1] = qc;
> memcpy(b0 + 2, hdr->addr2, ETH_ALEN);
> memcpy(b0 + 8, pn, CCMP_PN_LEN);
> - b0[14] = (dlen >> 8) & 0xff;
> - b0[15] = dlen & 0xff;
>
> /* AAD:
> * FC with bits 4..6 and 11..13 masked to zero; 14 is always one
> @@ -166,16 +160,6 @@ static void ccmp_init_blocks(struct crypto_cipher *tfm,
> aad[a4_included ? 30 : 24] = qc;
> /* rest of QC masked */
> }
> -
> - /* Start with the first block and AAD */
> - lib80211_ccmp_aes_encrypt(tfm, b0, auth);
> - xor_block(auth, aad, AES_BLOCK_LEN);
> - lib80211_ccmp_aes_encrypt(tfm, auth, auth);
> - xor_block(auth, &aad[AES_BLOCK_LEN], AES_BLOCK_LEN);
> - lib80211_ccmp_aes_encrypt(tfm, auth, auth);
> - b0[0] &= 0x07;
> - b0[14] = b0[15] = 0;
> - lib80211_ccmp_aes_encrypt(tfm, b0, s0);
> }
How about shifting the contents of aad over by 2 bytes and returning the AAD
length from this function instead? It's confusing to still manually format the
AAD length for CCM mode, when actually it's ignored now.
Also I suggest fixing up the naming:
ccmp_init_blocks() => ccmp_init_iv_and_aad()
b0 => iv
>
> static int lib80211_ccmp_hdr(struct sk_buff *skb, int hdr_len,
> @@ -218,13 +202,13 @@ static int lib80211_ccmp_hdr(struct sk_buff *skb, int hdr_len,
> static int lib80211_ccmp_encrypt(struct sk_buff *skb, int hdr_len, void *priv)
> {
> struct lib80211_ccmp_data *key = priv;
> - int data_len, i, blocks, last, len;
> - u8 *pos, *mic;
> struct ieee80211_hdr *hdr;
> - u8 *b0 = key->tx_b0;
> - u8 *b = key->tx_b;
> - u8 *e = key->tx_e;
> - u8 *s0 = key->tx_s0;
> + struct aead_request *req;
> + struct scatterlist sg[2];
> + u8 *aad = key->tx_aad;
> + u8 b0[AES_BLOCK_LEN];
> + int len, data_len;
> + int ret;
>
> if (skb_tailroom(skb) < CCMP_MIC_LEN || skb->len < hdr_len)
> return -1;
> @@ -234,31 +218,29 @@ static int lib80211_ccmp_encrypt(struct sk_buff *skb, int hdr_len, void *priv)
> if (len < 0)
> return -1;
>
> - pos = skb->data + hdr_len + CCMP_HDR_LEN;
> + req = kzalloc(sizeof(*req) + crypto_aead_reqsize(key->tfm), GFP_ATOMIC);
> + if (!req)
> + return -ENOMEM;
> +
Why not kzalloc() and kzfree() instead of aead_request_alloc() and
aead_request_free()? Same in lib80211_ccmp_decrypt().
Otherwise this patch looks good, though I'd like for someone to test it.
Thanks for doing this!
- Eric
^ permalink raw reply
* [PATCH 10/11] wil6210: set WIL_WMI_CALL_GENERAL_TO_MS as wmi_call timeout
From: Maya Erez @ 2019-06-16 7:26 UTC (permalink / raw)
To: Kalle Valo; +Cc: Ahmad Masri, linux-wireless, wil6210, Maya Erez
In-Reply-To: <1560669967-23706-1-git-send-email-merez@codeaurora.org>
From: Ahmad Masri <amasri@codeaurora.org>
Replace all wmi_call timeouts that are less than 100 msec to use
WIL_WMI_CALL_GENERAL_TO_MS (100 msec) as a default. Some of the
current wmi_call timeouts are too short and fails to receive its
waiting events.
Signed-off-by: Ahmad Masri <amasri@codeaurora.org>
Signed-off-by: Maya Erez <merez@codeaurora.org>
---
drivers/net/wireless/ath/wil6210/cfg80211.c | 3 ++-
drivers/net/wireless/ath/wil6210/debugfs.c | 2 +-
drivers/net/wireless/ath/wil6210/txrx.c | 9 +++++---
drivers/net/wireless/ath/wil6210/wil6210.h | 1 +
drivers/net/wireless/ath/wil6210/wmi.c | 32 +++++++++++++++++------------
5 files changed, 29 insertions(+), 18 deletions(-)
diff --git a/drivers/net/wireless/ath/wil6210/cfg80211.c b/drivers/net/wireless/ath/wil6210/cfg80211.c
index cd41a0b..52ae3d9 100644
--- a/drivers/net/wireless/ath/wil6210/cfg80211.c
+++ b/drivers/net/wireless/ath/wil6210/cfg80211.c
@@ -314,7 +314,8 @@ int wil_cid_fill_sinfo(struct wil6210_vif *vif, int cid,
memset(&reply, 0, sizeof(reply));
rc = wmi_call(wil, WMI_NOTIFY_REQ_CMDID, vif->mid, &cmd, sizeof(cmd),
- WMI_NOTIFY_REQ_DONE_EVENTID, &reply, sizeof(reply), 20);
+ WMI_NOTIFY_REQ_DONE_EVENTID, &reply, sizeof(reply),
+ WIL_WMI_CALL_GENERAL_TO_MS);
if (rc)
return rc;
diff --git a/drivers/net/wireless/ath/wil6210/debugfs.c b/drivers/net/wireless/ath/wil6210/debugfs.c
index bbe274c..4d0c867 100644
--- a/drivers/net/wireless/ath/wil6210/debugfs.c
+++ b/drivers/net/wireless/ath/wil6210/debugfs.c
@@ -1334,7 +1334,7 @@ static int bf_show(struct seq_file *s, void *data)
rc = wmi_call(wil, WMI_NOTIFY_REQ_CMDID, vif->mid,
&cmd, sizeof(cmd),
WMI_NOTIFY_REQ_DONE_EVENTID, &reply,
- sizeof(reply), 20);
+ sizeof(reply), WIL_WMI_CALL_GENERAL_TO_MS);
/* if reply is all-0, ignore this CID */
if (rc || is_all_zeros(&reply.evt, sizeof(reply.evt)))
continue;
diff --git a/drivers/net/wireless/ath/wil6210/txrx.c b/drivers/net/wireless/ath/wil6210/txrx.c
index 8790e5e..eae00aa 100644
--- a/drivers/net/wireless/ath/wil6210/txrx.c
+++ b/drivers/net/wireless/ath/wil6210/txrx.c
@@ -1037,7 +1037,8 @@ static int wil_vring_init_tx(struct wil6210_vif *vif, int id, int size,
if (!vif->privacy)
txdata->dot1x_open = true;
rc = wmi_call(wil, WMI_VRING_CFG_CMDID, vif->mid, &cmd, sizeof(cmd),
- WMI_VRING_CFG_DONE_EVENTID, &reply, sizeof(reply), 100);
+ WMI_VRING_CFG_DONE_EVENTID, &reply, sizeof(reply),
+ WIL_WMI_CALL_GENERAL_TO_MS);
if (rc)
goto out_free;
@@ -1125,7 +1126,8 @@ static int wil_tx_vring_modify(struct wil6210_vif *vif, int ring_id, int cid,
cmd.vring_cfg.tx_sw_ring.ring_mem_base = cpu_to_le64(vring->pa);
rc = wmi_call(wil, WMI_VRING_CFG_CMDID, vif->mid, &cmd, sizeof(cmd),
- WMI_VRING_CFG_DONE_EVENTID, &reply, sizeof(reply), 100);
+ WMI_VRING_CFG_DONE_EVENTID, &reply, sizeof(reply),
+ WIL_WMI_CALL_GENERAL_TO_MS);
if (rc)
goto fail;
@@ -1205,7 +1207,8 @@ int wil_vring_init_bcast(struct wil6210_vif *vif, int id, int size)
txdata->dot1x_open = true;
rc = wmi_call(wil, WMI_BCAST_VRING_CFG_CMDID, vif->mid,
&cmd, sizeof(cmd),
- WMI_VRING_CFG_DONE_EVENTID, &reply, sizeof(reply), 100);
+ WMI_VRING_CFG_DONE_EVENTID, &reply, sizeof(reply),
+ WIL_WMI_CALL_GENERAL_TO_MS);
if (rc)
goto out_free;
diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
index 038329b..6f456b3 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -99,6 +99,7 @@ static inline u32 WIL_GET_BITS(u32 x, int b0, int b1)
#define WIL_MAX_AMPDU_SIZE_128 (128 * 1024) /* FW/HW limit */
#define WIL_MAX_AGG_WSIZE_64 (64) /* FW/HW limit */
#define WIL6210_MAX_STATUS_RINGS (8)
+#define WIL_WMI_CALL_GENERAL_TO_MS 100
/* Hardware offload block adds the following:
* 26 bytes - 3-address QoS data header
diff --git a/drivers/net/wireless/ath/wil6210/wmi.c b/drivers/net/wireless/ath/wil6210/wmi.c
index 5d7eb52..542ef15 100644
--- a/drivers/net/wireless/ath/wil6210/wmi.c
+++ b/drivers/net/wireless/ath/wil6210/wmi.c
@@ -40,7 +40,6 @@
" 60G device led enablement. Set the led ID (0-2) to enable");
#define WIL_WAIT_FOR_SUSPEND_RESUME_COMP 200
-#define WIL_WMI_CALL_GENERAL_TO_MS 100
#define WIL_WMI_PCP_STOP_TO_MS 5000
/**
@@ -2059,7 +2058,8 @@ int wmi_echo(struct wil6210_priv *wil)
};
return wmi_call(wil, WMI_ECHO_CMDID, vif->mid, &cmd, sizeof(cmd),
- WMI_ECHO_RSP_EVENTID, NULL, 0, 50);
+ WMI_ECHO_RSP_EVENTID, NULL, 0,
+ WIL_WMI_CALL_GENERAL_TO_MS);
}
int wmi_set_mac_address(struct wil6210_priv *wil, void *addr)
@@ -2118,7 +2118,7 @@ int wmi_led_cfg(struct wil6210_priv *wil, bool enable)
rc = wmi_call(wil, WMI_LED_CFG_CMDID, vif->mid, &cmd, sizeof(cmd),
WMI_LED_CFG_DONE_EVENTID, &reply, sizeof(reply),
- 100);
+ WIL_WMI_CALL_GENERAL_TO_MS);
if (rc)
goto out;
@@ -2267,7 +2267,8 @@ int wmi_get_ssid(struct wil6210_vif *vif, u8 *ssid_len, void *ssid)
memset(&reply, 0, sizeof(reply));
rc = wmi_call(wil, WMI_GET_SSID_CMDID, vif->mid, NULL, 0,
- WMI_GET_SSID_EVENTID, &reply, sizeof(reply), 20);
+ WMI_GET_SSID_EVENTID, &reply, sizeof(reply),
+ WIL_WMI_CALL_GENERAL_TO_MS);
if (rc)
return rc;
@@ -2304,7 +2305,8 @@ int wmi_get_channel(struct wil6210_priv *wil, int *channel)
memset(&reply, 0, sizeof(reply));
rc = wmi_call(wil, WMI_GET_PCP_CHANNEL_CMDID, vif->mid, NULL, 0,
- WMI_GET_PCP_CHANNEL_EVENTID, &reply, sizeof(reply), 20);
+ WMI_GET_PCP_CHANNEL_EVENTID, &reply, sizeof(reply),
+ WIL_WMI_CALL_GENERAL_TO_MS);
if (rc)
return rc;
@@ -2400,7 +2402,8 @@ int wmi_stop_discovery(struct wil6210_vif *vif)
wil_dbg_wmi(wil, "sending WMI_DISCOVERY_STOP_CMDID\n");
rc = wmi_call(wil, WMI_DISCOVERY_STOP_CMDID, vif->mid, NULL, 0,
- WMI_DISCOVERY_STOPPED_EVENTID, NULL, 0, 100);
+ WMI_DISCOVERY_STOPPED_EVENTID, NULL, 0,
+ WIL_WMI_CALL_GENERAL_TO_MS);
if (rc)
wil_err(wil, "Failed to stop discovery\n");
@@ -2546,12 +2549,14 @@ int wmi_rxon(struct wil6210_priv *wil, bool on)
if (on) {
rc = wmi_call(wil, WMI_START_LISTEN_CMDID, vif->mid, NULL, 0,
WMI_LISTEN_STARTED_EVENTID,
- &reply, sizeof(reply), 100);
+ &reply, sizeof(reply),
+ WIL_WMI_CALL_GENERAL_TO_MS);
if ((rc == 0) && (reply.evt.status != WMI_FW_STATUS_SUCCESS))
rc = -EINVAL;
} else {
rc = wmi_call(wil, WMI_DISCOVERY_STOP_CMDID, vif->mid, NULL, 0,
- WMI_DISCOVERY_STOPPED_EVENTID, NULL, 0, 20);
+ WMI_DISCOVERY_STOPPED_EVENTID, NULL, 0,
+ WIL_WMI_CALL_GENERAL_TO_MS);
}
return rc;
@@ -2640,7 +2645,8 @@ int wmi_get_temperature(struct wil6210_priv *wil, u32 *t_bb, u32 *t_rf)
memset(&reply, 0, sizeof(reply));
rc = wmi_call(wil, WMI_TEMP_SENSE_CMDID, vif->mid, &cmd, sizeof(cmd),
- WMI_TEMP_SENSE_DONE_EVENTID, &reply, sizeof(reply), 100);
+ WMI_TEMP_SENSE_DONE_EVENTID, &reply, sizeof(reply),
+ WIL_WMI_CALL_GENERAL_TO_MS);
if (rc)
return rc;
@@ -2822,7 +2828,7 @@ int wmi_addba_rx_resp(struct wil6210_priv *wil,
rc = wmi_call(wil, WMI_RCP_ADDBA_RESP_CMDID, mid, &cmd, sizeof(cmd),
WMI_RCP_ADDBA_RESP_SENT_EVENTID, &reply, sizeof(reply),
- 100);
+ WIL_WMI_CALL_GENERAL_TO_MS);
if (rc)
return rc;
@@ -2904,7 +2910,7 @@ int wmi_ps_dev_profile_cfg(struct wil6210_priv *wil,
rc = wmi_call(wil, WMI_PS_DEV_PROFILE_CFG_CMDID, vif->mid,
&cmd, sizeof(cmd),
WMI_PS_DEV_PROFILE_CFG_EVENTID, &reply, sizeof(reply),
- 100);
+ WIL_WMI_CALL_GENERAL_TO_MS);
if (rc)
return rc;
@@ -2941,7 +2947,7 @@ int wmi_set_mgmt_retry(struct wil6210_priv *wil, u8 retry_short)
rc = wmi_call(wil, WMI_SET_MGMT_RETRY_LIMIT_CMDID, vif->mid,
&cmd, sizeof(cmd),
WMI_SET_MGMT_RETRY_LIMIT_EVENTID, &reply, sizeof(reply),
- 100);
+ WIL_WMI_CALL_GENERAL_TO_MS);
if (rc)
return rc;
@@ -2971,7 +2977,7 @@ int wmi_get_mgmt_retry(struct wil6210_priv *wil, u8 *retry_short)
memset(&reply, 0, sizeof(reply));
rc = wmi_call(wil, WMI_GET_MGMT_RETRY_LIMIT_CMDID, vif->mid, NULL, 0,
WMI_GET_MGMT_RETRY_LIMIT_EVENTID, &reply, sizeof(reply),
- 100);
+ WIL_WMI_CALL_GENERAL_TO_MS);
if (rc)
return rc;
--
1.9.1
^ permalink raw reply related
* [PATCH 05/11] wil6210: fix printout in wil_read_pmccfg
From: Maya Erez @ 2019-06-16 7:26 UTC (permalink / raw)
To: Kalle Valo; +Cc: Dedy Lansky, linux-wireless, wil6210, Maya Erez
In-Reply-To: <1560669967-23706-1-git-send-email-merez@codeaurora.org>
From: Dedy Lansky <dlansky@codeaurora.org>
Replace sprintf with snprintf which checks the destination buffer
size.
Signed-off-by: Dedy Lansky <dlansky@codeaurora.org>
Signed-off-by: Maya Erez <merez@codeaurora.org>
---
drivers/net/wireless/ath/wil6210/debugfs.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/ath/wil6210/debugfs.c b/drivers/net/wireless/ath/wil6210/debugfs.c
index 14778a1c..8eb6d49 100644
--- a/drivers/net/wireless/ath/wil6210/debugfs.c
+++ b/drivers/net/wireless/ath/wil6210/debugfs.c
@@ -976,9 +976,8 @@ static ssize_t wil_read_pmccfg(struct file *file, char __user *user_buf,
" - \"alloc <num descriptors> <descriptor_size>\" to allocate pmc\n"
" - \"free\" to free memory allocated for pmc\n";
- sprintf(text, "Last command status: %d\n\n%s",
- wil_pmc_last_cmd_status(wil),
- help);
+ snprintf(text, sizeof(text), "Last command status: %d\n\n%s",
+ wil_pmc_last_cmd_status(wil), help);
return simple_read_from_buffer(user_buf, count, ppos, text,
strlen(text) + 1);
--
1.9.1
^ permalink raw reply related
* [PATCH 09/11] wil6210: add support for reading multiple RFs temperature via debugfs
From: Maya Erez @ 2019-06-16 7:26 UTC (permalink / raw)
To: Kalle Valo; +Cc: Tzahi Sabo, linux-wireless, wil6210, Maya Erez
In-Reply-To: <1560669967-23706-1-git-send-email-merez@codeaurora.org>
From: Tzahi Sabo <stzahi@codeaurora.org>
Base-band chips support multi RFs chips. Add support for reading
multiple RFs temperature via debugfs.
Signed-off-by: Tzahi Sabo <stzahi@codeaurora.org>
Signed-off-by: Maya Erez <merez@codeaurora.org>
---
drivers/net/wireless/ath/wil6210/debugfs.c | 42 ++++++++++++++++++++------
drivers/net/wireless/ath/wil6210/wil6210.h | 3 ++
drivers/net/wireless/ath/wil6210/wmi.c | 42 ++++++++++++++++++++++++++
drivers/net/wireless/ath/wil6210/wmi.h | 47 +++++++++++++++++++++++++-----
4 files changed, 117 insertions(+), 17 deletions(-)
diff --git a/drivers/net/wireless/ath/wil6210/debugfs.c b/drivers/net/wireless/ath/wil6210/debugfs.c
index 8eb6d49..bbe274c 100644
--- a/drivers/net/wireless/ath/wil6210/debugfs.c
+++ b/drivers/net/wireless/ath/wil6210/debugfs.c
@@ -1372,7 +1372,7 @@ static void print_temp(struct seq_file *s, const char *prefix, s32 t)
{
switch (t) {
case 0:
- case ~(u32)0:
+ case WMI_INVALID_TEMPERATURE:
seq_printf(s, "%s N/A\n", prefix);
break;
default:
@@ -1385,17 +1385,41 @@ static void print_temp(struct seq_file *s, const char *prefix, s32 t)
static int temp_show(struct seq_file *s, void *data)
{
struct wil6210_priv *wil = s->private;
- s32 t_m, t_r;
- int rc = wmi_get_temperature(wil, &t_m, &t_r);
+ int rc, i;
- if (rc) {
- seq_puts(s, "Failed\n");
- return 0;
- }
+ if (test_bit(WMI_FW_CAPABILITY_TEMPERATURE_ALL_RF,
+ wil->fw_capabilities)) {
+ struct wmi_temp_sense_all_done_event sense_all_evt;
- print_temp(s, "T_mac =", t_m);
- print_temp(s, "T_radio =", t_r);
+ wil_dbg_misc(wil,
+ "WMI_FW_CAPABILITY_TEMPERATURE_ALL_RF is supported");
+ rc = wmi_get_all_temperatures(wil, &sense_all_evt);
+ if (rc) {
+ seq_puts(s, "Failed\n");
+ return 0;
+ }
+ print_temp(s, "T_mac =",
+ le32_to_cpu(sense_all_evt.baseband_t1000));
+ seq_printf(s, "Connected RFs [0x%08x]\n",
+ sense_all_evt.rf_bitmap);
+ for (i = 0; i < WMI_MAX_XIF_PORTS_NUM; i++) {
+ seq_printf(s, "RF[%d] = ", i);
+ print_temp(s, "",
+ le32_to_cpu(sense_all_evt.rf_t1000[i]));
+ }
+ } else {
+ s32 t_m, t_r;
+ wil_dbg_misc(wil,
+ "WMI_FW_CAPABILITY_TEMPERATURE_ALL_RF is not supported");
+ rc = wmi_get_temperature(wil, &t_m, &t_r);
+ if (rc) {
+ seq_puts(s, "Failed\n");
+ return 0;
+ }
+ print_temp(s, "T_mac =", t_m);
+ print_temp(s, "T_radio =", t_r);
+ }
return 0;
}
DEFINE_SHOW_ATTRIBUTE(temp);
diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
index afbc524..038329b 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -1252,6 +1252,9 @@ int wmi_add_cipher_key(struct wil6210_vif *vif, u8 key_index,
int wmi_update_ft_ies(struct wil6210_vif *vif, u16 ie_len, const void *ie);
int wmi_rxon(struct wil6210_priv *wil, bool on);
int wmi_get_temperature(struct wil6210_priv *wil, u32 *t_m, u32 *t_r);
+int wmi_get_all_temperatures(struct wil6210_priv *wil,
+ struct wmi_temp_sense_all_done_event
+ *sense_all_evt);
int wmi_disconnect_sta(struct wil6210_vif *vif, const u8 *mac, u16 reason,
bool del_sta);
int wmi_addba(struct wil6210_priv *wil, u8 mid,
diff --git a/drivers/net/wireless/ath/wil6210/wmi.c b/drivers/net/wireless/ath/wil6210/wmi.c
index cacafab..5d7eb52 100644
--- a/drivers/net/wireless/ath/wil6210/wmi.c
+++ b/drivers/net/wireless/ath/wil6210/wmi.c
@@ -486,6 +486,8 @@ static const char *cmdid2name(u16 cmdid)
return "WMI_UPDATE_FT_IES_CMD";
case WMI_RBUFCAP_CFG_CMDID:
return "WMI_RBUFCAP_CFG_CMD";
+ case WMI_TEMP_SENSE_ALL_CMDID:
+ return "WMI_TEMP_SENSE_ALL_CMDID";
default:
return "Untracked CMD";
}
@@ -632,6 +634,8 @@ static const char *eventid2name(u16 eventid)
return "WMI_FT_REASSOC_STATUS_EVENT";
case WMI_RBUFCAP_CFG_EVENTID:
return "WMI_RBUFCAP_CFG_EVENT";
+ case WMI_TEMP_SENSE_ALL_DONE_EVENTID:
+ return "WMI_TEMP_SENSE_ALL_DONE_EVENTID";
default:
return "Untracked EVENT";
}
@@ -2648,6 +2652,44 @@ int wmi_get_temperature(struct wil6210_priv *wil, u32 *t_bb, u32 *t_rf)
return 0;
}
+int wmi_get_all_temperatures(struct wil6210_priv *wil,
+ struct wmi_temp_sense_all_done_event
+ *sense_all_evt)
+{
+ struct wil6210_vif *vif = ndev_to_vif(wil->main_ndev);
+ int rc;
+ struct wmi_temp_sense_all_cmd cmd = {
+ .measure_baseband_en = true,
+ .measure_rf_en = true,
+ .measure_mode = TEMPERATURE_MEASURE_NOW,
+ };
+ struct {
+ struct wmi_cmd_hdr wmi;
+ struct wmi_temp_sense_all_done_event evt;
+ } __packed reply;
+
+ if (!sense_all_evt) {
+ wil_err(wil, "Invalid sense_all_evt value\n");
+ return -EINVAL;
+ }
+
+ memset(&reply, 0, sizeof(reply));
+ reply.evt.status = WMI_FW_STATUS_FAILURE;
+ rc = wmi_call(wil, WMI_TEMP_SENSE_ALL_CMDID, vif->mid, &cmd,
+ sizeof(cmd), WMI_TEMP_SENSE_ALL_DONE_EVENTID,
+ &reply, sizeof(reply), WIL_WMI_CALL_GENERAL_TO_MS);
+ if (rc)
+ return rc;
+
+ if (reply.evt.status == WMI_FW_STATUS_FAILURE) {
+ wil_err(wil, "Failed geting TEMP_SENSE_ALL\n");
+ return -EINVAL;
+ }
+
+ memcpy(sense_all_evt, &reply.evt, sizeof(reply.evt));
+ return 0;
+}
+
int wmi_disconnect_sta(struct wil6210_vif *vif, const u8 *mac, u16 reason,
bool del_sta)
{
diff --git a/drivers/net/wireless/ath/wil6210/wmi.h b/drivers/net/wireless/ath/wil6210/wmi.h
index da46fc8..3e37229 100644
--- a/drivers/net/wireless/ath/wil6210/wmi.h
+++ b/drivers/net/wireless/ath/wil6210/wmi.h
@@ -35,6 +35,7 @@
#define WMI_PROX_RANGE_NUM (3)
#define WMI_MAX_LOSS_DMG_BEACONS (20)
#define MAX_NUM_OF_SECTORS (128)
+#define WMI_INVALID_TEMPERATURE (0xFFFFFFFF)
#define WMI_SCHED_MAX_ALLOCS_PER_CMD (4)
#define WMI_RF_DTYPE_LENGTH (3)
#define WMI_RF_ETYPE_LENGTH (3)
@@ -64,6 +65,7 @@
#define WMI_QOS_MAX_WEIGHT 50
#define WMI_QOS_SET_VIF_PRIORITY (0xFF)
#define WMI_QOS_DEFAULT_PRIORITY (WMI_QOS_NUM_OF_PRIORITY)
+#define WMI_MAX_XIF_PORTS_NUM (8)
/* Mailbox interface
* used for commands and events
@@ -105,6 +107,7 @@ enum wmi_fw_capability {
WMI_FW_CAPABILITY_TX_REQ_EXT = 25,
WMI_FW_CAPABILITY_CHANNEL_4 = 26,
WMI_FW_CAPABILITY_IPA = 27,
+ WMI_FW_CAPABILITY_TEMPERATURE_ALL_RF = 30,
WMI_FW_CAPABILITY_MAX,
};
@@ -296,6 +299,7 @@ enum wmi_command_id {
WMI_SET_VRING_PRIORITY_WEIGHT_CMDID = 0xA10,
WMI_SET_VRING_PRIORITY_CMDID = 0xA11,
WMI_RBUFCAP_CFG_CMDID = 0xA12,
+ WMI_TEMP_SENSE_ALL_CMDID = 0xA13,
WMI_SET_MAC_ADDRESS_CMDID = 0xF003,
WMI_ABORT_SCAN_CMDID = 0xF007,
WMI_SET_PROMISCUOUS_MODE_CMDID = 0xF041,
@@ -1411,12 +1415,7 @@ struct wmi_rf_xpm_write_cmd {
u8 data_bytes[0];
} __packed;
-/* WMI_TEMP_SENSE_CMDID
- *
- * Measure MAC and radio temperatures
- *
- * Possible modes for temperature measurement
- */
+/* Possible modes for temperature measurement */
enum wmi_temperature_measure_mode {
TEMPERATURE_USE_OLD_VALUE = 0x01,
TEMPERATURE_MEASURE_NOW = 0x02,
@@ -1942,6 +1941,14 @@ struct wmi_set_ap_slot_size_cmd {
__le32 slot_size;
} __packed;
+/* WMI_TEMP_SENSE_ALL_CMDID */
+struct wmi_temp_sense_all_cmd {
+ u8 measure_baseband_en;
+ u8 measure_rf_en;
+ u8 measure_mode;
+ u8 reserved;
+} __packed;
+
/* WMI Events
* List of Events (target to host)
*/
@@ -2101,6 +2108,7 @@ enum wmi_event_id {
WMI_SET_VRING_PRIORITY_WEIGHT_EVENTID = 0x1A10,
WMI_SET_VRING_PRIORITY_EVENTID = 0x1A11,
WMI_RBUFCAP_CFG_EVENTID = 0x1A12,
+ WMI_TEMP_SENSE_ALL_DONE_EVENTID = 0x1A13,
WMI_SET_CHANNEL_EVENTID = 0x9000,
WMI_ASSOC_REQ_EVENTID = 0x9001,
WMI_EAPOL_RX_EVENTID = 0x9002,
@@ -2784,11 +2792,13 @@ struct wmi_fixed_scheduling_ul_config_event {
*/
struct wmi_temp_sense_done_event {
/* Temperature times 1000 (actual temperature will be achieved by
- * dividing the value by 1000)
+ * dividing the value by 1000). When temperature cannot be read from
+ * device return WMI_INVALID_TEMPERATURE
*/
__le32 baseband_t1000;
/* Temperature times 1000 (actual temperature will be achieved by
- * dividing the value by 1000)
+ * dividing the value by 1000). When temperature cannot be read from
+ * device return WMI_INVALID_TEMPERATURE
*/
__le32 rf_t1000;
} __packed;
@@ -4140,4 +4150,25 @@ struct wmi_rbufcap_cfg_event {
u8 reserved[3];
} __packed;
+/* WMI_TEMP_SENSE_ALL_DONE_EVENTID
+ * Measure MAC and all radio temperatures
+ */
+struct wmi_temp_sense_all_done_event {
+ /* enum wmi_fw_status */
+ u8 status;
+ /* Bitmap of connected RFs */
+ u8 rf_bitmap;
+ u8 reserved[2];
+ /* Temperature times 1000 (actual temperature will be achieved by
+ * dividing the value by 1000). When temperature cannot be read from
+ * device return WMI_INVALID_TEMPERATURE
+ */
+ __le32 rf_t1000[WMI_MAX_XIF_PORTS_NUM];
+ /* Temperature times 1000 (actual temperature will be achieved by
+ * dividing the value by 1000). When temperature cannot be read from
+ * device return WMI_INVALID_TEMPERATURE
+ */
+ __le32 baseband_t1000;
+} __packed;
+
#endif /* __WILOCITY_WMI_H__ */
--
1.9.1
^ permalink raw reply related
* [PATCH 06/11] wil6210: clear FW and ucode log address
From: Maya Erez @ 2019-06-16 7:26 UTC (permalink / raw)
To: Kalle Valo; +Cc: Maya Erez, linux-wireless, wil6210, Tzahi Sabo
In-Reply-To: <1560669967-23706-1-git-send-email-merez@codeaurora.org>
Clear the FW and ucode log address on device initialization to allow
user space app identify when the address was set by FW/ucode and it can
start read.
Signed-off-by: Tzahi Sabo <stzahi@codeaurora.org>
Signed-off-by: Maya Erez <merez@codeaurora.org>
---
drivers/net/wireless/ath/wil6210/main.c | 15 +++++++++++++++
drivers/net/wireless/ath/wil6210/pcie_bus.c | 1 +
drivers/net/wireless/ath/wil6210/wil6210.h | 1 +
3 files changed, 17 insertions(+)
diff --git a/drivers/net/wireless/ath/wil6210/main.c b/drivers/net/wireless/ath/wil6210/main.c
index 3c30076..f7b9e6b 100644
--- a/drivers/net/wireless/ath/wil6210/main.c
+++ b/drivers/net/wireless/ath/wil6210/main.c
@@ -1522,6 +1522,7 @@ int wil_ps_update(struct wil6210_priv *wil, enum wmi_ps_profile_type ps_profile)
static void wil_pre_fw_config(struct wil6210_priv *wil)
{
+ wil_clear_fw_log_addr(wil);
/* Mark FW as loaded from host */
wil_s(wil, RGF_USER_USAGE_6, 1);
@@ -1579,6 +1580,20 @@ static int wil_restore_vifs(struct wil6210_priv *wil)
}
/*
+ * Clear FW and ucode log start addr to indicate FW log is not ready. The host
+ * driver clears the addresses before FW starts and FW initializes the address
+ * when it is ready to send logs.
+ */
+void wil_clear_fw_log_addr(struct wil6210_priv *wil)
+{
+ /* FW log addr */
+ wil_w(wil, RGF_USER_USAGE_1, 0);
+ /* ucode log addr */
+ wil_w(wil, RGF_USER_USAGE_2, 0);
+ wil_dbg_misc(wil, "Cleared FW and ucode log address");
+}
+
+/*
* We reset all the structures, and we reset the UMAC.
* After calling this routine, you're expected to reload
* the firmware.
diff --git a/drivers/net/wireless/ath/wil6210/pcie_bus.c b/drivers/net/wireless/ath/wil6210/pcie_bus.c
index 4e2d922..9f5a914 100644
--- a/drivers/net/wireless/ath/wil6210/pcie_bus.c
+++ b/drivers/net/wireless/ath/wil6210/pcie_bus.c
@@ -420,6 +420,7 @@ static int wil_pcie_probe(struct pci_dev *pdev, const struct pci_device_id *id)
}
/* rollback to bus_disable */
+ wil_clear_fw_log_addr(wil);
rc = wil_if_add(wil);
if (rc) {
wil_err(wil, "wil_if_add failed: %d\n", rc);
diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
index 75abec2..afbc524 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -1425,4 +1425,5 @@ int wmi_addba_rx_resp_edma(struct wil6210_priv *wil, u8 mid, u8 cid,
void update_supported_bands(struct wil6210_priv *wil);
+void wil_clear_fw_log_addr(struct wil6210_priv *wil);
#endif /* __WIL6210_H__ */
--
1.9.1
^ permalink raw reply related
* [PATCH 01/11] wil6210: do not reset FW in STA to P2P client interface switch
From: Maya Erez @ 2019-06-16 7:25 UTC (permalink / raw)
To: Kalle Valo; +Cc: Alexei Avshalom Lazar, linux-wireless, wil6210, Maya Erez
In-Reply-To: <1560669967-23706-1-git-send-email-merez@codeaurora.org>
From: Alexei Avshalom Lazar <ailizaro@codeaurora.org>
Currently the FW is reset on every interface type change, because
of various FW bugs.
FW reset is not required when switching from STA to P2P client, hence
can be skipped.
Signed-off-by: Alexei Avshalom Lazar <ailizaro@codeaurora.org>
Signed-off-by: Maya Erez <merez@codeaurora.org>
---
drivers/net/wireless/ath/wil6210/cfg80211.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/wil6210/cfg80211.c b/drivers/net/wireless/ath/wil6210/cfg80211.c
index e9780fc..1a4223f 100644
--- a/drivers/net/wireless/ath/wil6210/cfg80211.c
+++ b/drivers/net/wireless/ath/wil6210/cfg80211.c
@@ -643,6 +643,16 @@ static int wil_cfg80211_del_iface(struct wiphy *wiphy,
return rc;
}
+static bool wil_is_safe_switch(enum nl80211_iftype from,
+ enum nl80211_iftype to)
+{
+ if (from == NL80211_IFTYPE_STATION &&
+ to == NL80211_IFTYPE_P2P_CLIENT)
+ return true;
+
+ return false;
+}
+
static int wil_cfg80211_change_iface(struct wiphy *wiphy,
struct net_device *ndev,
enum nl80211_iftype type,
@@ -668,7 +678,8 @@ static int wil_cfg80211_change_iface(struct wiphy *wiphy,
* because it can cause significant disruption
*/
if (!wil_has_other_active_ifaces(wil, ndev, true, false) &&
- netif_running(ndev) && !wil_is_recovery_blocked(wil)) {
+ netif_running(ndev) && !wil_is_recovery_blocked(wil) &&
+ !wil_is_safe_switch(wdev->iftype, type)) {
wil_dbg_misc(wil, "interface is up. resetting...\n");
mutex_lock(&wil->mutex);
__wil_down(wil);
--
1.9.1
^ permalink raw reply related
* [PATCH 08/11] wil6210: publish max_msdu_size to FW on BCAST ring
From: Maya Erez @ 2019-06-16 7:26 UTC (permalink / raw)
To: Kalle Valo; +Cc: Maya Erez, linux-wireless, wil6210
In-Reply-To: <1560669967-23706-1-git-send-email-merez@codeaurora.org>
Set max_msdu_size in WMI_BCAST_DESC_RING_ADD_CMD to allow FW
to optimize the buffers allocation for bcast packets.
Signed-off-by: Maya Erez <merez@codeaurora.org>
---
drivers/net/wireless/ath/wil6210/wmi.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/drivers/net/wireless/ath/wil6210/wmi.c b/drivers/net/wireless/ath/wil6210/wmi.c
index 298c918..cacafab 100644
--- a/drivers/net/wireless/ath/wil6210/wmi.c
+++ b/drivers/net/wireless/ath/wil6210/wmi.c
@@ -3835,6 +3835,7 @@ int wil_wmi_bcast_desc_ring_add(struct wil6210_vif *vif, int ring_id)
.ring_size = cpu_to_le16(ring->size),
.ring_id = ring_id,
},
+ .max_msdu_size = cpu_to_le16(wil_mtu2macbuf(mtu_max)),
.status_ring_id = wil->tx_sring_idx,
.encap_trans_type = WMI_VRING_ENC_TYPE_802_3,
};
--
1.9.1
^ permalink raw reply related
* [PATCH 03/11] wil6210: increase the frequency of status ring hw tail update
From: Maya Erez @ 2019-06-16 7:25 UTC (permalink / raw)
To: Kalle Valo; +Cc: Ahmad Masri, linux-wireless, wil6210, Maya Erez
In-Reply-To: <1560669967-23706-1-git-send-email-merez@codeaurora.org>
From: Ahmad Masri <amasri@codeaurora.org>
The driver updates Tx status ring HW tail only after it finishes
processing the whole status ring, while the HW is still transmitting
from other transmit rings. This can cause back-pressure on HW if
no status entries are available.
Update HW tail of Tx status ring without waiting for the end of the
processing to help feeding back the HW with status entries and to allow
additional packet transmission.
Signed-off-by: Ahmad Masri <amasri@codeaurora.org>
Signed-off-by: Maya Erez <merez@codeaurora.org>
---
drivers/net/wireless/ath/wil6210/txrx_edma.c | 16 +++++++++++++---
1 file changed, 13 insertions(+), 3 deletions(-)
diff --git a/drivers/net/wireless/ath/wil6210/txrx_edma.c b/drivers/net/wireless/ath/wil6210/txrx_edma.c
index 6140db5..dc040cd 100644
--- a/drivers/net/wireless/ath/wil6210/txrx_edma.c
+++ b/drivers/net/wireless/ath/wil6210/txrx_edma.c
@@ -26,6 +26,10 @@
#include "txrx.h"
#include "trace.h"
+/* Max number of entries (packets to complete) to update the hwtail of tx
+ * status ring. Should be power of 2
+ */
+#define WIL_EDMA_TX_SRING_UPDATE_HW_TAIL 128
#define WIL_EDMA_MAX_DATA_OFFSET (2)
/* RX buffer size must be aligned to 4 bytes */
#define WIL_EDMA_RX_BUF_LEN_DEFAULT (2048)
@@ -1155,7 +1159,7 @@ int wil_tx_sring_handler(struct wil6210_priv *wil,
struct wil_net_stats *stats;
struct wil_tx_enhanced_desc *_d;
unsigned int ring_id;
- unsigned int num_descs;
+ unsigned int num_descs, num_statuses = 0;
int i;
u8 dr_bit; /* Descriptor Ready bit */
struct wil_ring_tx_status msg;
@@ -1276,6 +1280,11 @@ int wil_tx_sring_handler(struct wil6210_priv *wil,
}
again:
+ num_statuses++;
+ if (num_statuses % WIL_EDMA_TX_SRING_UPDATE_HW_TAIL == 0)
+ /* update HW tail to allow HW to push new statuses */
+ wil_w(wil, sring->hwtail, sring->swhead);
+
wil_sring_advance_swhead(sring);
wil_get_next_tx_status_msg(sring, &msg);
@@ -1286,8 +1295,9 @@ int wil_tx_sring_handler(struct wil6210_priv *wil,
if (desc_cnt)
wil_update_net_queues(wil, vif, NULL, false);
- /* Update the HW tail ptr (RD ptr) */
- wil_w(wil, sring->hwtail, (sring->swhead - 1) % sring->size);
+ if (num_statuses % WIL_EDMA_TX_SRING_UPDATE_HW_TAIL != 0)
+ /* Update the HW tail ptr (RD ptr) */
+ wil_w(wil, sring->hwtail, (sring->swhead - 1) % sring->size);
return desc_cnt;
}
--
1.9.1
^ permalink raw reply related
* [PATCH 07/11] wil6210: update cid boundary check of wil_find_cid/_by_idx()
From: Maya Erez @ 2019-06-16 7:26 UTC (permalink / raw)
To: Kalle Valo; +Cc: Alexei Avshalom Lazar, linux-wireless, wil6210, Maya Erez
In-Reply-To: <1560669967-23706-1-git-send-email-merez@codeaurora.org>
From: Alexei Avshalom Lazar <ailizaro@codeaurora.org>
The return value of wil_find_cid()/wil_find_cid_by_idx() is
validated with the lower boundary value.
Check the upper boundary value as well.
Signed-off-by: Alexei Avshalom Lazar <ailizaro@codeaurora.org>
Signed-off-by: Maya Erez <merez@codeaurora.org>
---
drivers/net/wireless/ath/wil6210/cfg80211.c | 6 +++---
drivers/net/wireless/ath/wil6210/main.c | 4 ++--
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/wireless/ath/wil6210/cfg80211.c b/drivers/net/wireless/ath/wil6210/cfg80211.c
index 1a4223f..cd41a0b 100644
--- a/drivers/net/wireless/ath/wil6210/cfg80211.c
+++ b/drivers/net/wireless/ath/wil6210/cfg80211.c
@@ -380,8 +380,8 @@ static int wil_cfg80211_get_station(struct wiphy *wiphy,
wil_dbg_misc(wil, "get_station: %pM CID %d MID %d\n", mac, cid,
vif->mid);
- if (cid < 0)
- return cid;
+ if (!wil_cid_valid(wil, cid))
+ return -ENOENT;
rc = wil_cid_fill_sinfo(vif, cid, sinfo);
@@ -417,7 +417,7 @@ static int wil_cfg80211_dump_station(struct wiphy *wiphy,
int rc;
int cid = wil_find_cid_by_idx(wil, vif->mid, idx);
- if (cid < 0)
+ if (!wil_cid_valid(wil, cid))
return -ENOENT;
ether_addr_copy(mac, wil->sta[cid].addr);
diff --git a/drivers/net/wireless/ath/wil6210/main.c b/drivers/net/wireless/ath/wil6210/main.c
index f7b9e6b..173561f 100644
--- a/drivers/net/wireless/ath/wil6210/main.c
+++ b/drivers/net/wireless/ath/wil6210/main.c
@@ -340,7 +340,7 @@ static void _wil6210_disconnect_complete(struct wil6210_vif *vif,
wil_dbg_misc(wil,
"Disconnect complete %pM, CID=%d, reason=%d\n",
bssid, cid, reason_code);
- if (cid >= 0) /* disconnect 1 peer */
+ if (wil_cid_valid(wil, cid)) /* disconnect 1 peer */
wil_disconnect_cid_complete(vif, cid, reason_code);
} else { /* all */
wil_dbg_misc(wil, "Disconnect complete all\n");
@@ -452,7 +452,7 @@ static void _wil6210_disconnect(struct wil6210_vif *vif, const u8 *bssid,
cid = wil_find_cid(wil, vif->mid, bssid);
wil_dbg_misc(wil, "Disconnect %pM, CID=%d, reason=%d\n",
bssid, cid, reason_code);
- if (cid >= 0) /* disconnect 1 peer */
+ if (wil_cid_valid(wil, cid)) /* disconnect 1 peer */
wil_disconnect_cid(vif, cid, reason_code);
} else { /* all */
wil_dbg_misc(wil, "Disconnect all\n");
--
1.9.1
^ permalink raw reply related
* [PATCH 04/11] wil6210: Add support for setting RBUFCAP configuration
From: Maya Erez @ 2019-06-16 7:26 UTC (permalink / raw)
To: Kalle Valo; +Cc: Alexei Avshalom Lazar, linux-wireless, wil6210, Maya Erez
In-Reply-To: <1560669967-23706-1-git-send-email-merez@codeaurora.org>
From: Alexei Avshalom Lazar <ailizaro@codeaurora.org>
RBUFCAP support added in FW.
The RBUFCAP feature is amendment to the block ack mechanism to
prevent overloading of the recipient’s memory space, which may
happen in case the link speed is higher than STA’s capability
to process or consume incoming data.
The block ack policy (ba_policy) is now controlled by FW so driver
should ignore this field.
Add new debugfs "rbufcap" to configure RBUFCAP.
Signed-off-by: Alexei Avshalom Lazar <ailizaro@codeaurora.org>
Signed-off-by: Maya Erez <merez@codeaurora.org>
---
drivers/net/wireless/ath/wil6210/debugfs.c | 39 +++++++++++++++++++++++++++
drivers/net/wireless/ath/wil6210/rx_reorder.c | 31 ++++++++-------------
drivers/net/wireless/ath/wil6210/wil6210.h | 1 +
drivers/net/wireless/ath/wil6210/wmi.c | 39 +++++++++++++++++++++++++--
4 files changed, 88 insertions(+), 22 deletions(-)
diff --git a/drivers/net/wireless/ath/wil6210/debugfs.c b/drivers/net/wireless/ath/wil6210/debugfs.c
index 7a2c3fd..14778a1c 100644
--- a/drivers/net/wireless/ath/wil6210/debugfs.c
+++ b/drivers/net/wireless/ath/wil6210/debugfs.c
@@ -777,6 +777,44 @@ static ssize_t wil_write_file_rxon(struct file *file, const char __user *buf,
.open = simple_open,
};
+static ssize_t wil_write_file_rbufcap(struct file *file,
+ const char __user *buf,
+ size_t count, loff_t *ppos)
+{
+ struct wil6210_priv *wil = file->private_data;
+ int val;
+ int rc;
+
+ rc = kstrtoint_from_user(buf, count, 0, &val);
+ if (rc) {
+ wil_err(wil, "Invalid argument\n");
+ return rc;
+ }
+ /* input value: negative to disable, 0 to use system default,
+ * 1..ring size to set descriptor threshold
+ */
+ wil_info(wil, "%s RBUFCAP, descriptors threshold - %d\n",
+ val < 0 ? "Disabling" : "Enabling", val);
+
+ if (!wil->ring_rx.va || val > wil->ring_rx.size) {
+ wil_err(wil, "Invalid descriptors threshold, %d\n", val);
+ return -EINVAL;
+ }
+
+ rc = wmi_rbufcap_cfg(wil, val < 0 ? 0 : 1, val < 0 ? 0 : val);
+ if (rc) {
+ wil_err(wil, "RBUFCAP config failed: %d\n", rc);
+ return rc;
+ }
+
+ return count;
+}
+
+static const struct file_operations fops_rbufcap = {
+ .write = wil_write_file_rbufcap,
+ .open = simple_open,
+};
+
/* block ack control, write:
* - "add <ringid> <agg_size> <timeout>" to trigger ADDBA
* - "del_tx <ringid> <reason>" to trigger DELBA for Tx side
@@ -2364,6 +2402,7 @@ static void wil6210_debugfs_init_blobs(struct wil6210_priv *wil,
{"tx_latency", 0644, &fops_tx_latency},
{"link_stats", 0644, &fops_link_stats},
{"link_stats_global", 0644, &fops_link_stats_global},
+ {"rbufcap", 0244, &fops_rbufcap},
};
static void wil6210_debugfs_init_files(struct wil6210_priv *wil,
diff --git a/drivers/net/wireless/ath/wil6210/rx_reorder.c b/drivers/net/wireless/ath/wil6210/rx_reorder.c
index 1c79664..784239b 100644
--- a/drivers/net/wireless/ath/wil6210/rx_reorder.c
+++ b/drivers/net/wireless/ath/wil6210/rx_reorder.c
@@ -316,7 +316,7 @@ int wil_addba_rx_request(struct wil6210_priv *wil, u8 mid, u8 cid, u8 tid,
u16 agg_timeout = le16_to_cpu(ba_timeout);
u16 seq_ctrl = le16_to_cpu(ba_seq_ctrl);
struct wil_sta_info *sta;
- u16 agg_wsize = 0;
+ u16 agg_wsize;
/* bit 0: A-MSDU supported
* bit 1: policy (should be 0 for us)
* bits 2..5: TID
@@ -328,7 +328,6 @@ int wil_addba_rx_request(struct wil6210_priv *wil, u8 mid, u8 cid, u8 tid,
test_bit(WMI_FW_CAPABILITY_AMSDU, wil->fw_capabilities) &&
wil->amsdu_en && (param_set & BIT(0));
int ba_policy = param_set & BIT(1);
- u16 status = WLAN_STATUS_SUCCESS;
u16 ssn = seq_ctrl >> 4;
struct wil_tid_ampdu_rx *r;
int rc = 0;
@@ -355,27 +354,19 @@ int wil_addba_rx_request(struct wil6210_priv *wil, u8 mid, u8 cid, u8 tid,
agg_amsdu ? "+" : "-", !!ba_policy, dialog_token, ssn);
/* apply policies */
- if (ba_policy) {
- wil_err(wil, "BACK requested unsupported ba_policy == 1\n");
- status = WLAN_STATUS_INVALID_QOS_PARAM;
- }
- if (status == WLAN_STATUS_SUCCESS) {
- if (req_agg_wsize == 0) {
- wil_dbg_misc(wil, "Suggest BACK wsize %d\n",
- wil->max_agg_wsize);
- agg_wsize = wil->max_agg_wsize;
- } else {
- agg_wsize = min_t(u16,
- wil->max_agg_wsize, req_agg_wsize);
- }
+ if (req_agg_wsize == 0) {
+ wil_dbg_misc(wil, "Suggest BACK wsize %d\n",
+ wil->max_agg_wsize);
+ agg_wsize = wil->max_agg_wsize;
+ } else {
+ agg_wsize = min_t(u16, wil->max_agg_wsize, req_agg_wsize);
}
rc = wil->txrx_ops.wmi_addba_rx_resp(wil, mid, cid, tid, dialog_token,
- status, agg_amsdu, agg_wsize,
- agg_timeout);
- if (rc || (status != WLAN_STATUS_SUCCESS)) {
- wil_err(wil, "do not apply ba, rc(%d), status(%d)\n", rc,
- status);
+ WLAN_STATUS_SUCCESS, agg_amsdu,
+ agg_wsize, agg_timeout);
+ if (rc) {
+ wil_err(wil, "do not apply ba, rc(%d)\n", rc);
goto out;
}
diff --git a/drivers/net/wireless/ath/wil6210/wil6210.h b/drivers/net/wireless/ath/wil6210/wil6210.h
index 4498403..75abec2 100644
--- a/drivers/net/wireless/ath/wil6210/wil6210.h
+++ b/drivers/net/wireless/ath/wil6210/wil6210.h
@@ -1406,6 +1406,7 @@ int wmi_start_sched_scan(struct wil6210_priv *wil,
int wmi_mgmt_tx(struct wil6210_vif *vif, const u8 *buf, size_t len);
int wmi_mgmt_tx_ext(struct wil6210_vif *vif, const u8 *buf, size_t len,
u8 channel, u16 duration_ms);
+int wmi_rbufcap_cfg(struct wil6210_priv *wil, bool enable, u16 threshold);
int reverse_memcmp(const void *cs, const void *ct, size_t count);
diff --git a/drivers/net/wireless/ath/wil6210/wmi.c b/drivers/net/wireless/ath/wil6210/wmi.c
index 0a0818f..298c918 100644
--- a/drivers/net/wireless/ath/wil6210/wmi.c
+++ b/drivers/net/wireless/ath/wil6210/wmi.c
@@ -484,6 +484,8 @@ static const char *cmdid2name(u16 cmdid)
return "WMI_FT_REASSOC_CMD";
case WMI_UPDATE_FT_IES_CMDID:
return "WMI_UPDATE_FT_IES_CMD";
+ case WMI_RBUFCAP_CFG_CMDID:
+ return "WMI_RBUFCAP_CFG_CMD";
default:
return "Untracked CMD";
}
@@ -628,6 +630,8 @@ static const char *eventid2name(u16 eventid)
return "WMI_FT_AUTH_STATUS_EVENT";
case WMI_FT_REASSOC_STATUS_EVENTID:
return "WMI_FT_REASSOC_STATUS_EVENT";
+ case WMI_RBUFCAP_CFG_EVENTID:
+ return "WMI_RBUFCAP_CFG_EVENT";
default:
return "Untracked EVENT";
}
@@ -2124,6 +2128,37 @@ int wmi_led_cfg(struct wil6210_priv *wil, bool enable)
return rc;
}
+int wmi_rbufcap_cfg(struct wil6210_priv *wil, bool enable, u16 threshold)
+{
+ struct wil6210_vif *vif = ndev_to_vif(wil->main_ndev);
+ int rc;
+
+ struct wmi_rbufcap_cfg_cmd cmd = {
+ .enable = enable,
+ .rx_desc_threshold = cpu_to_le16(threshold),
+ };
+ struct {
+ struct wmi_cmd_hdr wmi;
+ struct wmi_rbufcap_cfg_event evt;
+ } __packed reply = {
+ .evt = {.status = WMI_FW_STATUS_FAILURE},
+ };
+
+ rc = wmi_call(wil, WMI_RBUFCAP_CFG_CMDID, vif->mid, &cmd, sizeof(cmd),
+ WMI_RBUFCAP_CFG_EVENTID, &reply, sizeof(reply),
+ WIL_WMI_CALL_GENERAL_TO_MS);
+ if (rc)
+ return rc;
+
+ if (reply.evt.status != WMI_FW_STATUS_SUCCESS) {
+ wil_err(wil, "RBUFCAP_CFG failed. status %d\n",
+ reply.evt.status);
+ rc = -EINVAL;
+ }
+
+ return rc;
+}
+
int wmi_pcp_start(struct wil6210_vif *vif,
int bi, u8 wmi_nettype, u8 chan, u8 hidden_ssid, u8 is_go)
{
@@ -2715,7 +2750,7 @@ int wmi_addba_rx_resp(struct wil6210_priv *wil,
.dialog_token = token,
.status_code = cpu_to_le16(status),
/* bit 0: A-MSDU supported
- * bit 1: policy (should be 0 for us)
+ * bit 1: policy (controlled by FW)
* bits 2..5: TID
* bits 6..15: buffer size
*/
@@ -2769,7 +2804,7 @@ int wmi_addba_rx_resp_edma(struct wil6210_priv *wil, u8 mid, u8 cid, u8 tid,
.dialog_token = token,
.status_code = cpu_to_le16(status),
/* bit 0: A-MSDU supported
- * bit 1: policy (should be 0 for us)
+ * bit 1: policy (controlled by FW)
* bits 2..5: TID
* bits 6..15: buffer size
*/
--
1.9.1
^ permalink raw reply related
* [PATCH 00/11] wil6210 patches
From: Maya Erez @ 2019-06-16 7:25 UTC (permalink / raw)
To: Kalle Valo; +Cc: Maya Erez, linux-wireless, wil6210
The following set of patces include multiple wil6210 bug fixes.
Ahmad Masri (4):
wil6210: enlarge Tx status ring size
wil6210: increase the frequency of status ring hw tail update
wil6210: set WIL_WMI_CALL_GENERAL_TO_MS as wmi_call timeout
wil6210: drop old event after wmi_call timeout
Alexei Avshalom Lazar (3):
wil6210: do not reset FW in STA to P2P client interface switch
wil6210: Add support for setting RBUFCAP configuration
wil6210: update cid boundary check of wil_find_cid/_by_idx()
Dedy Lansky (1):
wil6210: fix printout in wil_read_pmccfg
Maya Erez (2):
wil6210: clear FW and ucode log address
wil6210: publish max_msdu_size to FW on BCAST ring
Tzahi Sabo (1):
wil6210: add support for reading multiple RFs temperature via debugfs
drivers/net/wireless/ath/wil6210/cfg80211.c | 22 ++++-
drivers/net/wireless/ath/wil6210/debugfs.c | 88 +++++++++++++++---
drivers/net/wireless/ath/wil6210/main.c | 19 +++-
drivers/net/wireless/ath/wil6210/pcie_bus.c | 1 +
drivers/net/wireless/ath/wil6210/rx_reorder.c | 31 +++----
drivers/net/wireless/ath/wil6210/txrx.c | 9 +-
drivers/net/wireless/ath/wil6210/txrx_edma.c | 16 +++-
drivers/net/wireless/ath/wil6210/txrx_edma.h | 2 +-
drivers/net/wireless/ath/wil6210/wil6210.h | 6 ++
drivers/net/wireless/ath/wil6210/wmi.c | 127 ++++++++++++++++++++++----
drivers/net/wireless/ath/wil6210/wmi.h | 47 ++++++++--
11 files changed, 297 insertions(+), 71 deletions(-)
--
1.9.1
^ permalink raw reply
* [PATCH 02/11] wil6210: enlarge Tx status ring size
From: Maya Erez @ 2019-06-16 7:25 UTC (permalink / raw)
To: Kalle Valo; +Cc: Ahmad Masri, linux-wireless, wil6210, Maya Erez
In-Reply-To: <1560669967-23706-1-git-send-email-merez@codeaurora.org>
From: Ahmad Masri <amasri@codeaurora.org>
With multiple clients and in high throughput scenarios, Tx status ring
can get full and become a bottleneck in Tx transmission.
Set the default Tx status ring size order to 13, previous value was 12.
This will double the status ring size from 4K entries to 8K entries.
Signed-off-by: Ahmad Masri <amasri@codeaurora.org>
Signed-off-by: Maya Erez <merez@codeaurora.org>
---
drivers/net/wireless/ath/wil6210/txrx_edma.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/wil6210/txrx_edma.h b/drivers/net/wireless/ath/wil6210/txrx_edma.h
index bb4ff28..e9e6ea9 100644
--- a/drivers/net/wireless/ath/wil6210/txrx_edma.h
+++ b/drivers/net/wireless/ath/wil6210/txrx_edma.h
@@ -24,7 +24,7 @@
#define WIL_SRING_SIZE_ORDER_MAX (WIL_RING_SIZE_ORDER_MAX)
/* RX sring order should be bigger than RX ring order */
#define WIL_RX_SRING_SIZE_ORDER_DEFAULT (12)
-#define WIL_TX_SRING_SIZE_ORDER_DEFAULT (12)
+#define WIL_TX_SRING_SIZE_ORDER_DEFAULT (14)
#define WIL_RX_BUFF_ARR_SIZE_DEFAULT (2600)
#define WIL_DEFAULT_RX_STATUS_RING_ID 0
--
1.9.1
^ permalink raw reply related
* [PATCH 11/11] wil6210: drop old event after wmi_call timeout
From: Maya Erez @ 2019-06-16 7:26 UTC (permalink / raw)
To: Kalle Valo; +Cc: Ahmad Masri, linux-wireless, wil6210, Maya Erez
In-Reply-To: <1560669967-23706-1-git-send-email-merez@codeaurora.org>
From: Ahmad Masri <amasri@codeaurora.org>
This change fixes a rare race condition of handling WMI events after
wmi_call expires.
wmi_recv_cmd immediately handles an event when reply_buf is defined and
a wmi_call is waiting for the event.
However, in case the wmi_call has already timed-out, there will be no
waiting/running wmi_call and the event will be queued in WMI queue and
will be handled later in wmi_event_handle.
Meanwhile, a new similar wmi_call for the same command and event may
be issued. In this case, when handling the queued event we got WARN_ON
printed.
Fixing this case as a valid timeout and drop the unexpected event.
Signed-off-by: Ahmad Masri <amasri@codeaurora.org>
Signed-off-by: Maya Erez <merez@codeaurora.org>
---
drivers/net/wireless/ath/wil6210/wmi.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/wil6210/wmi.c b/drivers/net/wireless/ath/wil6210/wmi.c
index 542ef15..475b1a2 100644
--- a/drivers/net/wireless/ath/wil6210/wmi.c
+++ b/drivers/net/wireless/ath/wil6210/wmi.c
@@ -3303,7 +3303,18 @@ static void wmi_event_handle(struct wil6210_priv *wil,
/* check if someone waits for this event */
if (wil->reply_id && wil->reply_id == id &&
wil->reply_mid == mid) {
- WARN_ON(wil->reply_buf);
+ if (wil->reply_buf) {
+ /* event received while wmi_call is waiting
+ * with a buffer. Such event should be handled
+ * in wmi_recv_cmd function. Handling the event
+ * here means a previous wmi_call was timeout.
+ * Drop the event and do not handle it.
+ */
+ wil_err(wil,
+ "Old event (%d, %s) while wmi_call is waiting. Drop it and Continue waiting\n",
+ id, eventid2name(id));
+ return;
+ }
wmi_evt_call_handler(vif, id, evt_data,
len - sizeof(*wmi));
--
1.9.1
^ permalink raw reply related
* Re: [PATCH] wireless: airo: switch to skcipher interface
From: Eric Biggers @ 2019-06-16 7:12 UTC (permalink / raw)
To: Ard Biesheuvel; +Cc: linux-wireless, kvalo, linux-crypto, herbert
In-Reply-To: <20190614093603.22771-1-ard.biesheuvel@linaro.org>
On Fri, Jun 14, 2019 at 11:36:03AM +0200, Ard Biesheuvel wrote:
> The AIRO driver applies a ctr(aes) on a buffer of considerable size
> (2400 bytes), and instead of invoking the crypto API to handle this
> in its entirety, it open codes the counter manipulation and invokes
> the AES block cipher directly.
>
> Let's fix this, by switching to the sync skcipher API instead.
>
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> ---
> NOTE: build tested only, since I don't have the hardware
>
> drivers/net/wireless/cisco/airo.c | 57 ++++++++++----------
> 1 file changed, 27 insertions(+), 30 deletions(-)
>
Need to also select CRYPTO_CTR in drivers/net/wireless/cisco/Kconfig under
AIRO_CS, and I think also CRYPTO_BLKCIPHER under AIRO.
> diff --git a/drivers/net/wireless/cisco/airo.c b/drivers/net/wireless/cisco/airo.c
> index 3f5a14112c6b..2d29ad10505b 100644
> --- a/drivers/net/wireless/cisco/airo.c
> +++ b/drivers/net/wireless/cisco/airo.c
> @@ -49,6 +49,9 @@
> #include <linux/kthread.h>
> #include <linux/freezer.h>
>
> +#include <crypto/aes.h>
> +#include <crypto/skcipher.h>
> +
> #include <net/cfg80211.h>
> #include <net/iw_handler.h>
>
> @@ -951,7 +954,7 @@ typedef struct {
> } mic_statistics;
>
> typedef struct {
> - u32 coeff[((EMMH32_MSGLEN_MAX)+3)>>2];
> + __be32 coeff[((EMMH32_MSGLEN_MAX)+3)>>2];
> u64 accum; // accumulated mic, reduced to u32 in final()
> int position; // current position (byte offset) in message
> union {
> @@ -1216,7 +1219,7 @@ struct airo_info {
> struct iw_spy_data spy_data;
> struct iw_public_data wireless_data;
> /* MIC stuff */
> - struct crypto_cipher *tfm;
> + struct crypto_sync_skcipher *tfm;
> mic_module mod[2];
> mic_statistics micstats;
> HostRxDesc rxfids[MPI_MAX_FIDS]; // rx/tx/config MPI350 descriptors
> @@ -1291,14 +1294,14 @@ static int flashrestart(struct airo_info *ai,struct net_device *dev);
> static int RxSeqValid (struct airo_info *ai,miccntx *context,int mcast,u32 micSeq);
> static void MoveWindow(miccntx *context, u32 micSeq);
> static void emmh32_setseed(emmh32_context *context, u8 *pkey, int keylen,
> - struct crypto_cipher *tfm);
> + struct crypto_sync_skcipher *tfm);
> static void emmh32_init(emmh32_context *context);
> static void emmh32_update(emmh32_context *context, u8 *pOctets, int len);
> static void emmh32_final(emmh32_context *context, u8 digest[4]);
> static int flashpchar(struct airo_info *ai,int byte,int dwelltime);
>
> static void age_mic_context(miccntx *cur, miccntx *old, u8 *key, int key_len,
> - struct crypto_cipher *tfm)
> + struct crypto_sync_skcipher *tfm)
> {
> /* If the current MIC context is valid and its key is the same as
> * the MIC register, there's nothing to do.
> @@ -1359,7 +1362,7 @@ static int micsetup(struct airo_info *ai) {
> int i;
>
> if (ai->tfm == NULL)
> - ai->tfm = crypto_alloc_cipher("aes", 0, 0);
> + ai->tfm = crypto_alloc_sync_skcipher("ctr(aes)", 0, 0);
>
> if (IS_ERR(ai->tfm)) {
> airo_print_err(ai->dev->name, "failed to load transform for AES");
> @@ -1624,37 +1627,31 @@ static void MoveWindow(miccntx *context, u32 micSeq)
>
> /* mic accumulate */
> #define MIC_ACCUM(val) \
> - context->accum += (u64)(val) * context->coeff[coeff_position++];
> -
> -static unsigned char aes_counter[16];
> + context->accum += (u64)(val) * be32_to_cpu(context->coeff[coeff_position++]);
You could alternatively call be32_to_cpu_array() after the AES encryption.
But this works too.
>
> /* expand the key to fill the MMH coefficient array */
> static void emmh32_setseed(emmh32_context *context, u8 *pkey, int keylen,
> - struct crypto_cipher *tfm)
> + struct crypto_sync_skcipher *tfm)
> {
> /* take the keying material, expand if necessary, truncate at 16-bytes */
> /* run through AES counter mode to generate context->coeff[] */
>
> - int i,j;
> - u32 counter;
> - u8 *cipher, plain[16];
> -
> - crypto_cipher_setkey(tfm, pkey, 16);
> - counter = 0;
> - for (i = 0; i < ARRAY_SIZE(context->coeff); ) {
> - aes_counter[15] = (u8)(counter >> 0);
> - aes_counter[14] = (u8)(counter >> 8);
> - aes_counter[13] = (u8)(counter >> 16);
> - aes_counter[12] = (u8)(counter >> 24);
> - counter++;
> - memcpy (plain, aes_counter, 16);
> - crypto_cipher_encrypt_one(tfm, plain, plain);
> - cipher = plain;
> - for (j = 0; (j < 16) && (i < ARRAY_SIZE(context->coeff)); ) {
> - context->coeff[i++] = ntohl(*(__be32 *)&cipher[j]);
> - j += 4;
> - }
> - }
> + SYNC_SKCIPHER_REQUEST_ON_STACK(req, tfm);
> + struct scatterlist dst, src;
> + u8 iv[AES_BLOCK_SIZE] = {};
> + int ret;
> +
> + crypto_sync_skcipher_setkey(tfm, pkey, 16);
> +
> + sg_init_one(&dst, context->coeff, sizeof(context->coeff));
> + sg_init_one(&src, page_address(ZERO_PAGE(0)), sizeof(context->coeff));
Should add:
BUILD_BUG_ON(sizeof(context->coeff) > PAGE_SIZE);
Or alternatively, instead of using ZERO_PAGE, just memset() the buffer to zero
and encrypt it in-place. That would be less fragile.
> +
> + skcipher_request_set_sync_tfm(req, tfm);
> + skcipher_request_set_callback(req, 0, NULL, NULL);
> + skcipher_request_set_crypt(req, &src, &dst, sizeof(context->coeff), iv);
> +
> + ret = crypto_skcipher_encrypt(req);
> + WARN_ON_ONCE(ret);
> }
>
> /* prepare for calculation of a new mic */
> @@ -2415,7 +2412,7 @@ void stop_airo_card( struct net_device *dev, int freeres )
> ai->shared, ai->shared_dma);
> }
> }
> - crypto_free_cipher(ai->tfm);
> + crypto_free_sync_skcipher(ai->tfm);
> del_airo_dev(ai);
> free_netdev( dev );
> }
> --
> 2.20.1
>
Otherwise this patch looks correct to me.
The actual crypto in this driver, on the other hand, looks very outdated and
broken. Apparently it's implementing some Cisco proprietary extension to WEP
that uses a universal hashing based MAC, where the hash key is generated from
AES-CTR. But the MAC is only 32 bits, and the universal hash (MMH) is
implemented incorrectly: there's an off-by-one error in emmh32_final() in the
code that is supposed to be an optimized version of 'sum % ((1ULL << 32) + 15)'.
Do we know whether anyone is actually using this, or is this just another old
driver that's sitting around unused?
- Eric
^ permalink raw reply
* Re: wpa_supplicant 2.8 fails in brcmf_cfg80211_set_pmk
From: Stefan Wahren @ 2019-06-15 17:21 UTC (permalink / raw)
To: Arend van Spriel, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
Wright Feng, linux-wireless, brcm80211-dev-list.pdl,
brcm80211-dev-list
In-Reply-To: <06f7bda7-eeaf-536b-a583-7c9bc5f681f5@gmx.net>
Am 15.06.19 um 19:01 schrieb Stefan Wahren:
> Hi,
>
> i was able to reproduce an (maybe older issue) with 4-way handshake
> offloading for 802.1X in the brcmfmac driver. My setup consists of
> Raspberry Pi 3 B (current linux-next, arm64/defconfig) on STA side and a
> Raspberry Pi 3 A+ (Linux 4.19) on AP side.
Looks like Raspberry Pi isn't the only affected platform [3], [4].
[3] - https://bugzilla.redhat.com/show_bug.cgi?id=1665608
[4] - https://bugzilla.kernel.org/show_bug.cgi?id=202521
^ permalink raw reply
* wpa_supplicant 2.8 fails in brcmf_cfg80211_set_pmk
From: Stefan Wahren @ 2019-06-15 17:01 UTC (permalink / raw)
To: Arend van Spriel, Franky Lin, Hante Meuleman, Chi-Hsien Lin,
Wright Feng, linux-wireless, brcm80211-dev-list.pdl,
brcm80211-dev-list
Hi,
i was able to reproduce an (maybe older issue) with 4-way handshake
offloading for 802.1X in the brcmfmac driver. My setup consists of
Raspberry Pi 3 B (current linux-next, arm64/defconfig) on STA side and a
Raspberry Pi 3 A+ (Linux 4.19) on AP side. The issue occurs on the STA
side with wpa_supplicant 2.8, which gives the following output:
Configure PMK for driver-based RSN 4-way handshake
EAPOL: Successfully fetched key (len=32)
RSN: Configure PMK for driver-based 4-way handshake - hexdump(len=32):
[REMOVED]
wpa_driver_nl80211_set_key: ifindex=3 (wlan0) alg=5 addr=(nil) key_idx=0
set_tx=0 seq_len=0 key_len=32
nl80211: Set PMK to the driver for b8:27:eb:6c:5e:c9
nl80211: PMK - hexdump(len=32): [REMOVED]
nl80211: Set PMK failed: ret=-22 (Invalid argument)
During this the kernel also gave this warning:
[ 874.485374] WARNING: CPU: 0 PID: 460 at
drivers/net/wireless/broadcom/brcm80211/brcmfmac/cfg80211.c:5208
brcmf_cfg80211_set_pmk+0x3c/0x58 [brcmfmac]
[ 874.504523] Modules linked in: 8021q garp stp mrp llc bcm2835_v4l2(C)
brcmfmac vc4 v4l2_common videobuf2_vmalloc videobuf2_memops
videobuf2_v4l2 cec videobuf2_common drm_kms_helper videodev brcmutil
hci_uart cfg80211 mc btbcm drm snd_bcm2835(C) bluetooth smsc95xx
crct10dif_ce usbnet ecdh_generic ecc drm_panel_orientation_quirks
raspberrypi_hwmon rfkill bcm2835_rng bcm2835_thermal pwm_bcm2835
i2c_bcm2835 rng_core vchiq(C) ip_tables x_tables ipv6 nf_defrag_ipv6
[ 874.558134] CPU: 0 PID: 460 Comm: wpa_supplicant Tainted: G
WC 5.2.0-rc4-next-20190614-g65beedb66 #3
[ 874.574984] Hardware name: Raspberry Pi 3 Model B (DT)
[ 874.586546] pstate: 80000005 (Nzcv daif -PAN -UAO)
[ 874.597817] pc : brcmf_cfg80211_set_pmk+0x3c/0x58 [brcmfmac]
[ 874.610049] lr : nl80211_set_pmk+0x16c/0x1a8 [cfg80211]
[ 874.621776] sp : ffff000011aab910
[ 874.631533] x29: ffff000011aab910 x28: ffff80002ec5a000
[ 874.643326] x27: 0000000000000014 x26: ffff80002fd9c300
[ 874.655094] x25: ffff80002fd9c000 x24: ffff80002ec5c000
[ 874.666843] x23: 00000000ffffff95 x22: ffff80002ec5d050
[ 874.678580] x21: ffff80002ec5d008 x20: ffff000011aaba30
[ 874.690336] x19: ffff000011349000 x18: 0000000000000000
[ 874.702080] x17: 0000000000000000 x16: 0000000000000000
[ 874.713809] x15: 0000000000000000 x14: be1127680d12277d
[ 874.725547] x13: 8ba575fc53793d9f x12: ffff000008dff8a8
[ 874.737297] x11: 0000000000000fe0 x10: 0000000000000000
[ 874.749059] x9 : ffff000010c12068 x8 : ffff000010c12050
[ 874.760832] x7 : ffff000008dfe8c8 x6 : 000000000000003f
[ 874.772598] x5 : 0000000000000008 x4 : 000000006ceb27b8
[ 874.784349] x3 : ffff000008ef1eb0 x2 : ffff000011aab978
[ 874.796091] x1 : 0000000000000000 x0 : ffff80002ec5c7c0
[ 874.807853] Call trace:
[ 874.816698] brcmf_cfg80211_set_pmk+0x3c/0x58 [brcmfmac]
[ 874.828399] nl80211_set_pmk+0x16c/0x1a8 [cfg80211]
[ 874.839327] genl_family_rcv_msg+0x364/0x460
[ 874.849343] genl_rcv_msg+0x5c/0xc0
[ 874.858282] netlink_rcv_skb+0x5c/0x128
[ 874.867486] genl_rcv+0x34/0x48
[ 874.875956] netlink_unicast+0x190/0x1f8
[ 874.885203] netlink_sendmsg+0x2cc/0x348
[ 874.894397] sock_sendmsg+0x18/0x30
[ 874.903124] ___sys_sendmsg+0x28c/0x2c8
[ 874.912216] __sys_sendmsg+0x6c/0xc8
[ 874.921040] __arm64_sys_sendmsg+0x20/0x28
[ 874.930408] el0_svc_common.constprop.0+0x64/0x160
[ 874.940520] el0_svc_handler+0x28/0x78
[ 874.949552] el0_svc+0x8/0xc
[ 874.957674] ---[ end trace 72f634728d4e750f ]---
Here are the information about the used firmware:
[ 11.622355] brcmfmac: brcmf_fw_alloc_request: using
brcm/brcmfmac43430-sdio for chip BCM43430/1
[ 11.637498] brcmfmac: brcmf_c_process_clm_blob: no clm_blob available
(err=-2), device may have limited channels available
[ 11.658814] brcmfmac: brcmf_c_preinit_dcmds: Firmware: BCM43430/1
wl0: Oct 23 2017 03:55:53 version 7.45.98.38 (r674442 CY) FWID 01-e58d219f
The actual STA configuration can be found here [1] and other report of
this issue here [2].
Any ideas how to fix this?
[1] - https://gist.github.com/lategoodbye/d4b5da60e905cbdf069affbd41cd14ab'
[2] - https://archlinuxarm.org/forum/viewtopic.php?f=60&t=13644
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox