Received-SPF: pass (google.com: best guess record for domain of linux-wireless-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
Message-ID: <8b13bfffccf363b565d00f51ce7d735f62a57135.camel@sipsolutions.net>
Subject: Re: [PATCH] cfg80211: use parallel_ops for genl
From:   Johannes Berg <johannes@sipsolutions.net>
To:     Denis Kenzior <denkenz@gmail.com>, linux-wireless@vger.kernel.org
Date:   Sat, 27 Jul 2019 20:41:27 +0200
In-Reply-To: <fe8ef214-5d02-da36-2680-6df3c683c154@gmail.com> (sfid-20190727_173339_760130_162AF66A)
References: <20190726191621.5031-1-johannes@sipsolutions.net>
         <fe8ef214-5d02-da36-2680-6df3c683c154@gmail.com>
         (sfid-20190727_173339_760130_162AF66A)
Content-Type: text/plain; charset="UTF-8"
User-Agent: Evolution 3.30.5 (3.30.5-1.fc29) 
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Sender: linux-wireless-owner@vger.kernel.org
Precedence: bulk

Hi Denis,

(huh, why did your mail make it to my inbox 3 hours ago ...? oh well)

> > +		kfree(attrbuf);
> > +		if (IS_ERR(*wdev)) {
> > +			kfree(attrbuf);
> 
> Hmm, you just freed attrbuf above?

Good catch.

I was being stupid, wrote the patch on one machine, then tested & fixed
it on another, and then sent out the original ...

> >   	if (!attrbuf[NL80211_ATTR_VENDOR_ID] ||
> > -	    !attrbuf[NL80211_ATTR_VENDOR_SUBCMD])
> > -		return -EINVAL;
> > +	    !attrbuf[NL80211_ATTR_VENDOR_SUBCMD]) {
> > +		err = -EINVAL;
> > +		goto out;
> > +	}
> 
> Might be nicer to just set err = -EINVAL before the if instead of using 
> {} here

Dunno. I don't generally like the values "leaking" out of where they're
intended, tends to hide compiler warnings when you forget to assign or
something ... I guess doing -EINVAL at least would fail safely :-)

I'll revise this then.

johannes