Skip to content

Commit 1c824e7

Browse files
pavitrajhagregkh
authored andcommitted
libceph: fix OOB read in decode_watchers() via missing bounds check
commit 00ead17 upstream. ceph_start_decoding() validates that struct_len bytes remain in the buffer after the encoding header, but accepts struct_len=0 as valid: ceph_decode_need(p, end, 0, bad) always passes. When a malicious or compromised OSD sends an obj_list_watch_response_t reply with struct_len=0, ceph_start_decoding() returns success with p == end, leaving zero bytes guaranteed for subsequent reads. The immediately following ceph_decode_32(p) in decode_watchers() has no preceding bounds check. With p == end this is a 4-byte read past the validated buffer boundary. The garbage value is then passed directly to kzalloc_objs() as the watcher count. The sibling function decode_watcher() already uses the safe variants (ceph_decode_copy_safe, ceph_decode_64_safe, ceph_decode_skip_32) after its own ceph_start_decoding() call. decode_watchers() is the only site that uses the bare variant, confirming an oversight. Fix by replacing ceph_decode_32(p) with ceph_decode_32_safe(p, end, *num_watchers, bad), consistent with the established pattern. Attacker model: a malicious or compromised OSD in a multi-tenant Ceph deployment (e.g. cloud) can trigger this against any kernel client that calls CEPH_OSD_OP_LIST_WATCHERS, without any further privileges beyond OSD session establishment. [ idryomov: trim changelog ] Cc: stable@vger.kernel.org Fixes: a4ed38d ("libceph: support for CEPH_OSD_OP_LIST_WATCHERS") Signed-off-by: Pavitra Jha <jhapavitra98@gmail.com> Reviewed-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com> Signed-off-by: Ilya Dryomov <idryomov@gmail.com> [ kept the tree's `kcalloc()` context line instead of upstream's `kzalloc_objs()` ] Signed-off-by: Sasha Levin <sashal@kernel.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
1 parent 3bad5e5 commit 1c824e7

1 file changed

Lines changed: 4 additions & 1 deletion

File tree

‎net/ceph/osd_client.c‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4995,7 +4995,7 @@ static int decode_watchers(void **p, void *end,
49954995
if (ret)
49964996
return ret;
49974997

4998-
*num_watchers = ceph_decode_32(p);
4998+
ceph_decode_32_safe(p, end, *num_watchers, bad);
49994999
*watchers = kcalloc(*num_watchers, sizeof(**watchers), GFP_NOIO);
50005000
if (!*watchers)
50015001
return -ENOMEM;
@@ -5009,6 +5009,9 @@ static int decode_watchers(void **p, void *end,
50095009
}
50105010

50115011
return 0;
5012+
5013+
bad:
5014+
return -EINVAL;
50125015
}
50135016

50145017
/*

0 commit comments

Comments
 (0)