[PATCH] libceph: fix a use after free issue in osdmap_set_max_osd

[r***@gmail.com]

From: Li RongQing <***@gmail.com>

If the state variable is krealloced successfully, map->osd_state will be
freed, once following two reallocation failed, and exit the function
without resetting map->osd_state, map->osd_state become a wild pointer.

fix it by resetting them after krealloc successfully.

Signed-off-by: Li RongQing <***@gmail.com>
---
net/ceph/osdmap.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c
index c547e46..81e9c66 100644
--- a/net/ceph/osdmap.c
+++ b/net/ceph/osdmap.c
@@ -671,15 +671,19 @@ static int osdmap_set_max_osd(struct ceph_osdmap *map, int max)
int i;

state = krealloc(map->osd_state, max*sizeof(*state), GFP_NOFS);
+ if (!state)
+ return -ENOMEM;
+ map->osd_state = state;
+
weight = krealloc(map->osd_weight, max*sizeof(*weight), GFP_NOFS);
- addr = krealloc(map->osd_addr, max*sizeof(*addr), GFP_NOFS);
- if (!state || !weight || !addr) {
- kfree(state);
- kfree(weight);
- kfree(addr);
+ if (!weight)
+ return -ENOMEM;
+ map->osd_weight = weight;

+ addr = krealloc(map->osd_addr, max*sizeof(*addr), GFP_NOFS);
+ if (!addr)
return -ENOMEM;
- }
+ map->osd_addr = addr;

for (i = map->max_osd; i < max; i++) {
state[i] = 0;
@@ -687,10 +691,6 @@ static int osdmap_set_max_osd(struct ceph_osdmap *map, int max)
memset(addr + i, 0, sizeof(*addr));
}

- map->osd_state = state;
- map->osd_weight = weight;
- map->osd_addr = addr;
-
if (map->osd_primary_affinity) {
u32 *affinity;

[Ilya Dryomov]

Post by r***@gmail.com
If the state variable is krealloced successfully, map->osd_state will be
freed, once following two reallocation failed, and exit the function
without resetting map->osd_state, map->osd_state become a wild pointer.
fix it by resetting them after krealloc successfully.
---
net/ceph/osdmap.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c
index c547e46..81e9c66 100644
--- a/net/ceph/osdmap.c
+++ b/net/ceph/osdmap.c
@@ -671,15 +671,19 @@ static int osdmap_set_max_osd(struct ceph_osdmap *map, int max)
int i;
state = krealloc(map->osd_state, max*sizeof(*state), GFP_NOFS);
+ if (!state)
+ return -ENOMEM;
+ map->osd_state = state;
+
weight = krealloc(map->osd_weight, max*sizeof(*weight), GFP_NOFS);
- addr = krealloc(map->osd_addr, max*sizeof(*addr), GFP_NOFS);
- if (!state || !weight || !addr) {
- kfree(state);
- kfree(weight);
- kfree(addr);
+ if (!weight)
+ return -ENOMEM;
+ map->osd_weight = weight;
+ addr = krealloc(map->osd_addr, max*sizeof(*addr), GFP_NOFS);
+ if (!addr)
return -ENOMEM;
- }
+ map->osd_addr = addr;
for (i = map->max_osd; i < max; i++) {
state[i] = 0;
@@ -687,10 +691,6 @@ static int osdmap_set_max_osd(struct ceph_osdmap *map, int max)
memset(addr + i, 0, sizeof(*addr));
}
- map->osd_state = state;
- map->osd_weight = weight;
- map->osd_addr = addr;
-
if (map->osd_primary_affinity) {
u32 *affinity;
--
1.7.10.4

Looks good. I'll apply it tomorrow.

Thanks,

Ilya
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html

[Ilya Dryomov]

Post by Ilya Dryomov

Post by r***@gmail.com
If the state variable is krealloced successfully, map->osd_state will be
freed, once following two reallocation failed, and exit the function
without resetting map->osd_state, map->osd_state become a wild pointer.
fix it by resetting them after krealloc successfully.
---
net/ceph/osdmap.c | 20 ++++++++++----------
1 file changed, 10 insertions(+), 10 deletions(-)
diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c
index c547e46..81e9c66 100644
--- a/net/ceph/osdmap.c
+++ b/net/ceph/osdmap.c
@@ -671,15 +671,19 @@ static int osdmap_set_max_osd(struct ceph_osdmap *map, int max)
int i;
state = krealloc(map->osd_state, max*sizeof(*state), GFP_NOFS);
+ if (!state)
+ return -ENOMEM;
+ map->osd_state = state;
+
weight = krealloc(map->osd_weight, max*sizeof(*weight), GFP_NOFS);
- addr = krealloc(map->osd_addr, max*sizeof(*addr), GFP_NOFS);
- if (!state || !weight || !addr) {
- kfree(state);
- kfree(weight);
- kfree(addr);
+ if (!weight)
+ return -ENOMEM;
+ map->osd_weight = weight;
+ addr = krealloc(map->osd_addr, max*sizeof(*addr), GFP_NOFS);
+ if (!addr)
return -ENOMEM;
- }
+ map->osd_addr = addr;
for (i = map->max_osd; i < max; i++) {
state[i] = 0;
@@ -687,10 +691,6 @@ static int osdmap_set_max_osd(struct ceph_osdmap *map, int max)
memset(addr + i, 0, sizeof(*addr));
}
- map->osd_state = state;
- map->osd_weight = weight;
- map->osd_addr = addr;
-
if (map->osd_primary_affinity) {
u32 *affinity;
--
1.7.10.4

Looks good. I'll apply it tomorrow.

Pushed a branch with your patch. Minor modifications: use map->*
instead of local variables for initialization and change primary
affinity case so it fits in. You can take a look at

https://github.com/ceph/ceph-client/commit/790415a024871a2100388ce4b3d485756fb90865

Thanks,

Ilya
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to ***@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html