Re: [PATCH 2/2] mac80211: Free mpath object when rhashtable insertion fails (fwd)

Re: [PATCH 2/2] mac80211: Free mpath object when rhashtable insertion fails (fwd)

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 6385 bytes --]

Perhaps a return is needed after line 436.

julia

---------- Forwarded message ----------
Date: Wed, 13 Feb 2019 20:29:38 +0800
From: kbuild test robot <lkp@intel.com>
To: kbuild@01.org
Cc: Julia Lawall <julia.lawall@lip6.fr>
Subject: Re: [PATCH 2/2] mac80211: Free mpath object when rhashtable insertion
    fails

CC: kbuild-all@01.org
In-Reply-To: <E1gtmu6-0001Vl-O7@gondobar>
References: <E1gtmu6-0001Vl-O7@gondobar>
TO: Herbert Xu <herbert@gondor.apana.org.au>
CC: David Miller <davem@davemloft.net>, johannes@sipsolutions.net, linux-wireless@vger.kernel.org, netdev@vger.kernel.org, j@w1.fi, tgraf@suug.ch, johannes.berg@intel.com
CC:

Hi Herbert,

I love your patch! Perhaps something to improve:

[auto build test WARNING on mac80211-next/master]
[also build test WARNING on v5.0-rc4 next-20190212]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Herbert-Xu/mac80211-Fix-incorrect-usage-of-rhashtable-walk-API/20190213-181325
base:   https://git.kernel.org/pub/scm/linux/kernel/git/jberg/mac80211-next.git master
:::::: branch date: 2 hours ago
:::::: commit date: 2 hours ago

>> net/mac80211/mesh_pathtbl.c:448:8-17: ERROR: reference preceded by free on line 436

# https://github.com/0day-ci/linux/commit/a0886e834aacf883ceaf0c34c842c4cdb4d318fd
git remote add linux-review https://github.com/0day-ci/linux
git remote update linux-review
git checkout a0886e834aacf883ceaf0c34c842c4cdb4d318fd
vim +448 net/mac80211/mesh_pathtbl.c

