[PATCH RFC v3] brcmfmac: sanitize DMI strings

[PATCH RFC v3] brcmfmac: sanitize DMI strings

[Victor Bravo]

Hi,

as I expect to be busy tomorrow, I'm sending polished, hopefully more
readable version of the patch, and also some more implementation details
which can be useful in the search for proper sanitizing function.

Patch description:
Sanitize DMI strings in brcmfmac driver to make resulting filenames
contain only safe characters. This version replaces all non-printable
characters incl. delete (1-31, 127-255), spaces and slashes with
underscores.

This change breaks backward compatibility, but adds control over strings
passed to firmware loader and compatibility with CONFIG_EXTRA_FIRMWARE
which doesn't support spaces in filenames.

Implementation details:
The algorithm for sanitizing function was selected after following
choices were considered (C = number of allowed characters , N = number
of characters in sanitized strings):

ALGORITHM	TIME COMPL.	DATA	DISADVANTAGES
bitmask-based	O(N)		16	no macro to make mask from string
hardcoded-check	O(N)		0	not universal, big / prone to growth
runtime-mask	O(C+N)		16+C	C+N and 16+C don't seem "best" enough
string-based	O(C*N)		C	slow, big, far from "best possible"

(runtime-mask means bitmask-based algorithm with runtime mask
initialization from string of allowed characters)

Bitmask-based implementation won due to results summarized above.
It's only 7-bit clear, because this allows to save 16 bytes of data
at the price of single (char < 0) comparison, and C char is kind
of 7-bit anyway.

Known issues:
Function like brcmf_dmi_sanitize() shouldn't be part of driver, as
this practice leads to duplicate code, which is not acceptable for many
good reasons. An existing function from lib/ should be used instead,
or implemented there if no suitable replacement is found.
---
Changes from v1: don't revert fresh commit by someone else
Changes from v2: outfactor check to BRCMF_DMI_SANE_CHAR in an attempt to
		 make the code more readable, improve comments, +Luis

Signed-off-by: Victor Bravo <1905@spmblk.com>

diff --git a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c
index 7535cb0d4ac0..c8cb9c0b3f6e 100644
--- a/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c
+++ b/drivers/net/wireless/broadcom/brcm80211/brcmfmac/dmi.c
@@ -23,6 +23,18 @@
 /* The DMI data never changes so we can use a static buf for this */
 static char dmi_board_type[128];
 
+/*
+ * Array of 128 bits representing 7-bit characters allowed in DMI strings,
+ * byte-based to avoid endianess issues.
+ */
+static unsigned char brcmf_dmi_allowed_chars[] = {
+	0x00, 0x00, 0x00, 0x00, 0xfe, 0x7f, 0xff, 0xff,
+	0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0x7f
+};
+
+/* Safe character to replace disallowed ones. */
+#define BRCMF_DMI_SAFE_CHAR '_'
+
 struct brcmf_dmi_data {
 	u32 chip;
 	u32 chiprev;
@@ -99,6 +111,19 @@ static const struct dmi_system_id dmi_platform_data[] = {
 	{}
 };
 
+/* Checks character value agains bitmask of allowed characters */
+#define BRCMF_DMI_SANE_CHAR(mask, value) \
+	(mask[value / 8] & (1 << (value % 8)))
+
+void brcmf_dmi_sanitize(char *dest, const unsigned char *allowed, char safe)
+{
+	while (*dest) {
+		if ((*dest < 0) || !BRCMF_DMI_SANE_CHAR(allowed, *dest))
+			*dest = safe;
+		dest++;
+	}
+}
+
 void brcmf_dmi_probe(struct brcmf_mp_device *settings, u32 chip, u32 chiprev)
 {
 	const struct dmi_system_id *match;
@@ -126,6 +151,9 @@ void brcmf_dmi_probe(struct brcmf_mp_device *settings, u32 chip, u32 chiprev)
 	if (sys_vendor && product_name) {
 		snprintf(dmi_board_type, sizeof(dmi_board_type), "%s-%s",
 			 sys_vendor, product_name);
+		brcmf_dmi_sanitize(dmi_board_type,
+				   brcmf_dmi_allowed_chars,
+				   BRCMF_DMI_SAFE_CHAR);
 		settings->board_type = dmi_board_type;
 	}
 }