[PATCH -mm] slab: update_memcg_params: explicitly check that old array != NULL

Vladimir Davydov
2015-01-26T10:01:42Z

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@oracle.com>
Signed-off-by: Vladimir Davydov <vdavydov@parallels.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