Linux wireless drivers development
 help / color / mirror / Atom feed
* [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 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 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 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 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 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 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

* 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

* 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: 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] 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] 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: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: 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

* [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

* [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 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 5/7] lib/hexdump.c: Allow multiple groups to be separated by lines '|'
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>

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

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

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

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 include/linux/printk.h |  3 +++
 lib/hexdump.c          | 59 ++++++++++++++++++++++++++++++++++++------
 2 files changed, 54 insertions(+), 8 deletions(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index 97dd29a2bd77..c6b748f66a82 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -484,6 +484,9 @@ enum {
 
 #define HEXDUMP_ASCII			BIT(0)
 #define HEXDUMP_SUPPRESS_REPEATED	BIT(1)
+#define HEXDUMP_2_GRP_LINES		BIT(2)
+#define HEXDUMP_4_GRP_LINES		BIT(3)
+#define HEXDUMP_8_GRP_LINES		BIT(4)
 
 extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
 			      int groupsize, char *linebuf, size_t linebuflen,
diff --git a/lib/hexdump.c b/lib/hexdump.c
index 08c6084d7daa..4da7d24826fb 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -77,6 +77,23 @@ char *bin2hex(char *dst, const void *src, size_t count)
 }
 EXPORT_SYMBOL(bin2hex);
 
+static const char *group_separator(int group, u64 flags)
+{
+	if (group == 0)
+		return " ";
+
+	if ((flags & HEXDUMP_8_GRP_LINES) && !((group) % 8))
+		return "|";
+
+	if ((flags & HEXDUMP_4_GRP_LINES) && !((group) % 4))
+		return "|";
+
+	if ((flags & HEXDUMP_2_GRP_LINES) && !((group) % 2))
+		return "|";
+
+	return " ";
+}
+
 /**
  * hex_dump_to_buffer - convert a blob of data to "hex ASCII" in memory
  * @buf: data blob to dump
@@ -87,6 +104,9 @@ EXPORT_SYMBOL(bin2hex);
  * @linebuflen: total size of @linebuf, including space for terminating NUL
  * @flags: A bitwise OR of the following flags:
  *	HEXDUMP_ASCII:			include ASCII after the hex output
+ *	HEXDUMP_2_GRP_LINES:		insert a '|' after every 2 groups
+ *	HEXDUMP_4_GRP_LINES:		insert a '|' after every 4 groups
+ *	HEXDUMP_8_GRP_LINES:		insert a '|' after every 8 groups
  *
  * hex_dump_to_buffer() works on one "line" of output at a time, converting
  * <groupsize> bytes of input to hexadecimal (and optionally printable ASCII)
@@ -118,6 +138,7 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
 	int j, lx = 0;
 	int ascii_column;
 	int ret;
+	int line_chars = 0;
 
 	if (!is_power_of_2(groupsize) || groupsize > 8)
 		groupsize = 1;
@@ -144,7 +165,8 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
 
 		for (j = 0; j < ngroups; j++) {
 			ret = snprintf(linebuf + lx, linebuflen - lx,
-				       "%s%16.16llx", j ? " " : "",
+				       "%s%16.16llx",
+				       j ? group_separator(j, flags) : "",
 				       get_unaligned(ptr8 + j));
 			if (ret >= linebuflen - lx)
 				goto overflow1;
@@ -155,7 +177,8 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
 
 		for (j = 0; j < ngroups; j++) {
 			ret = snprintf(linebuf + lx, linebuflen - lx,
-				       "%s%8.8x", j ? " " : "",
+				       "%s%8.8x",
+				       j ? group_separator(j, flags) : "",
 				       get_unaligned(ptr4 + j));
 			if (ret >= linebuflen - lx)
 				goto overflow1;
@@ -166,7 +189,8 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
 
 		for (j = 0; j < ngroups; j++) {
 			ret = snprintf(linebuf + lx, linebuflen - lx,
-				       "%s%4.4x", j ? " " : "",
+				       "%s%4.4x",
+				       j ? group_separator(j, flags) : "",
 				       get_unaligned(ptr2 + j));
 			if (ret >= linebuflen - lx)
 				goto overflow1;
@@ -196,11 +220,26 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
 			goto overflow2;
 		linebuf[lx++] = ' ';
 	}
+
+	if (flags & HEXDUMP_2_GRP_LINES)
+		line_chars = groupsize * 2;
+	if (flags & HEXDUMP_4_GRP_LINES)
+		line_chars = groupsize * 4;
+	if (flags & HEXDUMP_8_GRP_LINES)
+		line_chars = groupsize * 8;
+
 	for (j = 0; j < len; j++) {
 		if (linebuflen < lx + 2)
 			goto overflow2;
 		ch = ptr[j];
 		linebuf[lx++] = (isascii(ch) && isprint(ch)) ? ch : '.';
+
+		if (line_chars && ((j + 1) < len) &&
+				((j + 1) % line_chars == 0)) {
+			if (linebuflen < lx + 2)
+				goto overflow2;
+			linebuf[lx++] = '|';
+		}
 	}
 nil:
 	linebuf[lx] = '\0';
@@ -208,7 +247,8 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
 overflow2:
 	linebuf[lx++] = '\0';
 overflow1:
-	return (flags & HEXDUMP_ASCII) ? ascii_column + len :
+	return (flags & HEXDUMP_ASCII) ? ascii_column + len +
+					(len - 1) / line_chars :
 					 (groupsize * 2 + 1) * ngroups - 1;
 }
 EXPORT_SYMBOL(hex_dump_to_buffer);
@@ -246,7 +286,7 @@ static void announce_skipped(const char *level, const char *prefix_str,
 	if (count == 0)
 		return;
 
-	printk("%s%s ** Skipped %lu bytes of value 0x%x **\n",
+	printk("%s%s ** Skipped %lu bytes of value 0x%02x **\n",
 	       level, prefix_str, count, val);
 }
 
@@ -266,6 +306,9 @@ static void announce_skipped(const char *level, const char *prefix_str,
  *	HEXDUMP_ASCII:			include ASCII after the hex output
  *	HEXDUMP_SUPPRESS_REPEATED:	suppress repeated lines of identical
  *					bytes
+ *	HEXDUMP_2_GRP_LINES:		insert a '|' after every 2 groups
+ *	HEXDUMP_4_GRP_LINES:		insert a '|' after every 4 groups
+ *	HEXDUMP_8_GRP_LINES:		insert a '|' after every 8 groups
  *
  * Given a buffer of u8 data, print_hex_dump() prints a hex + ASCII dump
  * to the kernel log at the specified kernel log level, with an optional
@@ -295,14 +338,14 @@ void print_hex_dump_ext(const char *level, const char *prefix_str,
 	u8 skipped_val = 0;
 	size_t skipped = 0;
 
-
 	if (rowsize % groupsize)
 		rowsize -= rowsize % groupsize;
 
 	/* Worst case line length:
-	 * 2 hex chars + space per byte in, 2 spaces, 1 char per byte in, NULL
+	 * 2 hex chars + space per byte in, 2 spaces, 1 char per byte in,
+	 * 1 char per N groups, NULL
 	 */
-	linebuf_len = rowsize * 3 + 2 + rowsize + 1;
+	linebuf_len = rowsize * 3 + 2 + rowsize + rowsize / groupsize + 1;
 	linebuf = kzalloc(linebuf_len, GFP_KERNEL);
 	if (!linebuf) {
 		printk("%s%shexdump: Could not alloc %u bytes for buffer\n",
-- 
2.21.0


^ permalink raw reply related

* [PATCH v3 4/7] lib/hexdump.c: Replace ascii bool in hex_dump_to_buffer with flags
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>

In order to support additional features in hex_dump_to_buffer, replace
the ascii bool parameter with flags.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 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 +-
 drivers/net/ethernet/synopsys/dwc-xlgmac-common.c |  2 +-
 drivers/net/wireless/ath/ath10k/debug.c           |  3 ++-
 drivers/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                            |  8 ++++----
 lib/hexdump.c                                     | 15 ++++++++-------
 lib/test_hexdump.c                                |  5 +++--
 14 files changed, 33 insertions(+), 29 deletions(-)

diff --git a/drivers/gpu/drm/i915/intel_engine_cs.c b/drivers/gpu/drm/i915/intel_engine_cs.c
index eea9bec04f1b..5df5fffdb848 100644
--- a/drivers/gpu/drm/i915/intel_engine_cs.c
+++ b/drivers/gpu/drm/i915/intel_engine_cs.c
@@ -1340,7 +1340,7 @@ static void hexdump(struct drm_printer *m, const void *buf, size_t len)
 		WARN_ON_ONCE(hex_dump_to_buffer(buf + pos, len - pos,
 						rowsize, sizeof(u32),
 						line, sizeof(line),
-						false) >= sizeof(line));
+						0) >= sizeof(line));
 		drm_printf(m, "[%04zx] %s\n", pos, line);
 
 		prev = buf + pos;
diff --git a/drivers/isdn/hardware/mISDN/mISDNisar.c b/drivers/isdn/hardware/mISDN/mISDNisar.c
index fd5c52f37802..ccc0ee9d894f 100644
--- a/drivers/isdn/hardware/mISDN/mISDNisar.c
+++ b/drivers/isdn/hardware/mISDN/mISDNisar.c
@@ -71,7 +71,8 @@ send_mbox(struct isar_hw *isar, u8 his, u8 creg, u8 len, u8 *msg)
 
 			while (l < (int)len) {
 				hex_dump_to_buffer(msg + l, len - l, 32, 1,
-						   isar->log, 256, 1);
+						   isar->log, 256,
+						   HEXDUMP_ASCII);
 				pr_debug("%s: %s %02x: %s\n", isar->name,
 					 __func__, l, isar->log);
 				l += 32;
@@ -100,7 +101,8 @@ rcv_mbox(struct isar_hw *isar, u8 *msg)
 
 			while (l < (int)isar->clsb) {
 				hex_dump_to_buffer(msg + l, isar->clsb - l, 32,
-						   1, isar->log, 256, 1);
+						   1, isar->log, 256,
+						   HEXDUMP_ASCII);
 				pr_debug("%s: %s %02x: %s\n", isar->name,
 					 __func__, l, isar->log);
 				l += 32;
diff --git a/drivers/mailbox/mailbox-test.c b/drivers/mailbox/mailbox-test.c
index 4555d678fadd..23c3fbafdcb2 100644
--- a/drivers/mailbox/mailbox-test.c
+++ b/drivers/mailbox/mailbox-test.c
@@ -209,7 +209,7 @@ static ssize_t mbox_test_message_read(struct file *filp, char __user *userbuf,
 		hex_dump_to_buffer(ptr,
 				   MBOX_BYTES_PER_LINE,
 				   MBOX_BYTES_PER_LINE, 1, touser + l,
-				   MBOX_HEXDUMP_LINE_LEN, true);
+				   MBOX_HEXDUMP_LINE_LEN, HEXDUMP_ASCII);
 
 		ptr += MBOX_BYTES_PER_LINE;
 		l += MBOX_HEXDUMP_LINE_LEN;
diff --git a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
index 3dd0cecddba8..1e26410cf6c2 100644
--- a/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
+++ b/drivers/net/ethernet/amd/xgbe/xgbe-drv.c
@@ -2992,7 +2992,7 @@ void xgbe_print_pkt(struct net_device *netdev, struct sk_buff *skb, bool tx_rx)
 		unsigned int len = min(skb->len - i, 32U);
 
 		hex_dump_to_buffer(&skb->data[i], len, 32, 1,
-				   buffer, sizeof(buffer), false);
+				   buffer, sizeof(buffer), 0);
 		netdev_dbg(netdev, "  %#06x: %s\n", i, buffer);
 	}
 
