From: Alex Williamson <email@example.com> To: Christoph Hellwig <firstname.lastname@example.org> Cc: email@example.com, firstname.lastname@example.org, email@example.com, Yuan Yao <firstname.lastname@example.org> Subject: Re: [PATCH] vfio/pci: Sanity check IGD OpRegion Size Date: Thu, 13 May 2021 08:43:59 -0600 [thread overview] Message-ID: <email@example.com> (raw) In-Reply-To: <YJjO3n6jWYDoYPAo@infradead.org> On Mon, 10 May 2021 07:12:46 +0100 Christoph Hellwig <firstname.lastname@example.org> wrote: > On Fri, May 07, 2021 at 12:53:17PM -0600, Alex Williamson wrote: > > + /* > > + * The OpRegion size field is specified as size in KB, but there have been > > + * user reports where this field appears to report size in bytes. If we > > + * read 8192, assume this is the case. > > + */ > > Please avoid pointlesly spilling the comment line over 80 chars. Oops, I didn't notice I was using a wider terminal. Fixed. > > + if (size == OPREGION_SIZE) > > Shouldn't this be a range tests, i.e. >= ? My concern here is how far we go down the path of trying to figure out what a sane size range is for this table an how/if we try to assume the BIOS intentions. The precise value of 8192 is not only absurdly large, but happens to coincide with the default table size, so it seems likely that we can infer this specific misinterpretation. If the BIOS has used a different value, suggesting they're trying to do something more extensive than a basic implementation, but still managed to botch the units for the size field, we should probably disregard it entirely. We can probably do that for smaller values as well, but I don't know where the line between reasonable and absurd is crossed. Would it make more sense to export e820__get_entry_type() so that we can validate that the full range of the table fits within an e820 mapping, which I understand should be ACPI NVS in this case? Thanks, Alex
prev parent reply other threads:[~2021-05-13 14:44 UTC|newest] Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top 2021-05-07 18:53 Alex Williamson 2021-05-08 4:50 ` Zhenyu Wang 2021-05-10 1:10 ` Yuan Yao 2021-05-10 1:34 ` Alex Williamson 2021-05-10 3:14 ` Yuan Yao 2021-05-10 6:12 ` Christoph Hellwig 2021-05-13 14:43 ` Alex Williamson [this message]
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 \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --firstname.lastname@example.org \ --email@example.com \ --subject='Re: [PATCH] vfio/pci: Sanity check IGD OpRegion Size' \ /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
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).