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

Stewart Smith stewart at linux.ibm.com
Wed May 22 11:39:41 AEST 2019


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?

> 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?

> +
> +	if (ctx->msg_alloc_len == 0)
> +		ctx->msg_alloc_len = sizeof(*ctx->msg);
> +
>  	ctx->msg = malloc(ctx->msg_alloc_len);
>  	if (!ctx->msg) {
>  		pr_log(LOG_ERR, "FW: Can't allocate PRD message buffer: %m");
> -- 
> 2.14.3
>

-- 
Stewart Smith
OPAL Architect, IBM.



More information about the Skiboot mailing list