diff --git a/drivers/net/ethernet/synopsys/dwc-xlgmac-common.c b/drivers/net/ethernet/synopsys/dwc-xlgmac-common.c
index eb1c6b03c329..b80adfa1f890 100644
--- a/drivers/net/ethernet/synopsys/dwc-xlgmac-common.c
+++ b/drivers/net/ethernet/synopsys/dwc-xlgmac-common.c
@@ -349,7 +349,7 @@ void xlgmac_print_pkt(struct net_device *netdev,
 		unsigned int len = min(skb->len - i, 32U);
 
 		hex_dump_to_buffer(&skb->data[i], len, 32, 1,
-				   buffer, sizeof(buffer), false);
+				   buffer, sizeof(buffer), 0);
 		netdev_dbg(netdev, "  %#06x: %s\n", i, buffer);
 	}
 
diff --git a/drivers/net/wireless/ath/ath10k/debug.c b/drivers/net/wireless/ath/ath10k/debug.c
index 32d967a31c65..4c99ea03226d 100644
--- a/drivers/net/wireless/ath/ath10k/debug.c
+++ b/drivers/net/wireless/ath/ath10k/debug.c
@@ -2662,7 +2662,8 @@ void ath10k_dbg_dump(struct ath10k *ar,
 						(unsigned int)(ptr - buf));
 			hex_dump_to_buffer(ptr, len - (ptr - buf), 16, 1,
 					   linebuf + linebuflen,
-					   sizeof(linebuf) - linebuflen, true);
+					   sizeof(linebuf) - linebuflen,
+					   HEXDUMP_ASCII);
 			dev_printk(KERN_DEBUG, ar->dev, "%s\n", linebuf);
 		}
 	}