b15dc38b9 Bob Copeland         2016-02-28  390
eb2b9311f Luis Carlos Cobo     2008-02-23  391  /**
eb2b9311f Luis Carlos Cobo     2008-02-23  392   * mesh_path_add - allocate and add a new path to the mesh path table
bf7cd94dc Johannes Berg        2013-02-15  393   * @dst: destination address of the path (ETH_ALEN length)
f698d856f Jasper Bryant-Greene 2008-08-03  394   * @sdata: local subif
eb2b9311f Luis Carlos Cobo     2008-02-23  395   *
af901ca18 André Goddard Rosa   2009-11-14  396   * Returns: 0 on success
eb2b9311f Luis Carlos Cobo     2008-02-23  397   *
eb2b9311f Luis Carlos Cobo     2008-02-23  398   * State: the initial state of the new path is set to 0
eb2b9311f Luis Carlos Cobo     2008-02-23  399   */
ae76eef02 Bob Copeland         2013-03-29  400  struct mesh_path *mesh_path_add(struct ieee80211_sub_if_data *sdata,
ae76eef02 Bob Copeland         2013-03-29  401  				const u8 *dst)
eb2b9311f Luis Carlos Cobo     2008-02-23  402  {
349eb8cf4 Johannes Berg        2011-05-14  403  	struct mesh_table *tbl;
eb2b9311f Luis Carlos Cobo     2008-02-23  404  	struct mesh_path *mpath, *new_mpath;
60854fd94 Bob Copeland         2016-03-02  405  	int ret;
eb2b9311f Luis Carlos Cobo     2008-02-23  406
b203ca391 Joe Perches          2012-05-08  407  	if (ether_addr_equal(dst, sdata->vif.addr))
eb2b9311f Luis Carlos Cobo     2008-02-23  408  		/* never add ourselves as neighbours */
ae76eef02 Bob Copeland         2013-03-29  409  		return ERR_PTR(-ENOTSUPP);
eb2b9311f Luis Carlos Cobo     2008-02-23  410
eb2b9311f Luis Carlos Cobo     2008-02-23  411  	if (is_multicast_ether_addr(dst))
ae76eef02 Bob Copeland         2013-03-29  412  		return ERR_PTR(-ENOTSUPP);
eb2b9311f Luis Carlos Cobo     2008-02-23  413
472dbc45d Johannes Berg        2008-09-11  414  	if (atomic_add_unless(&sdata->u.mesh.mpaths, 1, MESH_MAX_MPATHS) == 0)
ae76eef02 Bob Copeland         2013-03-29  415  		return ERR_PTR(-ENOSPC);
ae76eef02 Bob Copeland         2013-03-29  416
b15dc38b9 Bob Copeland         2016-02-28  417  	new_mpath = mesh_path_new(sdata, dst, GFP_ATOMIC);
402d7752e Pavel Emelyanov      2008-05-06  418  	if (!new_mpath)
60854fd94 Bob Copeland         2016-03-02  419  		return ERR_PTR(-ENOMEM);
402d7752e Pavel Emelyanov      2008-05-06  420
60854fd94 Bob Copeland         2016-03-02  421  	tbl = sdata->u.mesh.mesh_paths;
60854fd94 Bob Copeland         2016-03-02  422  	do {
60854fd94 Bob Copeland         2016-03-02  423  		ret = rhashtable_lookup_insert_fast(&tbl->rhead,
60854fd94 Bob Copeland         2016-03-02  424  						    &new_mpath->rhash,
60854fd94 Bob Copeland         2016-03-02  425  						    mesh_rht_params);
f84e71a94 Pavel Emelyanov      2008-05-06  426
60854fd94 Bob Copeland         2016-03-02  427  		if (ret == -EEXIST)
60854fd94 Bob Copeland         2016-03-02  428  			mpath = rhashtable_lookup_fast(&tbl->rhead,
60854fd94 Bob Copeland         2016-03-02  429  						       dst,
60854fd94 Bob Copeland         2016-03-02  430  						       mesh_rht_params);
05b0152f1 Herbert Xu           2019-02-13  431  		else if (!ret)
05b0152f1 Herbert Xu           2019-02-13  432  			hlist_add_head(&new_mpath->walk_list, &tbl->walk_head);
60854fd94 Bob Copeland         2016-03-02  433  	} while (unlikely(ret == -EEXIST && !mpath));
f5ea9120b Johannes Berg        2009-08-07  434
a0886e834 Herbert Xu           2019-02-13  435  	if (ret)
a0886e834 Herbert Xu           2019-02-13 @436  		kfree(new_mpath);
a0886e834 Herbert Xu           2019-02-13  437
a0886e834 Herbert Xu           2019-02-13  438  	if (ret != -EEXIST)
60854fd94 Bob Copeland         2016-03-02  439  		return ERR_PTR(ret);
ae76eef02 Bob Copeland         2013-03-29  440
60854fd94 Bob Copeland         2016-03-02  441  	/* At this point either new_mpath was added, or we found a
60854fd94 Bob Copeland         2016-03-02  442  	 * matching entry already in the table; in the latter case
60854fd94 Bob Copeland         2016-03-02  443  	 * free the unnecessary new entry.
60854fd94 Bob Copeland         2016-03-02  444  	 */
a0886e834 Herbert Xu           2019-02-13  445  	if (ret == -EEXIST)
60854fd94 Bob Copeland         2016-03-02  446  		new_mpath = mpath;
60854fd94 Bob Copeland         2016-03-02  447  	sdata->u.mesh.mesh_paths_generation++;
60854fd94 Bob Copeland         2016-03-02 @448  	return new_mpath;
18889231e Javier Cardona       2009-08-10  449  }
eb2b9311f Luis Carlos Cobo     2008-02-23  450

:::::: The code at line 448 was first introduced by commit
:::::: 60854fd94573f0d3b80b55b40cf0140a0430f3ab mac80211: mesh: convert path table to rhashtable

:::::: TO: Bob Copeland <me@bobcopeland.com>
:::::: CC: Johannes Berg <johannes.berg@intel.com>

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: [PATCH 2/2] mac80211: Free mpath object when rhashtable insertion fails (fwd)

[Herbert Xu]

On Wed, Feb 13, 2019 at 01:50:20PM +0100, Julia Lawall wrote:
> Perhaps a return is needed after line 436.

I don't think so.  The kfree only occurs in the case where the
hash table insertion fails.  While we should only return an error
if both the hash table insertion failed and the error is not due
to an existing entry (EEXIST).

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt