On Sun, Jan 28, 2024 at 11:29:33AM +0100, Erick Archer wrote:
> > It's a bit concerning that ->event_rings is set multiple times, but only
> > allocated one time. It's either unnecessary or there is a potential
> > memory corruption bug. If it's really necessary then there should be a
> > check that the new size is <= the size of the original buffer that we
> > allocated.
>
> The ->event_rings is set twice. In the mhi_ep_mmio_init function and in
> the mhi_ep_mmio_update_ner function.
>
It's not about the type.
The event_rings struct member is the number of elements in the
mhi_cntrl->mhi_event array. However, we ->event_rings without
re-allocating mhi_cntrl->mhi_event so those are not in sync any more.
So since we don't know the number of elements in the mhi_cntrl->mhi_event
array leading to memory corruption.
> void mhi_ep_mmio_init(struct mhi_ep_cntrl *mhi_cntrl)
> {
> [...]
> mhi_cntrl->event_rings = FIELD_GET(MHICFG_NER_MASK, regval);
> [...]
> }
>
> void mhi_ep_mmio_update_ner(struct mhi_ep_cntrl *mhi_cntrl)
> {
> [...]
> mhi_cntrl->event_rings = FIELD_GET(MHICFG_NER_MASK, regval);
> [...]
> }
These ->event_rings assignments look exactly the same. It depends on
regval. So possibly one could be deleted.
regards,
dan carpenter
