Re: [PATCH v2 1/3] libceph: fix overflow in __decode_pool_names()

2012-06-06 Thread Xi Wang
On Jun 6, 2012, at 12:26 PM, Alex Elder wrote: diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c index 29ad46e..f80afc3 100644 --- a/net/ceph/osdmap.c +++ b/net/ceph/osdmap.c @@ -495,15 +495,12 @@ static int __decode_pool_names(void **p, void *end, struct ceph_osdmap *map)

Re: [PATCH 2/3] libceph: fix overflow in osdmap_decode()

2012-06-06 Thread Xi Wang
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

Re: [PATCH 2/3] libceph: fix overflow in osdmap_decode()

2012-06-06 Thread Xi Wang
On Jun 6, 2012, at 3:14 PM, Alex Elder wrote: It probably won't matter because in time if the value is too large then one of the checks inside the loop might bail out. But catching it as early as possible is always better. Yeah I Agree. :-) - xi -- To unsubscribe from this list: send the

[PATCH 1/3] ligceph: fix overflow in __decode_pool_names()

2012-04-29 Thread Xi Wang
use kstrndup rather than kmalloc/memcpy. Signed-off-by: Xi Wang xi.w...@gmail.com --- net/ceph/osdmap.c |9 +++-- 1 files changed, 3 insertions(+), 6 deletions(-) diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c index 29ad46e..f80afc3 100644 --- a/net/ceph/osdmap.c +++ b/net/ceph/osdmap.c

[PATCH 3/3] libceph: fix overflow in osdmap_apply_incremental()

2012-04-29 Thread Xi Wang
On 32-bit systems, a large `pglen' would overflow `pglen*sizeof(u32)' and bypass the check ceph_decode_need(p, end, pglen*sizeof(u32), bad). It would also overflow the subsequent kmalloc() size, leading to out-of-bounds write. Signed-off-by: Xi Wang xi.w...@gmail.com --- net/ceph/osdmap.c |4

[PATCH v2 1/3] libceph: fix overflow in __decode_pool_names()

2012-04-29 Thread Xi Wang
use kstrndup rather than kmalloc/memcpy. Signed-off-by: Xi Wang xi.w...@gmail.com --- Subject corrected. Sorry, my bad. --- net/ceph/osdmap.c |9 +++-- 1 files changed, 3 insertions(+), 6 deletions(-) diff --git a/net/ceph/osdmap.c b/net/ceph/osdmap.c index 29ad46e..f80afc3 100644

Re: [PATCH] rbd: fix integer overflow in rbd_header_from_disk()

2012-04-18 Thread Xi Wang
On Apr 18, 2012, at 10:21 AM, Alex Elder wrote: This looks good, however I have changed it to use UINT_MAX rather than ULONG_MAX, because on some architectures size_t (__kernel_size_t) is defined as type (unsigned int). It is the more conservative value, and even on architectures where

Re: [PATCH] rbd: fix integer overflow in rbd_header_from_disk()

2012-04-18 Thread Xi Wang
On Apr 18, 2012, at 2:47 PM, Alex Elder wrote: It may not matter in practice, but I prefer to be very careful. As long as you're going out of your way to avoid overflow might as well make sure it works on all architectures in the process. I agree. Probably a cleaner way is to define

[PATCH RESEND] ceph: avoid panic with mismatched symlink sizes in fill_inode()

2012-02-03 Thread Xi Wang
Return -EINVAL rather than panic if iinfo-symlink_len and inode-i_size do not match. Also use kstrndup rather than kmalloc/memcpy. Signed-off-by: Xi Wang xi.w...@gmail.com --- fs/ceph/inode.c | 11 ++- 1 files changed, 6 insertions(+), 5 deletions(-) diff --git a/fs/ceph/inode.c b/fs

[PATCH v2] ceph: avoid panic with mismatched symlink sizes in fill_inode()

2012-02-03 Thread Xi Wang
Return -EINVAL rather than panic if iinfo-symlink_len and inode-i_size do not match. Also use kstrndup rather than kmalloc/memcpy. Signed-off-by: Xi Wang xi.w...@gmail.com --- fs/ceph/inode.c | 11 ++- 1 files changed, 6 insertions(+), 5 deletions(-) diff --git a/fs/ceph/inode.c b/fs

[PATCH RESEND] ceph: fix length validation in parse_reply_info()

2012-01-14 Thread Xi Wang
len is read from network and thus needs validation. Otherwise, given a bogus len value, p+len could be an out-of-bounds pointer, which is used in further parsing. Signed-off-by: Xi Wang xi.w...@gmail.com --- fs/ceph/mds_client.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff

Re: [PATCH RESEND] ceph: fix length validation in parse_reply_info()

2012-01-14 Thread Xi Wang
On Jan 15, 2012, at 12:18 AM, Sage Weil wrote: Applied. Sorry I missed this the first time around! Thanks. ;-) - xi -- To unsubscribe from this list: send the line unsubscribe ceph-devel in the body of a message to majord...@vger.kernel.org More majordomo info at

[PATCH RESEND] ceph: fix bounds checking macros

2012-01-14 Thread Xi Wang
The bounds check (*p + n end) can be bypassed with a large n due to pointer wraparound, especially when n is read from network. Change the check to a safer form (n end - *p). Signed-off-by: Xi Wang xi.w...@gmail.com --- include/linux/ceph/decode.h |9 +++-- 1 files changed, 7

[PATCH 0/3] ceph: fix multiple out-of-bounds checks

2011-12-14 Thread Xi Wang
Fix several possible out-of-bounds pointers if the incoming message is corrupted or malicious. Xi Wang (3): ceph: fix out-of-bounds pointers in parse_reply_info() ceph: fix bounds check macros ceph_decode_need and ceph_encode_need ceph: avoid panic with mismatched symbolic link sizes

[PATCH 1/3] ceph: fix out-of-bounds pointers in parse_reply_info()

2011-12-14 Thread Xi Wang
Given a large len, the pointer p+len used in further parsing could be out of bounds. Signed-off-by: Xi Wang xi.w...@gmail.com --- fs/ceph/mds_client.c |2 ++ 1 files changed, 2 insertions(+), 0 deletions(-) diff --git a/fs/ceph/mds_client.c b/fs/ceph/mds_client.c index 6203d80..be1415f

[PATCH 3/3] ceph: avoid panic with mismatched symbolic link sizes in fill_inode()

2011-12-14 Thread Xi Wang
Return -EINVAL with mismatched iinfo-symlink_len and inode-i_size. Also use kstrndup rather than kmalloc/memcpy. Signed-off-by: Xi Wang xi.w...@gmail.com --- fs/ceph/inode.c | 10 +- 1 files changed, 5 insertions(+), 5 deletions(-) diff --git a/fs/ceph/inode.c b/fs/ceph/inode.c index

[PATCH 2/3] ceph: fix bounds check macros ceph_decode_need and ceph_encode_need

2011-12-14 Thread Xi Wang
Given a large n, the bounds check (*p + n end) can be bypassed due to pointer wraparound. A safer check is (n end - *p). Signed-off-by: Xi Wang xi.w...@gmail.com --- include/linux/ceph/decode.h |9 +++-- 1 files changed, 7 insertions(+), 2 deletions(-) diff --git a/include/linux/ceph