devicetree.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Christian Marangi <ansuelsmth@gmail.com>
To: "Rafał Miłecki" <rafal@milecki.pl>,
	"Srinivas Kandagatla" <srinivas.kandagatla@linaro.org>,
	"Rob Herring" <robh+dt@kernel.org>,
	"Krzysztof Kozlowski" <krzysztof.kozlowski+dt@linaro.org>,
	"Conor Dooley" <conor+dt@kernel.org>,
	devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: Christian Marangi <ansuelsmth@gmail.com>
Subject: [PATCH 3/3] nvmem: u-boot-env: Handle "reduced" ASCII address declaration
Date: Mon, 24 Jul 2023 10:26:32 +0200	[thread overview]
Message-ID: <20230724082632.21133-3-ansuelsmth@gmail.com> (raw)
In-Reply-To: <20230724082632.21133-1-ansuelsmth@gmail.com>

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@gmail.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


  parent reply	other threads:[~2023-07-24 15:32 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-07-24  8:26 [PATCH 1/3] dt-bindings: nvmem: u-boot,env: Add support for u-boot,env-size Christian Marangi
2023-07-24  8:26 ` [PATCH 2/3] nvmem: u-boot-env: Permit to declare custom env-size Christian Marangi
2023-07-24  8:26 ` Christian Marangi [this message]
2023-07-26 16:36 ` [PATCH 1/3] dt-bindings: nvmem: u-boot,env: Add support for u-boot,env-size Rob Herring
2023-07-27 19:04   ` Christian Marangi

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230724082632.21133-3-ansuelsmth@gmail.com \
    --to=ansuelsmth@gmail.com \
    --cc=conor+dt@kernel.org \
    --cc=devicetree@vger.kernel.org \
    --cc=krzysztof.kozlowski+dt@linaro.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rafal@milecki.pl \
    --cc=robh+dt@kernel.org \
    --cc=srinivas.kandagatla@linaro.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).