diff --git a/drivers/net/wireless/intel/iwlegacy/3945-mac.c b/drivers/net/wireless/intel/iwlegacy/3945-mac.c
index b82da75a9ae3..81c4b178527a 100644
--- a/drivers/net/wireless/intel/iwlegacy/3945-mac.c
+++ b/drivers/net/wireless/intel/iwlegacy/3945-mac.c
@@ -3232,7 +3232,7 @@ il3945_show_measurement(struct device *d, struct device_attribute *attr,
 
 	while (size && PAGE_SIZE - len) {
 		hex_dump_to_buffer(data + ofs, size, 16, 1, buf + len,
-				   PAGE_SIZE - len, true);
+				   PAGE_SIZE - len, HEXDUMP_ASCII);
 		len = strlen(buf);
 		if (PAGE_SIZE - len)
 			buf[len++] = '\n';
diff --git a/drivers/platform/chrome/wilco_ec/debugfs.c b/drivers/platform/chrome/wilco_ec/debugfs.c
index f163476d080d..c53b18539f52 100644
--- a/drivers/platform/chrome/wilco_ec/debugfs.c
+++ b/drivers/platform/chrome/wilco_ec/debugfs.c
@@ -148,7 +148,7 @@ static ssize_t raw_read(struct file *file, char __user *user_buf, size_t count,
 					     debug_info->response_size,
 					     16, 1, debug_info->formatted_data,
 					     sizeof(debug_info->formatted_data),
-					     true);
+					     HEXDUMP_ASCII);
 		/* Only return response the first time it is read */
 		debug_info->response_size = 0;
 	}
diff --git a/drivers/scsi/scsi_logging.c b/drivers/scsi/scsi_logging.c
index 39b8cc4574b4..c7f3eb41d32c 100644
--- a/drivers/scsi/scsi_logging.c
+++ b/drivers/scsi/scsi_logging.c
@@ -262,7 +262,7 @@ void scsi_print_command(struct scsi_cmnd *cmd)
 						 "CDB[%02x]: ", k);
 				hex_dump_to_buffer(&cmd->cmnd[k], linelen,
 						   16, 1, logbuf + off,
-						   logbuf_len - off, false);
+						   logbuf_len - off, 0);
 			}
 			dev_printk(KERN_INFO, &cmd->device->sdev_gendev, "%s",
 				   logbuf);
@@ -273,8 +273,7 @@ void scsi_print_command(struct scsi_cmnd *cmd)
 	if (!WARN_ON(off > logbuf_len - 49)) {
 		off += scnprintf(logbuf + off, logbuf_len - off, " ");
 		hex_dump_to_buffer(cmd->cmnd, cmd->cmd_len, 16, 1,
-				   logbuf + off, logbuf_len - off,
-				   false);
+				   logbuf + off, logbuf_len - off, 0);
 	}
 out_printk:
 	dev_printk(KERN_INFO, &cmd->device->sdev_gendev, "%s", logbuf);
@@ -353,8 +352,7 @@ scsi_log_dump_sense(const struct scsi_device *sdev, const char *name, int tag,
 		off = sdev_format_header(logbuf, logbuf_len,
 					 name, tag);
 		hex_dump_to_buffer(&sense_buffer[i], len, 16, 1,
-				   logbuf + off, logbuf_len - off,
-				   false);
+				   logbuf + off, logbuf_len - off, 0);
 		dev_printk(KERN_INFO, &sdev->sdev_gendev, "%s", logbuf);
 	}
 	scsi_log_release_buffer(logbuf);
diff --git a/drivers/staging/fbtft/fbtft-core.c b/drivers/staging/fbtft/fbtft-core.c
index 9b07badf4c6c..2e5df5cc9d61 100644
--- a/drivers/staging/fbtft/fbtft-core.c
+++ b/drivers/staging/fbtft/fbtft-core.c
@@ -61,7 +61,7 @@ void fbtft_dbg_hex(const struct device *dev, int groupsize,
 	va_end(args);
 
 	hex_dump_to_buffer(buf, len, 32, groupsize, text + text_len,
-			   512 - text_len, false);
+			   512 - text_len, 0);
 
 	if (len > 32)
 		dev_info(dev, "%s ...\n", text);
diff --git a/fs/seq_file.c b/fs/seq_file.c
index abe27ec43176..472a8acb7405 100644
--- a/fs/seq_file.c
+++ b/fs/seq_file.c
@@ -874,7 +874,8 @@ void seq_hex_dump(struct seq_file *m, const char *prefix_str, int prefix_type,
 
 		size = seq_get_buf(m, &buffer);
 		ret = hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
-					 buffer, size, ascii);
+					 buffer, size,
+					 ascii ? HEXDUMP_ASCII : 0);
 		seq_commit(m, ret < size ? ret : -1);
 
 		seq_putc(m, '\n');
diff --git a/include/linux/printk.h b/include/linux/printk.h
index d7754799cfe0..97dd29a2bd77 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -482,13 +482,13 @@ enum {
 	DUMP_PREFIX_OFFSET
 };
 
-extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
-			      int groupsize, char *linebuf, size_t linebuflen,
-			      bool ascii);
-
 #define HEXDUMP_ASCII			BIT(0)
 #define HEXDUMP_SUPPRESS_REPEATED	BIT(1)
 
+extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
+			      int groupsize, char *linebuf, size_t linebuflen,
+			      u32 flags);
+
 #ifdef CONFIG_PRINTK
 extern void print_hex_dump_ext(const char *level, const char *prefix_str,
 			   int prefix_type, int rowsize, int groupsize,
diff --git a/lib/hexdump.c b/lib/hexdump.c
index b781f888884e..08c6084d7daa 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -85,7 +85,8 @@ EXPORT_SYMBOL(bin2hex);
  * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1)
  * @linebuf: where to put the converted data
  * @linebuflen: total size of @linebuf, including space for terminating NUL
- * @ascii: include ASCII after the hex output
+ * @flags: A bitwise OR of the following flags:
+ *	HEXDUMP_ASCII:			include ASCII after the hex output
  *
  * hex_dump_to_buffer() works on one "line" of output at a time, converting
  * <groupsize> bytes of input to hexadecimal (and optionally printable ASCII)
