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

["Luck, Tony" <tony.luck@intel.com>]

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.

> +				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;
> +					}
> +					count++;
> +					if (pos >= str_len)
> +						break;
> +				}
> +				v5param->einjv2_struct.component_arr_count = count;
> +
> +				/* clear buffer after user input for next injection */
> +				memset(user_input, 0, COMP_ARR_SIZE);
> +			} else {

-Tony