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