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-next>] [day] [month] [year] [list]
Date:	Mon, 26 Jan 2015 13:01:19 +0300
From:	Vladimir Davydov <vdavydov@...allels.com>
To:	Andrew Morton <akpm@...ux-foundation.org>
CC:	Dan Carpenter <dan.carpenter@...cle.com>,
	Johannes Weiner <hannes@...xchg.org>,
	Michal Hocko <mhocko@...e.cz>,
	Christoph Lameter <cl@...ux.com>,
	Pekka Enberg <penberg@...nel.org>,
	David Rientjes <rientjes@...gle.com>,
	Joonsoo Kim <iamjoonsoo.kim@....com>, <linux-mm@...ck.org>,
	<linux-kernel@...r.kernel.org>
Subject: [PATCH -mm] slab: update_memcg_params: explicitly check that old array != NULL

tree:   git://git.cmpxchg.org/linux-mmotm.git master
head:   c64429bcc60a702f19f5cfdb5c39277863278a8c
commit: 5d06629c100b942a51f02b4d886c116ba3afb32a [200/417] slab: embed memcg_cache_params to kmem_cache

mm/slab_common.c:166 update_memcg_params() warn: variable dereferenced before check 'old' (see line 162)

git remote add mmotm git://git.cmpxchg.org/linux-mmotm.git
git remote update mmotm
git checkout 5d06629c100b942a51f02b4d886c116ba3afb32a
vim +/old +166 mm/slab_common.c

5d06629c Vladimir Davydov 2015-01-24  156                                       lockdep_is_held(&slab_mutex));
5d06629c Vladimir Davydov 2015-01-24  157       new = kzalloc(sizeof(struct memcg_cache_array) +
5d06629c Vladimir Davydov 2015-01-24  158                     new_array_size * sizeof(void *), GFP_KERNEL);
5d06629c Vladimir Davydov 2015-01-24  159       if (!new)
6f817f4c Vladimir Davydov 2014-10-09  160               return -ENOMEM;
6f817f4c Vladimir Davydov 2014-10-09  161
5d06629c Vladimir Davydov 2015-01-24 @162       memcpy(new->entries, old->entries,
88a0b848 Vladimir Davydov 2015-01-24  163              memcg_nr_cache_ids * sizeof(void *));
6f817f4c Vladimir Davydov 2014-10-09  164
5d06629c Vladimir Davydov 2015-01-24  165       rcu_assign_pointer(s->memcg_params.memcg_caches, new);
5d06629c Vladimir Davydov 2015-01-24 @166       if (old)
5d06629c Vladimir Davydov 2015-01-24  167               kfree_rcu(old, rcu);
6f817f4c Vladimir Davydov 2014-10-09  168       return 0;
6f817f4c Vladimir Davydov 2014-10-09  169  }

This warning is false-positive, because @old equals NULL iff
@memcg_nr_cache_ids equals 0. Moreover, this function had been acting in
exactly the same fashion before it was reworked by the culprit. Anyways,
let's add an explicit check if @old is not NULL before passing it to
@memcpy() to make static analysis tools happy.

fixes: slab-embed-memcg_cache_params-to-kmem_cache
Reported-by: Dan Carpenter <dan.carpenter@...cle.com>
Signed-off-by: Vladimir Davydov <vdavydov@...allels.com>
---
 mm/slab_common.c |    9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/mm/slab_common.c b/mm/slab_common.c
index bf4a42b2c5ba..0dd9eb4e0f87 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -153,15 +153,16 @@ static int update_memcg_params(struct kmem_cache *s, int new_array_size)
 	if (!is_root_cache(s))
 		return 0;
 
-	old = rcu_dereference_protected(s->memcg_params.memcg_caches,
-					lockdep_is_held(&slab_mutex));
 	new = kzalloc(sizeof(struct memcg_cache_array) +
 		      new_array_size * sizeof(void *), GFP_KERNEL);
 	if (!new)
 		return -ENOMEM;
 
-	memcpy(new->entries, old->entries,
-	       memcg_nr_cache_ids * sizeof(void *));
+	old = rcu_dereference_protected(s->memcg_params.memcg_caches,
+					lockdep_is_held(&slab_mutex));
+	if (old)
+		memcpy(new->entries, old->entries,
+		       memcg_nr_cache_ids * sizeof(void *));
 
 	rcu_assign_pointer(s->memcg_params.memcg_caches, new);
 	if (old)
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