@@ -97,7 +98,7 @@ EXPORT_SYMBOL(bin2hex);
  *
  * E.g.:
  *   hex_dump_to_buffer(frame->data, frame->len, 16, 1,
- *			linebuf, sizeof(linebuf), true);
+ *			linebuf, sizeof(linebuf), HEXDUMP_ASCII);
  *
  * example output buffer:
  * 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f  @ABCDEFGHIJKLMNO
@@ -109,7 +110,7 @@ EXPORT_SYMBOL(bin2hex);
  * string if enough space had been available.
  */
 int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
-		       char *linebuf, size_t linebuflen, bool ascii)
+		       char *linebuf, size_t linebuflen, u32 flags)
 {
 	const u8 *ptr = buf;
 	int ngroups;
@@ -187,7 +188,7 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
 		if (j)
 			lx--;
 	}
-	if (!ascii)
+	if (!(flags & HEXDUMP_ASCII))
 		goto nil;
 
 	while (lx < ascii_column) {
@@ -207,7 +208,8 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
 overflow2:
 	linebuf[lx++] = '\0';
 overflow1:
-	return ascii ? ascii_column + len : (groupsize * 2 + 1) * ngroups - 1;
+	return (flags & HEXDUMP_ASCII) ? ascii_column + len :
+					 (groupsize * 2 + 1) * ngroups - 1;
 }
 EXPORT_SYMBOL(hex_dump_to_buffer);
 
@@ -336,8 +338,7 @@ void print_hex_dump_ext(const char *level, const char *prefix_str,
 		}
 
 		hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
-				   linebuf, linebuf_len,
-				   flags & HEXDUMP_ASCII);
+				   linebuf, linebuf_len, flags);
 
 		switch (prefix_type) {
 		case DUMP_PREFIX_ADDRESS:
diff --git a/lib/test_hexdump.c b/lib/test_hexdump.c
index 6ab75a209b43..ae340c5c1c6f 100644
--- a/lib/test_hexdump.c
+++ b/lib/test_hexdump.c
@@ -166,7 +166,7 @@ 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);
+			   ascii ? HEXDUMP_ASCII : 0);
 
 	memset(test, FILL_CHAR, sizeof(test));
 	test_hexdump_prepare_test(len, rowsize, groupsize, test, sizeof(test),
@@ -204,7 +204,8 @@ 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);
+	rc = hex_dump_to_buffer(data_b, len, rowsize, groupsize, buf, buflen,
+				ascii ? HEXDUMP_ASCII : 0);
 
 	/*
 	 * Caller must provide the data length multiple of groupsize. The
-- 
2.21.0


^ permalink raw reply related

* [PATCH v3 1/7] lib/hexdump.c: Fix selftests
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 overflow tests did not account for the situation where no
overflow occurs and len < rowsize.

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

The selftests now pass.

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 lib/test_hexdump.c | 47 ++++++++++++++++++++++++++--------------------
 1 file changed, 27 insertions(+), 20 deletions(-)

diff --git a/lib/test_hexdump.c b/lib/test_hexdump.c
index 5144899d3c6b..d78ddd62ffd0 100644
--- a/lib/test_hexdump.c
+++ b/lib/test_hexdump.c
@@ -163,45 +163,52 @@ static void __init test_hexdump_overflow(size_t buflen, size_t len,
 {
 	char test[TEST_HEXDUMP_BUF_SIZE];
 	char buf[TEST_HEXDUMP_BUF_SIZE];
-	int rs = rowsize, gs = groupsize;
-	int ae, he, e, f, r;
-	bool a;
+	int ascii_len, hex_len, expected_len, fill_point, ngroups, rc;
+	bool match;
 
 	total_tests++;
 
 	memset(buf, FILL_CHAR, sizeof(buf));
 
-	r = hex_dump_to_buffer(data_b, len, rs, gs, buf, buflen, ascii);
+	rc = hex_dump_to_buffer(data_b, len, rowsize, groupsize, buf, buflen, ascii);
 
 	/*
 	 * Caller must provide the data length multiple of groupsize. The
 	 * calculations below are made with that assumption in mind.
 	 */
-	ae = rs * 2 /* hex */ + rs / gs /* spaces */ + 1 /* space */ + len /* ascii */;
-	he = (gs * 2 /* hex */ + 1 /* space */) * len / gs - 1 /* no trailing space */;
+	ngroups = rowsize / groupsize;
+	hex_len = (groupsize * 2 /* hex */ + 1 /* spaces */) * ngroups
+		  - 1 /* no trailing space */;
+	ascii_len = hex_len + 2 /* space */ + len /* ascii */;
+
+	if (len < rowsize) {
+		ngroups = len / groupsize;
+		hex_len = (groupsize * 2 /* hex */ + 1 /* spaces */) * ngroups
+		  - 1 /* no trailing space */;
+	}
 
-	if (ascii)
-		e = ae;
-	else
-		e = he;
+	expected_len = (ascii) ? ascii_len : hex_len;
 
-	f = min_t(int, e + 1, buflen);
+	fill_point = min_t(int, expected_len + 1, buflen);
 	if (buflen) {
-		test_hexdump_prepare_test(len, rs, gs, test, sizeof(test), ascii);
-		test[f - 1] = '\0';
+		test_hexdump_prepare_test(len, rowsize, groupsize, test,
+					  sizeof(test), ascii);
+		test[fill_point - 1] = '\0';
 	}
-	memset(test + f, FILL_CHAR, sizeof(test) - f);
+	memset(test + fill_point, FILL_CHAR, sizeof(test) - fill_point);
 
-	a = r == e && !memcmp(test, buf, TEST_HEXDUMP_BUF_SIZE);
+	match = rc == expected_len && !memcmp(test, buf, TEST_HEXDUMP_BUF_SIZE);
 
 	buf[sizeof(buf) - 1] = '\0';
 
-	if (!a) {
-		pr_err("Len: %zu buflen: %zu strlen: %zu\n",
-			len, buflen, strnlen(buf, sizeof(buf)));
-		pr_err("Result: %d '%s'\n", r, buf);
-		pr_err("Expect: %d '%s'\n", e, test);
+	if (!match) {
+		pr_err("rowsize: %u groupsize: %u ascii: %d Len: %zu buflen: %zu strlen: %zu\n",
+			rowsize, groupsize, ascii, len, buflen,
+			strnlen(buf, sizeof(buf)));
+		pr_err("Result: %d '%-.*s'\n", rc, (int)buflen, buf);
+		pr_err("Expect: %d '%-.*s'\n", expected_len, (int)buflen, test);
 		failed_tests++;
+
 	}
 }
 
-- 
2.21.0


^ permalink raw reply related

