| Message ID | 20250214162109.3555300-1-quic_jhugo@quicinc.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | bus: mhi: host: Avoid possible uninitialized fw_load_type | expand |
On Fri, Feb 14, 2025 at 09:21:09AM -0700, Jeffrey Hugo wrote: > If mhi_fw_load_handler() bails out early because the EE is not capable > of loading firmware, we may reference fw_load_type in cleanup which is > uninitialized at this point. The cleanup code checks fw_load_type as a > proxy for knowing if fbc_image was allocated and needs to be freed, but > we can directly test for that. This avoids the possible uninitialized > access and appears to be clearer code. > > Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > Closes: https://lore.kernel.org/all/e3148ac4-7bb8-422d-ae0f-18a8eb15e269@stanley.mountain/ > Fixes: f88f1d0998ea ("bus: mhi: host: Add a policy to enable image transfer via BHIe in PBL") The best thing would be to squash this fix into the offending commit as the fixes tag would become meaningless once merged upstream. > Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com> Acked-by: Manivannan Sadhasivam <manivannan.sadhasivam@linaro.org> - Mani > --- > drivers/bus/mhi/host/boot.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/bus/mhi/host/boot.c b/drivers/bus/mhi/host/boot.c > index c8e48f621a8c..efa3b6dddf4d 100644 > --- a/drivers/bus/mhi/host/boot.c > +++ b/drivers/bus/mhi/host/boot.c > @@ -608,7 +608,7 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl) > return; > > error_ready_state: > - if (fw_load_type == MHI_FW_LOAD_FBC) { > + if (mhi_cntrl->fbc_image) { > mhi_free_bhie_table(mhi_cntrl, mhi_cntrl->fbc_image); > mhi_cntrl->fbc_image = NULL; > } > -- > 2.34.1 >
On 2/14/2025 10:34 AM, Manivannan Sadhasivam wrote: > On Fri, Feb 14, 2025 at 09:21:09AM -0700, Jeffrey Hugo wrote: >> If mhi_fw_load_handler() bails out early because the EE is not capable >> of loading firmware, we may reference fw_load_type in cleanup which is >> uninitialized at this point. The cleanup code checks fw_load_type as a >> proxy for knowing if fbc_image was allocated and needs to be freed, but >> we can directly test for that. This avoids the possible uninitialized >> access and appears to be clearer code. >> >> Reported-by: Dan Carpenter <dan.carpenter@linaro.org> >> Closes: https://lore.kernel.org/all/e3148ac4-7bb8-422d-ae0f-18a8eb15e269@stanley.mountain/ >> Fixes: f88f1d0998ea ("bus: mhi: host: Add a policy to enable image transfer via BHIe in PBL") > > The best thing would be to squash this fix into the offending commit as the > fixes tag would become meaningless once merged upstream. I see your point, however the offending commit is already part of a pull request. I think we've missed the window for squashing. Thank you for the very quick review. -Jeff
On 2/14/2025 8:21 AM, Jeffrey Hugo wrote: > If mhi_fw_load_handler() bails out early because the EE is not capable > of loading firmware, we may reference fw_load_type in cleanup which is > uninitialized at this point. The cleanup code checks fw_load_type as a > proxy for knowing if fbc_image was allocated and needs to be freed, but > we can directly test for that. This avoids the possible uninitialized > access and appears to be clearer code. > > Reported-by: Dan Carpenter <dan.carpenter@linaro.org> > Closes: https://lore.kernel.org/all/e3148ac4-7bb8-422d-ae0f-18a8eb15e269@stanley.mountain/ > Fixes: f88f1d0998ea ("bus: mhi: host: Add a policy to enable image transfer via BHIe in PBL") > Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com> Reviewed-by: Carl Vanderlip <quic_carlv@quicinc.com>
diff --git a/drivers/bus/mhi/host/boot.c b/drivers/bus/mhi/host/boot.c index c8e48f621a8c..efa3b6dddf4d 100644 --- a/drivers/bus/mhi/host/boot.c +++ b/drivers/bus/mhi/host/boot.c @@ -608,7 +608,7 @@ void mhi_fw_load_handler(struct mhi_controller *mhi_cntrl) return; error_ready_state: - if (fw_load_type == MHI_FW_LOAD_FBC) { + if (mhi_cntrl->fbc_image) { mhi_free_bhie_table(mhi_cntrl, mhi_cntrl->fbc_image); mhi_cntrl->fbc_image = NULL; }
If mhi_fw_load_handler() bails out early because the EE is not capable of loading firmware, we may reference fw_load_type in cleanup which is uninitialized at this point. The cleanup code checks fw_load_type as a proxy for knowing if fbc_image was allocated and needs to be freed, but we can directly test for that. This avoids the possible uninitialized access and appears to be clearer code. Reported-by: Dan Carpenter <dan.carpenter@linaro.org> Closes: https://lore.kernel.org/all/e3148ac4-7bb8-422d-ae0f-18a8eb15e269@stanley.mountain/ Fixes: f88f1d0998ea ("bus: mhi: host: Add a policy to enable image transfer via BHIe in PBL") Signed-off-by: Jeffrey Hugo <quic_jhugo@quicinc.com> --- drivers/bus/mhi/host/boot.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)