Messages in this thread | | | Date | Wed, 27 Jul 2022 10:25:04 +0200 | From | Jean Delvare <> | Subject | Re: [PATCH v1 1/1] firmware: dmi: Don't take garbage into consideration in dmi_smbios3_present() |
| |
Hi Andy,
On Tue, 26 Jul 2022 12:43:29 +0300, Andy Shevchenko wrote: > The byte at offset 6 represent length. Don't take it and drop it immediately > by using proper accessor, i.e. get_unaligned_be24().
The subject sounds like you are fixing a bug, while this is only, at best, a minor optimization.
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com> > --- > drivers/firmware/dmi_scan.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/firmware/dmi_scan.c b/drivers/firmware/dmi_scan.c > index b2ea318a10a4..24537ce29bc4 100644 > --- a/drivers/firmware/dmi_scan.c > +++ b/drivers/firmware/dmi_scan.c > @@ -630,7 +630,7 @@ static int __init dmi_smbios3_present(const u8 *buf) > { > if (memcmp(buf, "_SM3_", 5) == 0 && > buf[6] < 32 && dmi_checksum(buf, buf[6])) { > - dmi_ver = get_unaligned_be32(buf + 6) & 0xFFFFFF; > + dmi_ver = get_unaligned_be24(buf + 7); > dmi_num = 0; /* No longer specified */ > dmi_len = get_unaligned_le32(buf + 12); > dmi_base = get_unaligned_le64(buf + 16);
I admit I did not know about get_unaligned_be24(). While I agree that it makes the source code look better, one downside is that it actually increases the binary size on x86_64. The reason is that get_unaligned_be32() is optimized by assembly instruction bswapl, while get_unaligned_be24() is not. Situation appears to be the same on ia64 and arm. Only arm64 would apparently benefit from your proposed change.
I'm not too sure what is preferred in such situations.
-- Jean Delvare SUSE L3 Support
| |