* [PATCH v3 3/7] lib/hexdump.c: Optionally suppress lines of repeated bytes
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>

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

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

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

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 include/linux/printk.h | 25 +++++++++---
 lib/hexdump.c          | 91 ++++++++++++++++++++++++++++++++++++------
 2 files changed, 99 insertions(+), 17 deletions(-)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index cefd374c47b1..d7754799cfe0 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -481,13 +481,18 @@ enum {
 	DUMP_PREFIX_ADDRESS,
 	DUMP_PREFIX_OFFSET
 };
+
 extern int hex_dump_to_buffer(const void *buf, size_t len, int rowsize,
 			      int groupsize, char *linebuf, size_t linebuflen,
 			      bool ascii);
+
+#define HEXDUMP_ASCII			BIT(0)
+#define HEXDUMP_SUPPRESS_REPEATED	BIT(1)
+
 #ifdef CONFIG_PRINTK
-extern void print_hex_dump(const char *level, const char *prefix_str,
+extern void print_hex_dump_ext(const char *level, const char *prefix_str,
 			   int prefix_type, int rowsize, int groupsize,
-			   const void *buf, size_t len, bool ascii);
+			   const void *buf, size_t len, u32 flags);
 #if defined(CONFIG_DYNAMIC_DEBUG)
 #define print_hex_dump_bytes(prefix_str, prefix_type, buf, len)	\
 	dynamic_hex_dump(prefix_str, prefix_type, 16, 1, buf, len, true)
@@ -496,18 +501,28 @@ extern void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
 				 const void *buf, size_t len);
 #endif /* defined(CONFIG_DYNAMIC_DEBUG) */
 #else
-static inline void print_hex_dump(const char *level, const char *prefix_str,
+static inline void print_hex_dump_ext(const char *level, const char *prefix_str,
 				  int prefix_type, int rowsize, int groupsize,
-				  const void *buf, size_t len, bool ascii)
+				  const void *buf, size_t len, u32 flags)
 {
 }
 static inline void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
 					const void *buf, size_t len)
 {
 }
-
 #endif
 
+static __always_inline void print_hex_dump(const char *level,
+					   const char *prefix_str,
+					   int prefix_type, int rowsize,
+					   int groupsize, const void *buf,
+					   size_t len, bool ascii)
+{
+	print_hex_dump_ext(level, prefix_str, prefix_type, rowsize, groupsize,
+			buf, len, ascii ? HEXDUMP_ASCII : 0);
+}
+
+
 #if defined(CONFIG_DYNAMIC_DEBUG)
 #define print_hex_dump_debug(prefix_str, prefix_type, rowsize,	\
 			     groupsize, buf, len, ascii)	\
diff --git a/lib/hexdump.c b/lib/hexdump.c
index 3943507bc0e9..b781f888884e 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -212,8 +212,44 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
 EXPORT_SYMBOL(hex_dump_to_buffer);
 
 #ifdef CONFIG_PRINTK
+
+/**
+ * buf_is_all - Check if a buffer contains only a single byte value
+ * @buf: pointer to the buffer
+ * @len: the size of the buffer in bytes
+ * @val: outputs the value if if the bytes are identical
+ */
+static bool buf_is_all(const u8 *buf, size_t len, u8 *val_out)
+{
+	size_t i;
+	u8 val;
+
+	if (len <= 1)
+		return false;
+
+	val = buf[0];
+
+	for (i = 1; i < len; i++) {
+		if (buf[i] != val)
+			return false;
+	}
+
+	*val_out = val;
+	return true;
+}
+
+static void announce_skipped(const char *level, const char *prefix_str,
+				   u8 val, size_t count)
+{
+	if (count == 0)
+		return;
+
+	printk("%s%s ** Skipped %lu bytes of value 0x%x **\n",
+	       level, prefix_str, count, val);
+}
+
 /**
- * print_hex_dump - print a text hex dump to syslog for a binary blob of data
+ * print_hex_dump_ext - dump a binary blob of data to syslog in hexadecimal
  * @level: kernel log level (e.g. KERN_DEBUG)
  * @prefix_str: string to prefix each line with;
  *  caller supplies trailing spaces for alignment if desired
@@ -224,6 +260,10 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
  * @buf: data blob to dump
  * @len: number of bytes in the @buf
  * @ascii: include ASCII after the hex output
+ * @flags: A bitwise OR of the following flags:
+ *	HEXDUMP_ASCII:			include ASCII after the hex output
+ *	HEXDUMP_SUPPRESS_REPEATED:	suppress repeated lines of identical
+ *					bytes
  *
  * Given a buffer of u8 data, print_hex_dump() prints a hex + ASCII dump
  * to the kernel log at the specified kernel log level, with an optional
@@ -234,22 +274,25 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
  * (optionally) ASCII output.
  *
  * E.g.:
- *   print_hex_dump(KERN_DEBUG, "raw data: ", DUMP_PREFIX_ADDRESS,
- *		    16, 1, frame->data, frame->len, true);
+ *   print_hex_dump_ext(KERN_DEBUG, "raw data: ", DUMP_PREFIX_ADDRESS,
+ *		    16, 1, frame->data, frame->len, HEXDUMP_ASCII);
  *
  * Example output using %DUMP_PREFIX_OFFSET and 1-byte mode:
  * 0009ab42: 40 41 42 43 44 45 46 47 48 49 4a 4b 4c 4d 4e 4f  @ABCDEFGHIJKLMNO
  * Example output using %DUMP_PREFIX_ADDRESS and 4-byte mode:
  * ffffffff88089af0: 73727170 77767574 7b7a7978 7f7e7d7c  pqrstuvwxyz{|}~.
  */
-void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
-		    int rowsize, int groupsize,
-		    const void *buf, size_t len, bool ascii)
+void print_hex_dump_ext(const char *level, const char *prefix_str,
+			int prefix_type, int rowsize, int groupsize,
+			const void *buf, size_t len, u32 flags)
 {
 	const u8 *ptr = buf;
-	int i, linelen, remaining = len;
+	int i, remaining = len;
 	unsigned char *linebuf;
 	unsigned int linebuf_len;
+	u8 skipped_val = 0;
+	size_t skipped = 0;
+
 
 	if (rowsize % groupsize)
 		rowsize -= rowsize % groupsize;
@@ -266,11 +309,35 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
 	}
 
 	for (i = 0; i < len; i += rowsize) {
-		linelen = min(remaining, rowsize);
+		int linelen = min(remaining, rowsize);
 		remaining -= rowsize;
 
+		if (flags & HEXDUMP_SUPPRESS_REPEATED) {
+			u8 new_val;
+
+			if (buf_is_all(ptr + i, linelen, &new_val)) {
+				if (new_val == skipped_val) {
+					skipped += linelen;
+					continue;
+				} else {
+					announce_skipped(level, prefix_str,
+							 skipped_val, skipped);
+					skipped_val = new_val;
+					skipped = linelen;
+					continue;
+				}
+			}
+		}
+
+		if (skipped) {
+			announce_skipped(level, prefix_str, skipped_val,
+					 skipped);
+			skipped = 0;
+		}
+
 		hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
-				   linebuf, linebuf_len, ascii);
+				   linebuf, linebuf_len,
+				   flags & HEXDUMP_ASCII);
 
 		switch (prefix_type) {
 		case DUMP_PREFIX_ADDRESS:
@@ -288,7 +355,7 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
 
 	kfree(linebuf);
 }
