linux-kernel - [PATCH 3/3] nvmem: u-boot-env: Handle "reduced" ASCII address declaration

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <20230724082632.21133-3-ansuelsmth@gmail.com>
Date:   Mon, 24 Jul 2023 10:26:32 +0200
From:   Christian Marangi <ansuelsmth@...il.com>
To:     Rafał Miłecki <rafal@...ecki.pl>,
        Srinivas Kandagatla <srinivas.kandagatla@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Conor Dooley <conor+dt@...nel.org>, devicetree@...r.kernel.org,
        linux-kernel@...r.kernel.org
Cc:     Christian Marangi <ansuelsmth@...il.com>
Subject: [PATCH 3/3] nvmem: u-boot-env: Handle "reduced" ASCII address declaration

Parsing ASCII values is hard as hex values can be declared in many ways
and OEM never follow a genera rules. This is especially problematic for
parsing MAC address writte in ASCII format where an hex values can be
written in 2 different format:
- a Full format with leading 0, 01 02 06 0A
- a Reduced format with trimmed leading 0, 1 2 6 A

The current U-Boot-env driver assume that the Full format is always used
by parsting the NVMEM cell size and expect always the form of
XX:XX:XX:XX:XX:XX to pass it directly to mac_pton that expects this
format.

Reality is that it's common for OEM to use the reduced format resulting
in MAC address in the format of XX:a:XX:XX:XX:XX:XX or a:XX:XX:XX:XX:XX
or XX:XX:XX:XX:XX:a.

To handle these special format additional care is needed.

Some additional logic are added to "normalize" the MAC address in ASCII
to a format that is accepted by mac_pton.

As using the NVMEM cell size is not enough anymore, some additional
validation are done by checking if we have at least a format of
X:X:X:X:X:X by checking if we have at least 5 ':' char while checking
the NVMEM cell. The size validation is changed to check a range of
ETH_ALEN + 5 (assuming a min valid MAC address of X:X:X:X:X:X) and
the full form by checking 3 * ETH_ALEN -1 (for the full format
XX:XX:XX:XX:XX:XX)

The parsing function try to be as redable as possible while saving some
code duplication and skip the use of memcpy function as the post_process
is called very little time just on driver probe.

Signed-off-by: Christian Marangi <ansuelsmth@...il.com>
---
 drivers/nvmem/u-boot-env.c | 50 ++++++++++++++++++++++++++++++++++++--
 1 file changed, 48 insertions(+), 2 deletions(-)

diff --git a/drivers/nvmem/u-boot-env.c b/drivers/nvmem/u-boot-env.c
index b64c37308789..e33565e872f8 100644
--- a/drivers/nvmem/u-boot-env.c
+++ b/drivers/nvmem/u-boot-env.c
@@ -76,12 +76,58 @@ static int u_boot_env_read(void *context, unsigned int offset, void *val,
 static int u_boot_env_read_post_process_ethaddr(void *context, const char *id, int index,
 						unsigned int offset, void *buf, size_t bytes)
 {
+	u8 *src_mac = buf;
 	u8 mac[ETH_ALEN];
 
-	if (bytes != 3 * ETH_ALEN - 1)
+	if (bytes < ETH_ALEN + 5 || bytes > 3 * ETH_ALEN - 1)
 		return -EINVAL;
 
-	if (!mac_pton(buf, mac))
+	/* ASCII address might need to be normalized, try to parse it */
+	if (bytes != 3 * ETH_ALEN - 1) {
+		u8 tmp_mac[3 * ETH_ALEN - 1];
+		int i = 0, j = 0;
+
+		while (i < bytes) {
+			/* First check if we need to handle a reduced section
+			 * or we are handling the last byte.
+			 * Example of reduced section:
+			 * - a:XX:XX:XX:XX:XX
+			 * - XX:a:XX:XX:XX:XX
+			 * - XX:XX:XX:XX:XX:a
+			 */
+			if (src_mac[i + 1] == ':' || i + 1 == bytes) {
+				tmp_mac[j] = '0';
+				tmp_mac[j + 1] = src_mac[i];
+				if (src_mac[i + 1] == ':')
+					tmp_mac[j + 2] = src_mac[i + 1];
+				i += 2;
+			/* Handle a full section or we are handling the last 2 byte.
+			 * Example of full section:
+			 * - aa:XX:XX:XX:XX:XX
+			 * - XX:aa:XX:XX:XX:XX
+			 * - XX:XX:XX:XX:XX:aa
+			 */
+			} else if (src_mac[i + 2] == ':' || i + 2 == bytes) {
+				tmp_mac[j] = src_mac[i];
+				tmp_mac[j + 1] = src_mac[i + 1];
+				if (src_mac[i + 2] == ':')
+					tmp_mac[j + 2] = src_mac[i + 2];
+				i += 3;
+			/* Return -EINVAL if we have a not valid ascii address.
+			 * We can use the logic of missing ':' to validate a
+			 * correct ASCII address.
+			 */
+			} else {
+				return -EINVAL;
+			}
+			j += 3;
+		}
+
+		/* Parse the normalized mac instead of the nvmem cell */
+		src_mac = tmp_mac;
+	}
+
+	if (!mac_pton(src_mac, mac))
 		return -EINVAL;
 
 	if (index)
-- 
2.40.1