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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <8D983423E7EDF846BB3056827B8CC5D15CFB9A9A@corpmail1.na.ads.idt.com>
Date:	Tue, 9 Feb 2016 13:56:09 +0000
From:	"Bounine, Alexandre" <Alexandre.Bounine@....com>
To:	Andrew Morton <akpm@...ux-foundation.org>
CC:	Matt Porter <mporter@...nel.crashing.org>,
	Aurelien Jacquiot <a-jacquiot@...com>,
	Andre van Herk <andre.van.herk@...drive-technologies.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: RE: [PATCH 22/30] rapidio: add global inbound port write interfaces

On Mon, Feb 2016 Andrew Morton <akpm@...ux-foundation.org> wrote:

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