[Skiboot] [PATCH V6 21/21] core/pldm: Get file handle and file length

[Frederic Barrat]

On 13/09/2022 12:27, Christophe Lombard wrote:
> diff --git a/core/pldm/pldm-file-io-requests.c b/core/pldm/pldm-file-io-requests.c
> index 0665a679..a1191b89 100644
> --- a/core/pldm/pldm-file-io-requests.c
> +++ b/core/pldm/pldm-file-io-requests.c
> @@ -34,6 +34,82 @@ static void file_io_init_complete(bool success)
>   	file_io_ready = true;
>   }
>   
> +#define CHKSUM_PADDING 8
> +
> +/*
> + * Retrieve the file handle and file length from the file attribute
> + * table.
> + */
> +static int find_file_handle_by_lid_id(const char *lid_id,
> +				      uint32_t *file_handle,
> +				      uint32_t *file_length)
> +{
> +	const struct pldm_file_attr_table_entry *file_entry;
> +	char *startptr, *endptr;
> +	uint16_t file_name_length;
> +
> +	if ((file_attr_table == NULL) || (file_attr_length == 0))
> +		return OPAL_PARAMETER;
> +
> +	startptr = (char *)file_attr_table;
> +	endptr = startptr + file_attr_length - CHKSUM_PADDING;
> +	*file_handle = 0;
> +	*file_length = 0;
> +
> +	while (startptr < endptr) {
> +		/* file entry:
> +		 *   4 Bytes: file handle
> +		 *   2 Bytes: file name length
> +		 *   <file name length> Bytes: file name
> +		 *   4 Bytes: file length
> +		 */
> +		file_entry = (struct pldm_file_attr_table_entry *)startptr;
> +
> +		*file_handle = le32_to_cpu(file_entry->file_handle);
> +		startptr += sizeof(uint32_t);
> +
> +		file_name_length = le16_to_cpu(file_entry->file_name_length);
> +		startptr += sizeof(file_name_length);
> +
> +		if (!strncmp(startptr, lid_id, strlen(lid_id))) {
> +			startptr += file_name_length;
> +			*file_length = le32_to_cpu(*(uint32_t *)startptr);
> +			break;
> +		}
> +		startptr += file_name_length;
> +		startptr += sizeof(uint32_t);
> +		startptr += sizeof(bitfield32_t);
> +	}
> +
> +	if (*file_length == 0) {
> +		prlog(PR_ERR, "%s - No file handle found, lid_id: %s\n",
> +			      __func__, lid_id);
> +		*file_handle = 0xff;
> +		*file_length = 0;
> +		return OPAL_PARAMETER;
> +	}


(sorry for the late comment, but I'm re-reading this as it's being used 
by a patch in a later series)

So 0xff is an invalid file handle (should probably be a macro). IIUC, in 
such a case, we'll still add the struct pldm_lid to the list of LIDs 
(lid_files in core/pldm/pldm-lid-files.c). Though we don't really expect 
it, one could try reading/writing such a lid!

- handles are apparently 16-bits and seem to be mostly sequential, but 
wouldn't it be safer to choose 0xffff as an invalid handle?

- we should detect operations on such invalid handles later (in 
read_file_req/write_file_req/...)


   Fred