mirror of
https://github.com/torvalds/linux.git
synced 2026-07-28 01:55:51 +02:00
firmware: stratix10-rsu: Fix NULL deref on rsu_send_msg() timeout in probe
rsu_send_msg() can return -ETIMEDOUT when wait_for_completion_interruptible_timeout() fires while the SMC call is still pending. In stratix10_rsu_probe(), the error paths for COMMAND_RSU_DCMF_VERSION, COMMAND_RSU_DCMF_STATUS, COMMAND_RSU_MAX_RETRY and COMMAND_RSU_GET_SPT_TABLE call stratix10_svc_free_channel() - which sets chan->scl to NULL - but then fall through and queue the next request on the same channel. The next svc kthread that runs will dereference pdata->chan->scl in its receive callback path, triggering a NULL pointer dereference identical to the one fixed by commitc45f726310("firmware: stratix10-rsu: Fix NULL pointer dereference when RSU is disabled") for the COMMAND_RSU_STATUS path. Apply the same cleanup pattern to the remaining failure paths: remove the async client, free the channel, and return early so no further messages are queued on a channel whose scl has been cleared. While at it, clean up stratix10_rsu_probe() in two ways without changing behavior: - Drop redundant zero-initialization of fields already cleared by devm_kzalloc(): client.receive_cb, status.* and spt0/1_address (INVALID_SPT_ADDRESS is 0x0). - Replace five identical 3-line error-cleanup blocks (stratix10_svc_remove_async_client() + stratix10_svc_free_channel() + return ret) with goto labels (remove_async_client, free_channel), matching the standard kernel resource-unwinding pattern and making it easier to extend the probe sequence without forgetting matching cleanup. Also move init_completion() next to mutex_init() so sync-primitive initialization is grouped before anything that could trigger a callback. Fixes:15847537b6("firmware: stratix10-rsu: Migrate RSU driver to use stratix10 asynchronous framework.") Cc: stable@kernel.org Assisted-by: Claude:claude-4.7-opus-high Cursor Signed-off-by: Dinh Nguyen <dinguyen@kernel.org> --- v2: Add a minor clean-up of the function stratix10_rsu_probe() to have a centralize exit for all the rsu_send_async_msg() and rsu_send_msg().
This commit is contained in:
parent
371aa06221
commit
bfd2eb9bba
|
|
@ -723,15 +723,9 @@ static int stratix10_rsu_probe(struct platform_device *pdev)
|
|||
return -ENOMEM;
|
||||
|
||||
priv->client.dev = dev;
|
||||
priv->client.receive_cb = NULL;
|
||||
priv->client.priv = priv;
|
||||
priv->status.current_image = 0;
|
||||
priv->status.fail_image = 0;
|
||||
priv->status.error_location = 0;
|
||||
priv->status.error_details = 0;
|
||||
priv->status.version = 0;
|
||||
priv->status.state = 0;
|
||||
priv->retry_counter = INVALID_RETRY_COUNTER;
|
||||
priv->max_retry = INVALID_RETRY_COUNTER;
|
||||
priv->dcmf_version.dcmf0 = INVALID_DCMF_VERSION;
|
||||
priv->dcmf_version.dcmf1 = INVALID_DCMF_VERSION;
|
||||
priv->dcmf_version.dcmf2 = INVALID_DCMF_VERSION;
|
||||
|
|
@ -740,11 +734,11 @@ static int stratix10_rsu_probe(struct platform_device *pdev)
|
|||
priv->dcmf_status.dcmf1 = INVALID_DCMF_STATUS;
|
||||
priv->dcmf_status.dcmf2 = INVALID_DCMF_STATUS;
|
||||
priv->dcmf_status.dcmf3 = INVALID_DCMF_STATUS;
|
||||
priv->max_retry = INVALID_RETRY_COUNTER;
|
||||
priv->spt0_address = INVALID_SPT_ADDRESS;
|
||||
priv->spt1_address = INVALID_SPT_ADDRESS;
|
||||
/* spt0/1_address and status fields default to 0 from kzalloc */
|
||||
|
||||
mutex_init(&priv->lock);
|
||||
init_completion(&priv->completion);
|
||||
|
||||
priv->chan = stratix10_svc_request_channel_byname(&priv->client,
|
||||
SVC_CLIENT_RSU);
|
||||
if (IS_ERR(priv->chan)) {
|
||||
|
|
@ -756,11 +750,9 @@ static int stratix10_rsu_probe(struct platform_device *pdev)
|
|||
ret = stratix10_svc_add_async_client(priv->chan, false);
|
||||
if (ret) {
|
||||
dev_err(dev, "failed to add async client\n");
|
||||
stratix10_svc_free_channel(priv->chan);
|
||||
return ret;
|
||||
goto free_channel;
|
||||
}
|
||||
|
||||
init_completion(&priv->completion);
|
||||
platform_set_drvdata(pdev, priv);
|
||||
|
||||
/* get the initial state from firmware */
|
||||
|
|
@ -768,41 +760,44 @@ static int stratix10_rsu_probe(struct platform_device *pdev)
|
|||
rsu_async_status_callback);
|
||||
if (ret) {
|
||||
dev_err(dev, "Error, getting RSU status %i\n", ret);
|
||||
stratix10_svc_remove_async_client(priv->chan);
|
||||
stratix10_svc_free_channel(priv->chan);
|
||||
return ret;
|
||||
goto remove_async_client;
|
||||
}
|
||||
|
||||
/* get DCMF version from firmware */
|
||||
ret = rsu_send_msg(priv, COMMAND_RSU_DCMF_VERSION,
|
||||
0, rsu_dcmf_version_callback);
|
||||
ret = rsu_send_msg(priv, COMMAND_RSU_DCMF_VERSION, 0,
|
||||
rsu_dcmf_version_callback);
|
||||
if (ret) {
|
||||
dev_err(dev, "Error, getting DCMF version %i\n", ret);
|
||||
stratix10_svc_free_channel(priv->chan);
|
||||
goto remove_async_client;
|
||||
}
|
||||
|
||||
ret = rsu_send_msg(priv, COMMAND_RSU_DCMF_STATUS,
|
||||
0, rsu_dcmf_status_callback);
|
||||
ret = rsu_send_msg(priv, COMMAND_RSU_DCMF_STATUS, 0,
|
||||
rsu_dcmf_status_callback);
|
||||
if (ret) {
|
||||
dev_err(dev, "Error, getting DCMF status %i\n", ret);
|
||||
stratix10_svc_free_channel(priv->chan);
|
||||
goto remove_async_client;
|
||||
}
|
||||
|
||||
ret = rsu_send_msg(priv, COMMAND_RSU_MAX_RETRY, 0,
|
||||
rsu_max_retry_callback);
|
||||
if (ret) {
|
||||
dev_err(dev, "Error, getting RSU max retry %i\n", ret);
|
||||
stratix10_svc_free_channel(priv->chan);
|
||||
goto remove_async_client;
|
||||
}
|
||||
|
||||
|
||||
ret = rsu_send_async_msg(dev, priv, COMMAND_RSU_GET_SPT_TABLE, 0,
|
||||
rsu_async_get_spt_table_callback);
|
||||
if (ret) {
|
||||
dev_err(dev, "Error, getting SPT table %i\n", ret);
|
||||
stratix10_svc_free_channel(priv->chan);
|
||||
goto remove_async_client;
|
||||
}
|
||||
|
||||
return 0;
|
||||
|
||||
remove_async_client:
|
||||
stratix10_svc_remove_async_client(priv->chan);
|
||||
free_channel:
|
||||
stratix10_svc_free_channel(priv->chan);
|
||||
return ret;
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user