Xen-Devel Archive on lore.kernel.org
 help / color / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: Chao Gao <chao.gao@intel.com>
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	xen-devel@lists.xenproject.org, "Ashok Raj" <ashok.raj@intel.com>,
	"Wei Liu" <wl@xen.org>, "Roger Pau Monné" <roger.pau@citrix.com>
Subject: Re: [Xen-devel] [PATCH v9 01/15] microcode/intel: extend microcode_update_match()
Date: Thu, 29 Aug 2019 09:14:55 +0200
Message-ID: <60fc7674-0839-f4a9-5928-26d1dc0e36b9@suse.com> (raw)
In-Reply-To: <20190829071521.GA12650@gao-cwp>

On 29.08.2019 09:15, Chao Gao wrote:
> On Wed, Aug 28, 2019 at 05:12:34PM +0200, Jan Beulich wrote:
>> On 19.08.2019 03:25, Chao Gao wrote:
>>> --- a/xen/arch/x86/microcode_intel.c
>>> +++ b/xen/arch/x86/microcode_intel.c
>>> @@ -134,14 +134,39 @@ static int collect_cpu_info(unsigned int cpu_num, struct cpu_signature *csig)
>>>      return 0;
>>>  }
>>>  
>>> -static inline int microcode_update_match(
>>> -    unsigned int cpu_num, const struct microcode_header_intel *mc_header,
>>> -    int sig, int pf)
>>> +/* Check an update against the CPU signature and current update revision */
>>> +static enum microcode_match_result microcode_update_match(
>>> +    const struct microcode_header_intel *mc_header, unsigned int cpu)
>>>  {
>>> -    struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu_num);
>>> -
>>> -    return (sigmatch(sig, uci->cpu_sig.sig, pf, uci->cpu_sig.pf) &&
>>> -            (mc_header->rev > uci->cpu_sig.rev));
>>> +    const struct extended_sigtable *ext_header;
>>> +    const struct extended_signature *ext_sig;
>>> +    unsigned int i;
>>> +    struct ucode_cpu_info *uci = &per_cpu(ucode_cpu_info, cpu);
>>> +    unsigned int sig = uci->cpu_sig.sig;
>>> +    unsigned int pf = uci->cpu_sig.pf;
>>> +    unsigned int rev = uci->cpu_sig.rev;
>>> +    unsigned long data_size = get_datasize(mc_header);
>>> +    const void *end = (const void *)mc_header + get_totalsize(mc_header);
>>> +
>>> +    if ( sigmatch(sig, mc_header->sig, pf, mc_header->pf) )
>>> +        return (mc_header->rev > rev) ? NEW_UCODE : OLD_UCODE;
>>
>> Didn't you lose a range check against "end" ahead of this if()?
>> get_totalsize() and get_datasize() aiui also would need to live
>> after a range check, just a sizeof() (i.e. MC_HEADER_SIZE) based
>> one. This would also affect the caller as it seems.
> 
> I think microcode_sanity_check() is for this purpose. We can do
> sanity check before the if(). Perhaps, we can just add an assertion
> that sanity check won't fail. Because whenever sanity check failed
> when pasing an ucode blob, we just drop the ucode; we won't pass an
> broken ucode to this function.

Well - that's the main question. The purpose of this patch, after all,
is (aiui) to allow calling the function in more cases. If all callers
are indeed supposed to check the basic properties, then yes, an ASSERT()
would be fine.

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

  reply index

