libceph: fixup error handling in osdmap_apply_incremental()

The existing error handling scheme requires resetting err to -EINVAL
prior to calling any ceph_decode_* macro.  This is ugly and fragile,
and there already are a few places where we would return 0 on error,
due to a missing reset.  Follow osdmap_decode() and fix this by adding
a special e_inval label to be used by all ceph_decode_* macros.

Signed-off-by: Ilya Dryomov <ilya.dryomov@inktank.com>
Reviewed-by: Alex Elder <elder@linaro.org>

1 files changed, 34 insertions, 32 deletions

diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c
index d4a6b0df3627..b844a9273666 100644
--- a/net/ceph/osdmap.c
+++ b/net/ceph/osdmap.c
@@ -867,19 +867,19 @@ struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end,
         __s64 new_pool_max;
         __s32 new_flags, max;
         void *start = *p;
-        int err = -EINVAL;
+        int err;
         u16 version;
 
         dout("%s %p to %p len %d\n", __func__, *p, end, (int)(end - *p));
 
-        ceph_decode_16_safe(p, end, version, bad);
+        ceph_decode_16_safe(p, end, version, e_inval);
         if (version != 6) {
                 pr_warning("got unknown v %d != 6 of inc osdmap\n", version);
-                goto bad;
+                goto e_inval;
         }
 
         ceph_decode_need(p, end, sizeof(fsid)+sizeof(modified)+2*sizeof(u32),
-                         bad);
+                         e_inval);
         ceph_decode_copy(p, &fsid, sizeof(fsid));
         epoch = ceph_decode_32(p);
         BUG_ON(epoch != map->epoch+1);
@@ -888,7 +888,7 @@ struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end,
         new_flags = ceph_decode_32(p);
 
         /* full map? */
-        ceph_decode_32_safe(p, end, len, bad);
+        ceph_decode_32_safe(p, end, len, e_inval);
         if (len > 0) {
                 dout("apply_incremental full map len %d, %p to %p\n",
                      len, *p, end);
@@ -896,13 +896,14 @@ struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end,
         }
 
         /* new crush? */
-        ceph_decode_32_safe(p, end, len, bad);
+        ceph_decode_32_safe(p, end, len, e_inval);
         if (len > 0) {
-                dout("apply_incremental new crush map len %d, %p to %p\n",
-                     len, *p, end);
                 newcrush = crush_decode(*p, min(*p+len, end));
-                if (IS_ERR(newcrush))
-                        return ERR_CAST(newcrush);
+                if (IS_ERR(newcrush)) {
+                        err = PTR_ERR(newcrush);
+                        newcrush = NULL;
+                        goto bad;
+                }
                 *p += len;
         }
 
@@ -912,13 +913,13 @@ struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end,
         if (new_pool_max >= 0)
                 map->pool_max = new_pool_max;
 
-        ceph_decode_need(p, end, 5*sizeof(u32), bad);
+        ceph_decode_need(p, end, 5*sizeof(u32), e_inval);
 
         /* new max? */
         max = ceph_decode_32(p);
         if (max >= 0) {
                 err = osdmap_set_max_osd(map, max);
-                if (err < 0)
+                if (err)
                         goto bad;
         }
 
@@ -932,11 +933,11 @@ struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end,
         }
 
         /* new_pool */