-EXPORT_SYMBOL(print_hex_dump);
+EXPORT_SYMBOL(print_hex_dump_ext);
 
 #if !defined(CONFIG_DYNAMIC_DEBUG)
 /**
@@ -306,8 +373,8 @@ EXPORT_SYMBOL(print_hex_dump);
 void print_hex_dump_bytes(const char *prefix_str, int prefix_type,
 			  const void *buf, size_t len)
 {
-	print_hex_dump(KERN_DEBUG, prefix_str, prefix_type, 16, 1,
-		       buf, len, true);
+	print_hex_dump_ext(KERN_DEBUG, prefix_str, prefix_type, 16, 1,
+		       buf, len, HEXDUMP_ASCII);
 }
 EXPORT_SYMBOL(print_hex_dump_bytes);
 #endif /* !defined(CONFIG_DYNAMIC_DEBUG) */
-- 
2.21.0


^ permalink raw reply related

* [PATCH v3 2/7] lib/hexdump.c: Relax rowsize checks in hex_dump_to_buffer
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>

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

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

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

Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
---
 lib/hexdump.c      | 48 ++++++++++++++++++++++++++++--------------
 lib/test_hexdump.c | 52 ++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 75 insertions(+), 25 deletions(-)

diff --git a/lib/hexdump.c b/lib/hexdump.c
index 81b70ed37209..3943507bc0e9 100644
--- a/lib/hexdump.c
+++ b/lib/hexdump.c
@@ -12,6 +12,7 @@
 #include <linux/errno.h>
 #include <linux/kernel.h>
 #include <linux/export.h>
+#include <linux/slab.h>
 #include <asm/unaligned.h>
 
 const char hex_asc[] = "0123456789abcdef";
@@ -80,14 +81,15 @@ EXPORT_SYMBOL(bin2hex);
  * hex_dump_to_buffer - convert a blob of data to "hex ASCII" in memory
  * @buf: data blob to dump
  * @len: number of bytes in the @buf
- * @rowsize: number of bytes to print per line; must be 16 or 32
+ * @rowsize: number of bytes to print per line; must be a multiple of groupsize
  * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1)
  * @linebuf: where to put the converted data
  * @linebuflen: total size of @linebuf, including space for terminating NUL
  * @ascii: include ASCII after the hex output
  *
- * hex_dump_to_buffer() works on one "line" of output at a time, i.e.,
- * 16 or 32 bytes of input data converted to hex + ASCII output.
+ * hex_dump_to_buffer() works on one "line" of output at a time, converting
+ * <groupsize> bytes of input to hexadecimal (and optionally printable ASCII)
+ * until <rowsize> bytes have been emitted.
  *
  * Given a buffer of u8 data, hex_dump_to_buffer() converts the input data
  * to a hex + ASCII dump at the supplied memory location.