Thread overview: 57+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-08-19  1:25 [Xen-devel] [PATCH v9 00/15] improve late microcode loading Chao Gao
2019-08-19  1:25 ` [Xen-devel] [PATCH v9 01/15] microcode/intel: extend microcode_update_match() Chao Gao
2019-08-28 15:12   ` Jan Beulich
2019-08-29  7:15     ` Chao Gao
2019-08-29  7:14       ` Jan Beulich [this message]
2019-08-19  1:25 ` [Xen-devel] [PATCH v9 02/15] microcode/amd: fix memory leak Chao Gao
2019-08-19  1:25 ` [Xen-devel] [PATCH v9 03/15] microcode/amd: distinguish old and mismatched ucode in microcode_fits() Chao Gao
2019-08-19  1:25 ` [Xen-devel] [PATCH v9 04/15] microcode: introduce a global cache of ucode patch Chao Gao
2019-08-22 11:11   ` Roger Pau Monné
2019-08-28 15:21   ` Jan Beulich
2019-08-29 10:18   ` Jan Beulich
2019-08-19  1:25 ` [Xen-devel] [PATCH v9 05/15] microcode: clean up microcode_resume_cpu Chao Gao
2019-08-19  1:25 ` [Xen-devel] [PATCH v9 06/15] microcode: remove struct ucode_cpu_info Chao Gao
2019-08-19  1:25 ` [Xen-devel] [PATCH v9 07/15] microcode: remove pointless 'cpu' parameter Chao Gao
2019-08-19  1:25 ` [Xen-devel] [PATCH v9 08/15] microcode/amd: call svm_host_osvw_init() in common code Chao Gao
2019-08-22 13:08   ` Roger Pau Monné
2019-08-28 15:26   ` Jan Beulich
2019-08-19  1:25 ` [Xen-devel] [PATCH v9 09/15] microcode: pass a patch pointer to apply_microcode() Chao Gao
2019-08-19  1:25 ` [Xen-devel] [PATCH v9 10/15] microcode: split out apply_microcode() from cpu_request_microcode() Chao Gao
2019-08-22 13:59   ` Roger Pau Monné
2019-08-29 10:06     ` Jan Beulich
2019-08-30  3:22       ` Chao Gao
2019-08-30  7:25         ` Jan Beulich
2019-08-29 10:19   ` Jan Beulich
2019-08-19  1:25 ` [Xen-devel] [PATCH v9 11/15] microcode: unify loading update during CPU resuming and AP wakeup Chao Gao
2019-08-22 14:10   ` Roger Pau Monné
2019-08-22 16:44     ` Chao Gao
2019-08-23  9:09       ` Roger Pau Monné
2019-08-29  7:37         ` Chao Gao
2019-08-29  8:16           ` Roger Pau Monné
2019-08-29 10:26           ` Jan Beulich
2019-08-29 10:29   ` Jan Beulich
2019-08-19  1:25 ` [Xen-devel] [PATCH v9 12/15] microcode: reduce memory allocation and copy when creating a patch Chao Gao
2019-08-23  8:11   ` Roger Pau Monné
2019-08-26  7:03     ` Chao Gao
2019-08-26  8:11       ` Roger Pau Monné
2019-08-29 10:47   ` Jan Beulich
2019-08-19  1:25 ` [Xen-devel] [PATCH v9 13/15] x86/microcode: Synchronize late microcode loading Chao Gao
2019-08-19 10:27   ` Sergey Dyasli
2019-08-19 14:49     ` Chao Gao
2019-08-29 12:06   ` Jan Beulich
2019-08-30  3:30     ` Chao Gao
2019-08-19  1:25 ` [Xen-devel] [PATCH v9 14/15] microcode: remove microcode_update_lock Chao Gao
2019-08-19  1:25 ` [Xen-devel] [PATCH v9 15/15] microcode: block #NMI handling when loading an ucode Chao Gao
2019-08-23  8:46   ` Sergey Dyasli
2019-08-26  8:07     ` Chao Gao
2019-08-27  4:52       ` Chao Gao
2019-08-28  8:52         ` Sergey Dyasli
2019-08-29 12:11         ` Jan Beulich
2019-08-30  6:35           ` Chao Gao
2019-09-09  5:52             ` Chao Gao
2019-09-09  6:16               ` Jan Beulich
2019-08-29 12:22   ` Jan Beulich
2019-08-30  6:33     ` Chao Gao
2019-08-30  7:30       ` Jan Beulich
2019-08-22  7:51 ` [Xen-devel] [PATCH v9 00/15] improve late microcode loading Sergey Dyasli
2019-08-22 15:39   ` Chao Gao

Reply instructions:

You may reply publically 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=60fc7674-0839-f4a9-5928-26d1dc0e36b9@suse.com \
    --to=jbeulich@suse.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=ashok.raj@intel.com \
    --cc=chao.gao@intel.com \
    --cc=roger.pau@citrix.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.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

Xen-Devel Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/xen-devel/0 xen-devel/git/0.git
	git clone --mirror https://lore.kernel.org/xen-devel/1 xen-devel/git/1.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 xen-devel xen-devel/ https://lore.kernel.org/xen-devel \
		xen-devel@lists.xenproject.org xen-devel@lists.xen.org xen-devel@archiver.kernel.org
	public-inbox-index xen-devel

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.xenproject.lists.xen-devel


AGPL code for this site: git clone https://public-inbox.org/ public-inbox