All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Maciej S. Szmigiero" <mail@maciej.szmigiero.name>
To: Borislav Petkov <bp@alien8.de>
Cc: Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, "H. Peter Anvin" <hpa@zytor.com>,
	x86@kernel.org, linux-kernel@vger.kernel.org
Subject: [PATCH v7 5/9] x86/microcode/AMD: Split status from data to skip in verify_and_add_patch()
Date: Tue, 19 Jun 2018 20:47:35 +0200	[thread overview]
Message-ID: <84b3976f5879d7a58c3774d1f86e493b34c9759c.1529424596.git.mail@maciej.szmigiero.name> (raw)
In-Reply-To: <cover.1529424596.git.mail@maciej.szmigiero.name>

verify_and_add_patch() returned a single "int" value which encoded both
this function error status and also a length of microcode container data to
skip.

Unfortunately, ranges of these two values collide: the length of data to
skip can be any value between 1 and UINT_MAX, so, for example, error status
of -EINVAL maps to a valid return value of 4294967274 bytes.
That's why these two values need to be split.

Let's keep the common convention that a function zero return value means
success while a negative value means an error while moving the returned
length of microcode container data to skip to a separate output parameter.

Signed-off-by: Maciej S. Szmigiero <mail@maciej.szmigiero.name>
---
 arch/x86/kernel/cpu/microcode/amd.c | 35 +++++++++++++++--------------
 1 file changed, 18 insertions(+), 17 deletions(-)

diff --git a/arch/x86/kernel/cpu/microcode/amd.c b/arch/x86/kernel/cpu/microcode/amd.c
index c05531540b57..9e29374ce4d0 100644
--- a/arch/x86/kernel/cpu/microcode/amd.c
+++ b/arch/x86/kernel/cpu/microcode/amd.c
@@ -754,45 +754,46 @@ static void cleanup(void)
 }
 
 /*
- * We return the current size even if some of the checks failed so that
- * we can skip over the next patch. If we return a negative value, we
- * signal a grave error like a memory allocation has failed and the
- * driver cannot continue functioning normally. In such cases, we tear
- * down everything we've used up so far and exit.
+ * We return zero (success) and the current patch data size in @crnt_size
+ * even if some of the checks failed so that we can skip over the next patch.
+ * If we return a negative value, we signal a grave error like a memory
+ * allocation has failed and the driver cannot continue functioning normally.
+ * In such cases, we tear down everything we've used up so far and exit.
  */
-static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
+static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover,
+				unsigned int *crnt_size)
 {
 	struct microcode_header_amd *mc_hdr;
 	struct ucode_patch *patch;
-	unsigned int patch_size, crnt_size;
+	unsigned int patch_size;
 	u32 proc_fam;
 	u16 proc_id;
 
 	patch_size  = *(u32 *)(fw + 4);
-	crnt_size   = patch_size + SECTION_HDR_SIZE;
+	*crnt_size  = patch_size + SECTION_HDR_SIZE;
 	mc_hdr	    = (struct microcode_header_amd *)(fw + SECTION_HDR_SIZE);
 	proc_id	    = mc_hdr->processor_rev_id;
 
 	proc_fam = find_cpu_family_by_equiv_cpu(proc_id);
 	if (!proc_fam) {
 		pr_err("No patch family for equiv ID: 0x%04x\n", proc_id);
-		return crnt_size;
+		return 0;
 	}
 
 	/* check if patch is for the current family */
 	proc_fam = ((proc_fam >> 8) & 0xf) + ((proc_fam >> 20) & 0xff);
 	if (proc_fam != family)
-		return crnt_size;
+		return 0;
 
 	if (mc_hdr->nb_dev_id || mc_hdr->sb_dev_id) {
 		pr_err("Patch-ID 0x%08x: chipset-specific code unsupported.\n",
 			mc_hdr->patch_id);
-		return crnt_size;
+		return 0;
 	}
 
-	if (!verify_patch(family, fw, leftover, &crnt_size, false)) {
+	if (!verify_patch(family, fw, leftover, crnt_size, false)) {
 		pr_err("Patch-ID 0x%08x: size mismatch.\n", mc_hdr->patch_id);
-		return crnt_size;
+		return 0;
 	}
 
 	patch = kzalloc(sizeof(*patch), GFP_KERNEL);
@@ -818,7 +819,7 @@ static int verify_and_add_patch(u8 family, u8 *fw, unsigned int leftover)
 	/* ... and add to cache. */
 	update_cache(patch);
 
-	return crnt_size;
+	return 0;
 }
 
 static enum ucode_state __load_microcode_amd(u8 family, const u8 *data,
@@ -827,7 +828,6 @@ static enum ucode_state __load_microcode_amd(u8 family, const u8 *data,
 	enum ucode_state ret = UCODE_ERROR;
 	unsigned int leftover;
 	u8 *fw = (u8 *)data;
-	int crnt_size = 0;
 	int offset;
 
 	offset = install_equiv_cpu_table(data);
@@ -845,8 +845,9 @@ static enum ucode_state __load_microcode_amd(u8 family, const u8 *data,
 	}
 
 	while (leftover) {
-		crnt_size = verify_and_add_patch(family, fw, leftover);
-		if (crnt_size < 0)
+		unsigned int crnt_size;
+
+		if (verify_and_add_patch(family, fw, leftover, &crnt_size) < 0)
 			return ret;
 
 		fw	 += crnt_size;

  parent reply	other threads:[~2018-06-19 18:47 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-06-19 18:47 [PATCH v7 0/9] x86/microcode/AMD: Check microcode file sanity before loading it Maciej S. Szmigiero
2018-06-19 18:47 ` [PATCH v7 1/9] x86/microcode/AMD: Subtract SECTION_HDR_SIZE from file leftover length Maciej S. Szmigiero
2018-11-19 10:12   ` [tip:x86/microcode] " tip-bot for Maciej S. Szmigiero
2018-06-19 18:47 ` [PATCH v7 2/9] x86/microcode/AMD: Add microcode container data checking functions Maciej S. Szmigiero
2018-11-19 10:13   ` [tip:x86/microcode] x86/microcode/AMD: Add microcode container verification tip-bot for Maciej S. Szmigiero
2018-06-19 18:47 ` [PATCH v7 3/9] x86/microcode/AMD: Integrate verify_patch_size() into verify_patch() Maciej S. Szmigiero
2018-06-21  8:36   ` Borislav Petkov
2018-06-22 22:32     ` [PATCH 1/2] " Maciej S. Szmigiero
2018-06-22 22:33     ` [PATCH 2/2] x86/microcode/AMD: Check patch size for all known CPU families Maciej S. Szmigiero
2018-06-25 18:37       ` Borislav Petkov
2018-06-25 22:18         ` Maciej S. Szmigiero
2018-06-19 18:47 ` [PATCH v7 4/9] x86/microcode/AMD: Check microcode container data in the early loader Maciej S. Szmigiero
2018-06-19 18:47 ` Maciej S. Szmigiero [this message]
2018-06-19 18:47 ` [PATCH v7 6/9] x86/microcode/AMD: Check microcode container data in the late loader Maciej S. Szmigiero
2018-06-19 18:47 ` [PATCH v7 7/9] x86/microcode/AMD: Add a reminder about PATCH_MAX_SIZE macro Maciej S. Szmigiero
2018-06-19 18:47 ` [PATCH v7 8/9] x86/microcode/AMD: Convert CPU equivalence table variable into a struct Maciej S. Szmigiero
2018-06-19 18:47 ` [PATCH v7 9/9] x86/microcode/AMD: Check the equivalence table size when scanning it Maciej S. Szmigiero

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=84b3976f5879d7a58c3774d1f86e493b34c9759c.1529424596.git.mail@maciej.szmigiero.name \
    --to=mail@maciej.szmigiero.name \
    --cc=bp@alien8.de \
    --cc=hpa@zytor.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@redhat.com \
    --cc=tglx@linutronix.de \
    --cc=x86@kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.