On Jun 6, 2012, at 12:26 PM, Alex Elder wrote:
>
> Just above here we see:
> /* pg_temp */
> ceph_decode_32_safe(p, end, len, bad);
> for (i = 0; i < len; i++) {
>
> We haven't validated "len" here either. Looking at it I'm not sure
> we can do much, but I think we do know a few things should be true:
> - (len & (sizeof (u32) - 1)) == 0
> - len <= (UINT_MAX / (sizeof (struct ceph_pg) + sizeof (u32)))
> and further, if it's invalid to have a value for pg->len of
> zero, then we can instead assert:
> - len <= (UINT_MAX / (sizeof (struct ceph_pg) + 2 * sizeof (u32)))
>
> I don't know if it's that important do do a check like this though.
I don't see any overflow issue here. Are you worried about the loop
running for a while given a large n? How about this check?
/* pg_temp */
ceph_decode_32_safe(p, end, len, bad);
+ if (len > UINT_MAX / (sizeof(u32) + sizeof(u64)))
+ goto bad;
+ ceph_decode_need(p, end, len * (sizeof(u32) + sizeof(u64)), bad);
- xi
--
To unsubscribe from this list: send the line "unsubscribe ceph-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
