RE: [PATCH 22/30] rapidio: add global inbound port write interfaces

[Bounine, Alexandre]

On Mon, Feb 2016 Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:

> On Fri,  5 Feb 2016 18:19:38 -0500 Alexandre Bounine
> <alexandre.bounine@xxxxxxx> 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);