linux-kernel - Re: [PATCH v6 8/9] ACPI: APEI: EINJ: Enable EINJv2 error injections

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <aAfyyS9qUgzsQeLY@zaid-VirtualBox>
Date: Tue, 22 Apr 2025 12:49:29 -0700
From: Zaid Alali <zaidal@...amperecomputing.com>
To: "Luck, Tony" <tony.luck@...el.com>
Cc: rafael@...nel.org, lenb@...nel.org, james.morse@....com, bp@...en8.de,
	robert.moore@...el.com, Jonathan.Cameron@...wei.com,
	ira.weiny@...el.com, Benjamin.Cheatham@....com,
	dan.j.williams@...el.com, arnd@...db.de, Avadhut.Naik@....com,
	u.kleine-koenig@...gutronix.de, john.allen@....com,
	linux-acpi@...r.kernel.org, linux-kernel@...r.kernel.org,
	acpica-devel@...ts.linux.dev
Subject: Re: [PATCH v6 8/9] ACPI: APEI: EINJ: Enable EINJv2 error injections

On Fri, Apr 18, 2025 at 01:01:55PM -0700, Luck, Tony wrote:
> On Thu, Apr 17, 2025 at 03:00:18PM -0700, Zaid Alali wrote:
> > +			if (is_V2) {
> > +				int count = 0, bytes_read, pos = 0, nr_parsed = 0, str_len;
> > +				unsigned int comp, synd;
> 
> I've been staring at the ACPI spec to try and figure out how many bits
> are needed for comp and sync. The example in section 18.6.7 "Error
> Injection Version 2 Operation" has this in step 6:
> 
> Component Syndrome Array [0] = { 00000000000000000000000000000004 , 000000000000000000000000A5A5A5A5 }
> 
> Which really looks like 128-bit values!
> 
> So are "unsigned int" adequate? Even "u64" looks like it would fall short.

Hi Tony,

Hi Tony,

Thats a great point! I beleive I need to change "struct syndrom_array" to make all union members have
a 128-bit size

struct syndrome_array {
        union {
                u32     acpi_id;      ====> all should be 128-bit long
                u32     device_id;
                u32     pcie_sbdf;
                u8      vendor_id[16];
        } comp_id;
        union {
                u32     proc_synd;
                u32     mem_synd;
                u32     pcie_synd;
                u8      vendor_synd[16];
        } comp_synd;
};


> 
> > +				struct syndrome_array *component_arr;
> > +
> > +				component_arr = v5param->einjv2_struct.component_arr;
> > +				str_len = strlen(user_input);
> > +
> > +				while ((nr_parsed = sscanf(user_input + pos, "%x %x\n%n", &comp,
> 
> Parsing user input with sscanf() is a bit fragile.  Take a look at
> rdtgroup_schemata_write() which uses:
> 
> 	while ((tok = strsep(&buf, "\n")) != NULL) {
> 
> to split input into lines, and then strim() and strsep() to break
> up items within a line.
> 
> > +					&synd, &bytes_read))) {
> > +					pos += bytes_read;
> > +
> > +					if (nr_parsed != 2) {
> > +						kfree(v5param);
> > +						goto err_out;
> > +					}
> > +					if (count >= nr_components) {
> > +						kfree(v5param);
> > +						goto err_out;
> > +					}
> > +
> > +					switch (type) {
> > +					case EINJV2_PROCESSOR_ERROR:
> > +						component_arr[count].comp_id.acpi_id = comp;
> > +						component_arr[count].comp_synd.proc_synd = synd;
> > +						break;
> > +					case EINJV2_MEMORY_ERROR:
> > +						component_arr[count].comp_id.device_id = comp;
> > +						component_arr[count].comp_synd.mem_synd = synd;
> > +						break;
> > +					case EINJV2_PCIE_ERROR:
> > +						component_arr[count].comp_id.pcie_sbdf = comp;
> > +						component_arr[count].comp_synd.pcie_synd = synd;
> > +						break;

I also need to include another case here for EINJv2_VENDOR_ERROR
I will fix this in the next revision.

-Zaid

> > +					}
> > +					count++;
> > +					if (pos >= str_len)
> > +						break;
> > +				}