net: bridge: netfilter: use vzalloc() instead of vmalloc() for counterstmp
Subsystems (3)
PATCH 1/1 net: bridge: netfilter: use vzalloc() instead of vmalloc() for counterstmp
counterstmp is not cleared before it is used in get_counters(). it might be leaked partially when it is sent to userland later on.
--- net/bridge/netfilter/ebtables.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
net/bridge/netfilter/ebtables.c +2 -2
@@ -989,7 +989,7 @@ static int do_replace_finish(struct net *net, struct ebt_replace *repl,the check on the size is done later, when we have the lock */ if (repl->num_counters) {unsigned long size = repl->num_counters *sizeof (*counterstmp); - counterstmp =vmalloc (size); + counterstmp =vzalloc (size);if (!counterstmp)return -ENOMEM ; }
@@ -1410,7 +1410,7 @@ static int copy_counters_to_user(struct ebt_table *t,return -EINVAL ; } - counterstmp =vmalloc (nentries *sizeof (*counterstmp)); + counterstmp =vzalloc (nentries *sizeof (*counterstmp));if (!counterstmp)return -ENOMEM ;
Signature · 1 line
2.6.1
Pablo Neira Ayuso
maintainerQuoted 2 lines · Loganaden Velvindron
counterstmp is not cleared before it is used in
get_counters(). it might be leaked partially when it is sent to userland later on.
get_counters() is memcpy'ing the old counter to the counterstmp area and updating it.
Where is there leak?
Quoted 33 lines
Signed-off-by Loganaden Velvindron <[email protected]>--- net/bridge/netfilter/ebtables.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)net/bridge/netfilter/ebtables.c +2
-2@@ -989,7 +989,7 @@ static int do_replace_finish(struct net *net, struct ebt_replace *repl,the check on the size is done later, when we have the lock */ if (repl->num_counters) {unsigned long size = repl->num_counters *sizeof (*counterstmp); - counterstmp =vmalloc (size); + counterstmp =vzalloc (size);if (!counterstmp)return -ENOMEM ; }@@ -1410,7 +1410,7 @@ static int copy_counters_to_user(struct ebt_table *t,return -EINVAL ; } - counterstmp =vmalloc (nentries *sizeof (*counterstmp)); + counterstmp =vzalloc (nentries *sizeof (*counterstmp));if (!counterstmp)return -ENOMEM ;Signature · 5 lines
2.6.1 -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
Quoted 9 lines · Pablo Neira Ayuso
Quoted 2 lines · Loganaden Velvindron
counterstmp is not cleared before it is used in
get_counters(). it might be leaked partially when it is sent to userland later on.
get_counters()is memcpy'ing the old counter to the counterstmp area and updating it.Where is there leak?
In this case, none, due to correct memcpy parameters in get_counters().
However, as this is exposed later on to userland through:
if (copy_to_user(user, counterstmp,
nentries * sizeof(struct ebt_counter)))
zero'ing counterstmp malloc'ed memory, looks like a good idea.
Pablo Neira Ayuso
maintainerQuoted 11 lines · Loganaden Velvindron
Quoted 9 lines · Pablo Neira Ayuso
Quoted 2 lines · Loganaden Velvindron
counterstmp is not cleared before it is used in
get_counters(). it might be leaked partially when it is sent to userland later on.
get_counters()is memcpy'ing the old counter to the counterstmp area and updating it.Where is there leak?
Quoted 8 lines
In this case, none, due to correct memcpy parameters in
get_counters().However, as this is exposed later on to userland through:
if (copy_to_user(user, counterstmp,
nentries *sizeof(struct ebt_counter)))zero'ing counterstmp malloc'ed memory, looks like a good idea.
We allocate room for nentries, then we get_counters() those nentries that memcpy that area, and we copy back to user nentries.
Where is the problem?
Quoted 19 lines · Pablo Neira Ayuso
Quoted 18 lines · Loganaden Velvindron
Quoted 9 lines · Pablo Neira Ayuso
Quoted 2 lines · Loganaden Velvindron
counterstmp is not cleared before it is used in
get_counters(). it might be leaked partially when it is sent to userland later on.
get_counters()is memcpy'ing the old counter to the counterstmp area and updating it.Where is there leak?
In this case, none, due to correct memcpy parameters in
get_counters().However, as this is exposed later on to userland through:
if (copy_to_user(user, counterstmp,
nentries *sizeof(struct ebt_counter)))
Quoted 6 lines
Quoted 1 line
zero'ing counterstmp malloc'ed memory, looks like a good idea.
We allocate room for nentries, then we
get_counters()those nentries that memcpy that area, and we copy back to user nentries.Where is the problem?
None in this case.