Commit 86f1742b authored by Ilya Dryomov's avatar Ilya Dryomov Committed by Sage Weil
Browse files

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: default avatarIlya Dryomov <ilya.dryomov@inktank.com>
Reviewed-by: default avatarAlex Elder <elder@linaro.org>
parent 9902e682

net/ceph/osdmap.c

+34 −32
Original line number Diff line number Diff line
@@ -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);

CRA Git | Maintained and supported by SUSTech CRA and CCSE