[PATCH net] libceph: osdmap: Fix some NULL dereferences

[PATCH net] libceph: osdmap: Fix some NULL dereferences

[Dan Carpenter]

There are hidden gotos in the ceph_decode_* macros.  We need to set the
"err" variable on these error paths otherwise we end up returning
ERR_PTR(0) which is NULL.  It causes NULL dereferences in the callers.

Fixes: 278b1d709c6a ("libceph: ceph_decode_skip_* helpers")
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>

diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c
index 864789c5974e..c7521a847ef7 100644
--- a/net/ceph/osdmap.c
+++ b/net/ceph/osdmap.c
@@ -510,6 +510,7 @@ static struct crush_map *crush_decode(void *pbyval, void *end)
 		}
 	}
 
+	err = -EINVAL;
 	ceph_decode_skip_map(p, end, 32, string, bad); /* type_map */
 	ceph_decode_skip_map(p, end, 32, string, bad); /* name_map */
 	ceph_decode_skip_map(p, end, 32, string, bad); /* rule_name_map */
@@ -1825,9 +1826,9 @@ struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end,
 	if (struct_v >= 3) {
 		/* new_erasure_code_profiles */
 		ceph_decode_skip_map_of_map(p, end, string, string, string,
-					    bad);
+					    e_inval);
 		/* old_erasure_code_profiles */
-		ceph_decode_skip_set(p, end, string, bad);
+		ceph_decode_skip_set(p, end, string, e_inval);
 	}
 
 	if (struct_v >= 4) {

Re: [PATCH net] libceph: osdmap: Fix some NULL dereferences

[Ilya Dryomov]

On Thu, Jul 13, 2017 at 9:45 AM, Dan Carpenter <dan.carpenter@oracle.com> wrote:
> There are hidden gotos in the ceph_decode_* macros.  We need to set the
> "err" variable on these error paths otherwise we end up returning
> ERR_PTR(0) which is NULL.  It causes NULL dereferences in the callers.
>
> Fixes: 278b1d709c6a ("libceph: ceph_decode_skip_* helpers")
> Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
>
> diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c
> index 864789c5974e..c7521a847ef7 100644
> --- a/net/ceph/osdmap.c
> +++ b/net/ceph/osdmap.c
> @@ -510,6 +510,7 @@ static struct crush_map *crush_decode(void *pbyval, void *end)
>                 }
>         }
>
> +       err = -EINVAL;
>         ceph_decode_skip_map(p, end, 32, string, bad); /* type_map */
>         ceph_decode_skip_map(p, end, 32, string, bad); /* name_map */
>         ceph_decode_skip_map(p, end, 32, string, bad); /* rule_name_map */
> @@ -1825,9 +1826,9 @@ struct ceph_osdmap *osdmap_apply_incremental(void **p, void *end,
>         if (struct_v >= 3) {
>                 /* new_erasure_code_profiles */
>                 ceph_decode_skip_map_of_map(p, end, string, string, string,
> -                                           bad);
> +                                           e_inval);
>                 /* old_erasure_code_profiles */
> -               ceph_decode_skip_set(p, end, string, bad);
> +               ceph_decode_skip_set(p, end, string, e_inval);
>         }
>
>         if (struct_v >= 4) {

Hi Dan,

I applied osdmap_apply_incremental() hunk and fixed a similar bug in
osdmap_decode() (it's a not NULL deref, that's why smatch didn't catch
it).

I posted a separate patch for crush_decode().  This is the second time
in two months you are fixing it, so I wanted something more future-proof.

See the end result at https://github.com/ceph/ceph-client/commits/testing.

Thanks,

                Ilya

Re: [PATCH net] libceph: osdmap: Fix some NULL dereferences

[Dan Carpenter]

On Thu, Jul 13, 2017 at 06:13:22PM +0200, Ilya Dryomov wrote:
> 
> Hi Dan,
> 
> I applied osdmap_apply_incremental() hunk and fixed a similar bug in
> osdmap_decode() (it's a not NULL deref, that's why smatch didn't catch
> it).
> 

No...  :/  Smatch complained about it, but I somehow marked it as done
yesterday without sending a patch.  Thanks for catching that.

The Smatch check is just looking for places where we do ERR_PTR(0) or
ERR_PTR(valid_ptr).  That can be intentional, so there is some hand
reviewing involved.

regards,
dan carpenter