@@ -116,16 +118,17 @@ int hex_dump_to_buffer(const void *buf, size_t len, int rowsize, int groupsize,
 	int ascii_column;
 	int ret;
 
-	if (rowsize != 16 && rowsize != 32)
-		rowsize = 16;
-
-	if (len > rowsize)		/* limit to one line at a time */
-		len = rowsize;
 	if (!is_power_of_2(groupsize) || groupsize > 8)
 		groupsize = 1;
 	if ((len % groupsize) != 0)	/* no mixed size output */
 		groupsize = 1;
 
+	if (rowsize % groupsize)
+		rowsize -= rowsize % groupsize;
+
+	if (len > rowsize)		/* limit to one line at a time */
+		len = rowsize;
+
 	ngroups = len / groupsize;
 	ascii_column = rowsize * 2 + rowsize / groupsize + 1;
 
@@ -216,7 +219,7 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
  *  caller supplies trailing spaces for alignment if desired
  * @prefix_type: controls whether prefix of an offset, address, or none
  *  is printed (%DUMP_PREFIX_OFFSET, %DUMP_PREFIX_ADDRESS, %DUMP_PREFIX_NONE)
- * @rowsize: number of bytes to print per line; must be 16 or 32
+ * @rowsize: number of bytes to print per line; must be a multiple of groupsize
  * @groupsize: number of bytes to print at a time (1, 2, 4, 8; default = 1)
  * @buf: data blob to dump
  * @len: number of bytes in the @buf
@@ -226,10 +229,9 @@ EXPORT_SYMBOL(hex_dump_to_buffer);
  * to the kernel log at the specified kernel log level, with an optional
  * leading prefix.
  *
- * print_hex_dump() works on one "line" of output at a time, i.e.,
- * 16 or 32 bytes of input data converted to hex + ASCII output.
  * print_hex_dump() iterates over the entire input @buf, breaking it into
- * "line size" chunks to format and print.
+ * lines of rowsize/groupsize groups of input data converted to hex +
+ * (optionally) ASCII output.
  *
  * E.g.:
  *   print_hex_dump(KERN_DEBUG, "raw data: ", DUMP_PREFIX_ADDRESS,
@@ -246,17 +248,29 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
 {
 	const u8 *ptr = buf;
 	int i, linelen, remaining = len;
-	unsigned char linebuf[32 * 3 + 2 + 32 + 1];
+	unsigned char *linebuf;
+	unsigned int linebuf_len;
 
-	if (rowsize != 16 && rowsize != 32)
-		rowsize = 16;
+	if (rowsize % groupsize)
+		rowsize -= rowsize % groupsize;
+
+	/* Worst case line length:
+	 * 2 hex chars + space per byte in, 2 spaces, 1 char per byte in, NULL
+	 */
+	linebuf_len = rowsize * 3 + 2 + rowsize + 1;
+	linebuf = kzalloc(linebuf_len, GFP_KERNEL);
+	if (!linebuf) {
+		printk("%s%shexdump: Could not alloc %u bytes for buffer\n",
+			level, prefix_str, linebuf_len);
+		return;
+	}
 
 	for (i = 0; i < len; i += rowsize) {
 		linelen = min(remaining, rowsize);
 		remaining -= rowsize;
 
 		hex_dump_to_buffer(ptr + i, linelen, rowsize, groupsize,
-				   linebuf, sizeof(linebuf), ascii);
+				   linebuf, linebuf_len, ascii);
 
 		switch (prefix_type) {
 		case DUMP_PREFIX_ADDRESS:
@@ -271,6 +285,8 @@ void print_hex_dump(const char *level, const char *prefix_str, int prefix_type,
 			break;
 		}
 	}
+
+	kfree(linebuf);
 }
 EXPORT_SYMBOL(print_hex_dump);
 
diff --git a/lib/test_hexdump.c b/lib/test_hexdump.c
index d78ddd62ffd0..6ab75a209b43 100644
--- a/lib/test_hexdump.c
+++ b/lib/test_hexdump.c
@@ -14,15 +14,25 @@ static const unsigned char data_b[] = {
 	'\x70', '\xba', '\xc4', '\x24', '\x7d', '\x83', '\x34', '\x9b',	/* 08 - 0f */
 	'\xa6', '\x9c', '\x31', '\xad', '\x9c', '\x0f', '\xac', '\xe9',	/* 10 - 17 */
 	'\x4c', '\xd1', '\x19', '\x99', '\x43', '\xb1', '\xaf', '\x0c',	/* 18 - 1f */
+	'\x00', '\x01', '\x02', '\x03', '\x04', '\x05', '\x06', '\x07', /* 20 - 27 */
+	'\x0f', '\x0e', '\x0d', '\x0c', '\x0b', '\x0a', '\x09', '\x08', /* 28 - 2f */
+	'\x10', '\x11', '\x12', '\x13', '\x14', '\x15', '\x16', '\x17', /* 30 - 37 */
+	'\x1f', '\x1e', '\x1d', '\x1c', '\x1b', '\x1a', '\x19', '\x18', /* 38 - 3f */
 };
 
-static const unsigned char data_a[] = ".2.{....p..$}.4...1.....L...C...";
+static const unsigned char data_a[] = ".2.{....p..$}.4...1.....L...C..."
+				      "................................";
 
 static const char * const test_data_1[] __initconst = {
 	"be", "32", "db", "7b", "0a", "18", "93", "b2",
 	"70", "ba", "c4", "24", "7d", "83", "34", "9b",
 	"a6", "9c", "31", "ad", "9c", "0f", "ac", "e9",
 	"4c", "d1", "19", "99", "43", "b1", "af", "0c",
+	"00", "01", "02", "03", "04", "05", "06", "07",
+	"0f", "0e", "0d", "0c", "0b", "0a", "09", "08",
+	"10", "11", "12", "13", "14", "15", "16", "17",
+	"1f", "1e", "1d", "1c", "1b", "1a", "19", "18",
+	NULL
 };
 
 static const char * const test_data_2_le[] __initconst = {
@@ -30,6 +40,11 @@ static const char * const test_data_2_le[] __initconst = {
 	"ba70", "24c4", "837d", "9b34",
 	"9ca6", "ad31", "0f9c", "e9ac",
 	"d14c", "9919", "b143", "0caf",
+	"0100", "0302", "0504", "0706",
+	"0e0f", "0c0d", "0a0b", "0809",
+	"1110", "1312", "1514", "1716",
+	"1e1f", "1c1d", "1a1b", "1819",
+	NULL
 };
 
 static const char * const test_data_2_be[] __initconst = {
@@ -37,26 +52,43 @@ static const char * const test_data_2_be[] __initconst = {
 	"70ba", "c424", "7d83", "349b",
 	"a69c", "31ad", "9c0f", "ace9",
 	"4cd1", "1999", "43b1", "af0c",
+	"0001", "0203", "0405", "0607",
+	"0f0e", "0d0c", "0b0a", "0908",
+	"1011", "1213", "1415", "1617",
+	"1f1e", "1d1c", "1b1a", "1918",
+	NULL
 };
 
 static const char * const test_data_4_le[] __initconst = {
 	"7bdb32be", "b293180a", "24c4ba70", "9b34837d",
 	"ad319ca6", "e9ac0f9c", "9919d14c", "0cafb143",
+	"03020100", "07060504", "0c0d0e0f", "08090a0b",
+	"13121110", "17161514", "1c1d1e1f", "18191a1b",
+	NULL
 };
 
 static const char * const test_data_4_be[] __initconst = {
 	"be32db7b", "0a1893b2", "70bac424", "7d83349b",
 	"a69c31ad", "9c0face9", "4cd11999", "43b1af0c",
+	"00010203", "04050607",	"0f0e0d0c", "0b0a0908",
+	"10111213", "14151617",	"1f1e1d1c", "1b1a1918",
+	NULL
 };
 
 static const char * const test_data_8_le[] __initconst = {
 	"b293180a7bdb32be", "9b34837d24c4ba70",
 	"e9ac0f9cad319ca6", "0cafb1439919d14c",
+	"0706050403020100", "08090a0b0c0d0e0f",
+	"1716151413121110", "18191a1b1c1d1e1f",
+	NULL
 };
 
 static const char * const test_data_8_be[] __initconst = {
 	"be32db7b0a1893b2", "70bac4247d83349b",
 	"a69c31ad9c0face9", "4cd1199943b1af0c",
+	"0001020304050607", "0f0e0d0c0b0a0908",
+	"1011121314151617", "1f1e1d1c1b1a1918",
+	NULL
 };
 
 #define FILL_CHAR	'#'
@@ -75,9 +107,6 @@ static void __init test_hexdump_prepare_test(size_t len, int rowsize,
 	unsigned int i;
 	const bool is_be = IS_ENABLED(CONFIG_CPU_BIG_ENDIAN);
 
-	if (rs != 16 && rs != 32)
-		rs = 16;
-
 	if (l > rs)
 		l = rs;
 
@@ -97,7 +126,12 @@ static void __init test_hexdump_prepare_test(size_t len, int rowsize,
 	p = test;
 	for (i = 0; i < l / gs; i++) {
 		const char *q = *result++;
-		size_t amount = strlen(q);
+		size_t amount;
+
+		if (!q)
+			break;
+
+		amount = strlen(q);
 
 		memcpy(p, q, amount);
 		p += amount;
@@ -120,7 +154,7 @@ static void __init test_hexdump_prepare_test(size_t len, int rowsize,
 	*p = '\0';
 }
 
-#define TEST_HEXDUMP_BUF_SIZE		(32 * 3 + 2 + 32 + 1)
+#define TEST_HEXDUMP_BUF_SIZE		(64 * 3 + 2 + 64 + 1)
 
 static void __init test_hexdump(size_t len, int rowsize, int groupsize,
 				bool ascii)
@@ -215,7 +249,7 @@ static void __init test_hexdump_overflow(size_t buflen, size_t len,
 static void __init test_hexdump_overflow_set(size_t buflen, bool ascii)
 {
 	unsigned int i = 0;
-	int rs = (get_random_int() % 2 + 1) * 16;
+	int rs = (get_random_int() % 4 + 1) * 16;
 
 	do {
 		int gs = 1 << i;
@@ -230,11 +264,11 @@ static int __init test_hexdump_init(void)
 	unsigned int i;
 	int rowsize;
 
-	rowsize = (get_random_int() % 2 + 1) * 16;
+	rowsize = (get_random_int() % 4 + 1) * 16;
 	for (i = 0; i < 16; i++)
 		test_hexdump_set(rowsize, false);
 
-	rowsize = (get_random_int() % 2 + 1) * 16;
+	rowsize = (get_random_int() % 4 + 1) * 16;
 	for (i = 0; i < 16; i++)
 		test_hexdump_set(rowsize, true);
 
-- 
2.21.0


^ permalink raw reply related

* Re: [PATCH v3 3/7] lib/hexdump.c: Optionally suppress lines of repeated bytes
From: Alastair D'Silva @ 2019-06-17  4:07 UTC (permalink / raw)
  To: Jani Nikula
  Cc: 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-4-alastair@au1.ibm.com>

On Mon, 2019-06-17 at 12:04 +1000, Alastair D'Silva wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
> 
> Some buffers may only be partially filled with useful data, while the
> rest
> is padded (typically with 0x00 or 0xff).
> 
> This patch introduces a flag to allow the supression of lines of
> repeated
> bytes, which are replaced with '** Skipped %u bytes of value 0x%x **'
> 
> An inline wrapper function is provided for backwards compatibility
> with
> existing code, which maintains the original behaviour.
> 
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> ---
>  include/linux/printk.h | 25 +++++++++---
>  lib/hexdump.c          | 91 ++++++++++++++++++++++++++++++++++++--
> ----
>  2 files changed, 99 insertions(+), 17 deletions(-)
> 
> diff --git a/include/linux/printk.h b/include/linux/printk.h
> index cefd374c47b1..d7754799cfe0 100644
> --- a/include/linux/printk.h
> +++ b/include/linux/printk.h
> @@ -481,13 +481,18 @@ enum {
>  	DUMP_PREFIX_ADDRESS,
>  	DUMP_PREFIX_OFFSET
>  };
> +
>  extern int hex_dump_to_buffer(const void *buf, size_t len, int
> rowsize,
>  			      int groupsize, char *linebuf, size_t
> linebuflen,
>  			      bool ascii);
> +
> +#define HEXDUMP_ASCII			BIT(0)
> +#define HEXDUMP_SUPPRESS_REPEATED	BIT(1)
> +

This is missing the include of linux/bits.h, I'll fix this in the next
version.

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



^ permalink raw reply

* Re: [PATCH v3] {nl,mac}80211: allow 4addr AP operation on crypto controlled devices
From: Stefan Lippers-Hollmann @ 2019-06-17  5:07 UTC (permalink / raw)
  To: Johannes Berg; +Cc: Tom Psyborg, Manikanta Pubbisetty, linux-wireless
In-Reply-To: <dc9039be42df8d241b14d4f673f3c472dc113991.camel@sipsolutions.net>

Hi

On 2019-06-12, Johannes Berg wrote:
> On Thu, 2019-06-06 at 20:41 +0200, Tom Psyborg wrote:
> >
> > Applying this patch instead of v1 broke WDS operation between two
> > Litebeam AC Gen2 devices:
>
> I'm confused, and not even sure which version I applied now.
>
> Manikanta, can you please check this and which version I have and which
> changes I might need?

I've tested (and left it running/ monitored) for two days without
any problems between QCA9984 (ZyXEL nbg6817/ ipq8065, WDS-AP, affected
before 33d915d9e8ce811d8958915ccd18d71a66c7c495 "{nl,mac}80211: allow
4addr AP operation on crypto controlled devices" went in) and AR9340
(TP-Link TL-WDR3600/ AR9344, WDS-Client, not affected by this issue),
both under current (~2 days old) OpenWrt master[1] (ipq806x/ ath79,
respectively). This patch is working fine and fixes the previous
problems with 4addr on ath10k (QCA9984).

Thank you a lot for looking into this!

Regards
	Stefan Lippers-Hollmann

--
[1]	Loading modules backported from Linux version v4.19.32-0-g3a2156c839c7
	Backport generated by backports.git v4.19.32-1-0-g1c4f7569
	with this version of the patch applied, instead of v1 as
	currently present in OpenWrt/ master HEAD.

^ permalink raw reply

* Re: [PATCH 5.2 1/2] mwifiex: Don't abort on small, spec-compliant vendor IEs
From: Takashi Iwai @ 2019-06-17  6:12 UTC (permalink / raw)
  To: Brian Norris
  Cc: Ganapathi Bhat, Nishant Sarmukadam, Amitkumar Karwar, Xinming Hu,
	linux-kernel, linux-wireless, Takashi Iwai, Guenter Roeck
In-Reply-To: <20190615001321.241808-1-briannorris@chromium.org>

On Sat, 15 Jun 2019 02:13:20 +0200,
Brian Norris wrote:
> 
> Per the 802.11 specification, vendor IEs are (at minimum) only required
> to contain an OUI. A type field is also included in ieee80211.h (struct
> ieee80211_vendor_ie) but doesn't appear in the specification. The
> remaining fields (subtype, version) are a convention used in WMM
> headers.
> 
> Thus, we should not reject vendor-specific IEs that have only the
> minimum length (3 bytes) -- we should skip over them (since we only want
> to match longer IEs, that match either WMM or WPA formats). We can
> reject elements that don't have the minimum-required 3 byte OUI.
> 
> While we're at it, move the non-standard subtype and version fields into
> the WMM structs, to avoid this confusion in the future about generic
> "vendor header" attributes.
> 
> Fixes: 685c9b7750bf ("mwifiex: Abort at too short BSS descriptor element")
> Cc: Takashi Iwai <tiwai@suse.de>
> Signed-off-by: Brian Norris <briannorris@chromium.org>
> ---
> It appears that commit 685c9b7750bf is on its way to 5.2, so I labeled
> this bugfix for 5.2 as well.

Thanks for catching this.
Reviewed-by: Takashi Iwai <tiwai@suse.de>


Takashi

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox