On Mon, Feb 2016 Andrew Morton <[email protected]> wrote:
> On Fri, 5 Feb 2016 18:19:38 -0500 Alexandre Bounine
> <[email protected]> wrote:
>
> > +int rio_del_mport_pw_handler(struct rio_mport *mport, void *context,
> > + int (*pwcback)(struct rio_mport *mport,
> > + void *context, union rio_pw_msg *msg, int step))
> > +{
> > + int rc = -EINVAL;
> > + struct rio_pwrite *pwrite;
> > +
> > + mutex_lock(&mport->lock);
> > + list_for_each_entry(pwrite, &mport->pwrites, node) {
>
> You have a use-after-free here - list_for_each_entry() references the
> pwrite_node_next which was freed on the previous loop.
>
> I'll switch this to list_for_each_entry_safe. Please test that change
> and review the other patches for reoccurrences.
Each combination of callback and context is expected to be unique.
If matching pair is found, list_for_each_entry loop is terminated with 'break'
immediately.
Should be safe in this case.
Probably a comment about unique combination of callback and context should be
here(?).
Even if the pair is not unique, only the first entry will be deleted due to
having 'break' there.
>
> > + if (pwrite->pwcback == pwcback && pwrite->context ==
> context) {
> > + list_del(&pwrite->node);
> > + kfree(pwrite);
> > + rc = 0;
> > + break;
> > + }
> > + }
> > + mutex_unlock(&mport->lock);
> > +
> > + return rc;
> > +}
> > +EXPORT_SYMBOL_GPL(rio_del_mport_pw_handler);