-        ceph_decode_32_safe(p, end, len, bad);
+        ceph_decode_32_safe(p, end, len, e_inval);
         while (len--) {
                 struct ceph_pg_pool_info *pi;
 
-                ceph_decode_64_safe(p, end, pool, bad);
+                ceph_decode_64_safe(p, end, pool, e_inval);
                 pi = __lookup_pg_pool(&map->pg_pools, pool);
                 if (!pi) {
                         pi = kzalloc(sizeof(*pi), GFP_NOFS);
@@ -953,29 +954,28 @@ struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end,
         }
         if (version >= 5) {
                 err = __decode_pool_names(p, end, map);
-                if (err < 0)
+                if (err)
                         goto bad;
         }
 
         /* old_pool */
-        ceph_decode_32_safe(p, end, len, bad);
+        ceph_decode_32_safe(p, end, len, e_inval);
         while (len--) {
                 struct ceph_pg_pool_info *pi;
 
-                ceph_decode_64_safe(p, end, pool, bad);
+                ceph_decode_64_safe(p, end, pool, e_inval);
                 pi = __lookup_pg_pool(&map->pg_pools, pool);
                 if (pi)
                         __remove_pg_pool(&map->pg_pools, pi);
         }
 
         /* new_up */
-        err = -EINVAL;
-        ceph_decode_32_safe(p, end, len, bad);
+        ceph_decode_32_safe(p, end, len, e_inval);
         while (len--) {
                 u32 osd;
                 struct ceph_entity_addr addr;
-                ceph_decode_32_safe(p, end, osd, bad);
-                ceph_decode_copy_safe(p, end, &addr, sizeof(addr), bad);
+                ceph_decode_32_safe(p, end, osd, e_inval);
+                ceph_decode_copy_safe(p, end, &addr, sizeof(addr), e_inval);
                 ceph_decode_addr(&addr);
                 pr_info("osd%d up\n", osd);
                 BUG_ON(osd >= map->max_osd);
@@ -984,11 +984,11 @@ struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end,
         }
 
         /* new_state */
-        ceph_decode_32_safe(p, end, len, bad);
+        ceph_decode_32_safe(p, end, len, e_inval);
         while (len--) {
                 u32 osd;
                 u8 xorstate;
-                ceph_decode_32_safe(p, end, osd, bad);
+                ceph_decode_32_safe(p, end, osd, e_inval);
                 xorstate = **(u8 **)p;
                 (*p)++;  /* clean flag */
                 if (xorstate == 0)
@@ -1000,10 +1000,10 @@ struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end,
         }
 
         /* new_weight */
-        ceph_decode_32_safe(p, end, len, bad);
+        ceph_decode_32_safe(p, end, len, e_inval);
         while (len--) {
                 u32 osd, off;
-                ceph_decode_need(p, end, sizeof(u32)*2, bad);
+                ceph_decode_need(p, end, sizeof(u32)*2, e_inval);
                 osd = ceph_decode_32(p);
                 off = ceph_decode_32(p);
                 pr_info("osd%d weight 0x%x %s\n", osd, off,
@@ -1014,7 +1014,7 @@ struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end,
         }
 
         /* new_pg_temp */
-        ceph_decode_32_safe(p, end, len, bad);
+        ceph_decode_32_safe(p, end, len, e_inval);
         while (len--) {
                 struct ceph_pg_mapping *pg;
                 int j;
@@ -1024,22 +1024,22 @@ struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end,
                 err = ceph_decode_pgid(p, end, &pgid);
                 if (err)
                         goto bad;
-                ceph_decode_need(p, end, sizeof(u32), bad);
+                ceph_decode_need(p, end, sizeof(u32), e_inval);
                 pglen = ceph_decode_32(p);
                 if (pglen) {
-                        ceph_decode_need(p, end, pglen*sizeof(u32), bad);
+                        ceph_decode_need(p, end, pglen*sizeof(u32), e_inval);
 
                         /* removing existing (if any) */
                         (void) __remove_pg_mapping(&map->pg_temp, pgid);
 
                         /* insert */
-                        err = -EINVAL;
                         if (pglen > (UINT_MAX - sizeof(*pg)) / sizeof(u32))
-                                goto bad;
-                        err = -ENOMEM;
+                                goto e_inval;
                         pg = kmalloc(sizeof(*pg) + sizeof(u32)*pglen, GFP_NOFS);
-                        if (!pg)
+                        if (!pg) {
+                                err = -ENOMEM;
                                 goto bad;
+                        }
                         pg->pgid = pgid;
                         pg->len = pglen;
                         for (j = 0; j < pglen; j++)
@@ -1063,6 +1063,8 @@ struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end,
         dout("inc osdmap epoch %d max_osd %d\n", map->epoch, map->max_osd);
         return map;
 
+e_inval:
+        err = -EINVAL;
 bad:
         pr_err("corrupt inc osdmap (%d) epoch %d off %d (%p of %p-%p)\n",
                err, epoch, (int)(*p - start), *p, start, end);