Missing NULL checks - qmi-proxy.c
Aleksander Morgado
aleksander at aleksander.es
Wed Dec 30 14:00:07 UTC 2020
Hey Peter!
> > It's been suggested this is a transition case, where the CID might no longer be
> > valid. I'm not 100% on the code path that calls this, but it seems prudent
> > that libqmi protects itself no matter what.
> >
>
> If we add those NULL checks and the problem is that we're reading the
> struct once it's already been freed, we're really ignoring the
> original problem and the invalid memory usage will bite us somewhere
> else later on, maybe not as clear as in this case. I'm not against
> adding NULL checks, but we shouldn't treat those as valid use cases if
> the code path doesn't consider them valid. i.e. I would personally
> make them g_assert()s if the logic assumes they should never happen,
> not plain if() checks.
>
> I'll try to review again the current logic to see how this may happen.
>
Your changes and the original fix from Madhavadas all make sense,
please see: https://gitlab.freedesktop.org/mobile-broadband/libqmi/-/merge_requests/181#note_740922
Will also backport the fixes to the stable branch, thanks!
--
Aleksander
https://aleksander.es
More information about the libqmi-devel
mailing list