[Skiboot] [PATCH v4 10/10] opal-prd: Fix prd message size issue

[Vasant Hegde]

On 05/22/2019 07:09 AM, Stewart Smith wrote:
> Vasant Hegde <hegdevasant at linux.vnet.ibm.com> writes:
>> If prd messages size is insufficient then read_prd_msg() call fails with
>> below error. And caller is not reallocating sufficient buffer. Also its
>> hard to guess the size.
>>
>> sample log:
>> -----------
>> Mar 28 03:31:43 zz24p1 opal-prd: FW: error reading from firmware: alloc 32 rc -1: Invalid argument
>> Mar 28 03:31:43 zz24p1 opal-prd: FW: error reading from firmware: alloc 32 rc -1: Invalid argument
>> Mar 28 03:31:43 zz24p1 opal-prd: FW: error reading from firmware: alloc 32 rc -1: Invalid argument
>> ....
>>
>> Lets use `opal-msg-size` device tree property to allocate memory
>> for prd message.
>>
>> Signed-off-by: Vasant Hegde <hegdevasant at linux.vnet.ibm.com>
> 
> Also CC stable?

Yes! Will fix it in v5.

> 
>> diff --git a/external/opal-prd/opal-prd.c b/external/opal-prd/opal-prd.c
>> index 33ea5f5a8..9df0d2753 100644
>> --- a/external/opal-prd/opal-prd.c
>> +++ b/external/opal-prd/opal-prd.c
>> @@ -2120,7 +2120,9 @@ static int init_control_socket(struct opal_prd_ctx *ctx)
>>   
>>   static int run_prd_daemon(struct opal_prd_ctx *ctx)
>>   {
>> -	int rc;
>> +	char *opal_msg_path;
>> +	void *buf;
>> +	int rc, len;
>>   
>>   	/* log to syslog */
>>   	pr_log_daemon_init();
>> @@ -2130,8 +2132,25 @@ static int run_prd_daemon(struct opal_prd_ctx *ctx)
>>   	ctx->fd = -1;
>>   	ctx->socket = -1;
>>   
>> -	/* set up our message buffer */
>> -	ctx->msg_alloc_len = sizeof(*ctx->msg);
>> +	/*
>> +	 * Set up our message buffer. Use opal-msg-size device tree
>> +	 * property to get message buffer size.
>> +	 */
>> +	rc = asprintf(&opal_msg_path,
>> +		       "%s/ibm,opal/opal-msg-size", devicetree_base);
>> +	if (rc > 0) {
>> +		rc = open_and_read(opal_msg_path, &buf, &len);
>> +		if (rc == 0) {
>> +			ctx->msg_alloc_len = be32toh(*(__be32 *)buf);
>> +			free(buf);
>> +		}
>> +
>> +		free(opal_msg_path);
>> +	}
> 
> While in main() we do initialize ctx->msg_alloc_len (we memset the whole
> structure), in the interest of being explicit, it may be worthwhile
> setting it to zero first, just so we are obviously correct in the rc==0
> case?

sure.

-Vasant