mirror of
https://github.com/torvalds/linux.git
synced 2026-07-27 17:47:41 +02:00
libceph: fix two unsafe bare decodes in decode_lockers()
decode_lockers() in cls_lock_client.c contains two bare decode operations
that allow a malicious or compromised OSD to trigger slab-out-of-bounds
reads:
1. ceph_decode_32(p) at the num_lockers field has no preceding bounds
check. ceph_start_decoding() accepts struct_len=0 as valid -- the
internal ceph_decode_need(p, end, 0, bad) always passes -- so when an
OSD sends struct_len=0, ceph_start_decoding() returns success with
p == end. The immediately following bare ceph_decode_32(p) then reads
4 bytes past the validated buffer boundary. The garbage value is
passed directly to kzalloc_objs() as the locker count.
The sibling function decode_watchers() in osd_client.c already uses
ceph_decode_32_safe() after its own ceph_start_decoding() call.
decode_lockers() was the only site using the bare variant.
2. ceph_decode_8(p) after the decode_locker() loop has no preceding
bounds check. If an OSD crafts num_lockers such that the loop
advances p exactly to end, the subsequent bare ceph_decode_8(p) reads
one byte past the validated buffer boundary. The result is passed
directly into *type, which is used as a lock type discriminator by
callers, giving an OSD-controlled one-byte OOB read with direct
influence over the lock type field.
Fix both by replacing bare operations with their safe variants:
ceph_decode_32(p) -> ceph_decode_32_safe(p, end, *num_lockers,
err_inval)
ceph_decode_8(p) -> ceph_decode_8_safe(p, end, *type,
err_free_lockers)
The goto targets differ intentionally:
err_inval: is a new label returning -EINVAL directly. It is used for
the pre-allocation failure path where *lockers is not yet allocated
and must not be passed to ceph_free_lockers().
err_free_lockers: is the existing label. It is used for the
post-allocation failure path where *lockers is allocated and must
be freed.
ret is set to -EINVAL before ceph_decode_8_safe() so that
err_free_lockers returns the correct error code on bounds violation.
Without this, err_free_lockers would return a stale ret value (0 from
the successful decode_locker() loop), silently swallowing the error.
-EINVAL is correct for both failure paths. The data received from the
OSD is structurally malformed. -ENOMEM would misrepresent the failure
class to callers and to stable@ backporters triaging error paths.
Attacker model: a malicious or compromised OSD in a multi-tenant Ceph
deployment can trigger this against any kernel client that issues the
lock.get_info class method (e.g. during RBD exclusive lock acquisition).
[ idryomov: trim changelog, formatting ]
Cc: stable@vger.kernel.org
Fixes: d4ed4a5305 ("libceph: support for lock.lock_info")
Signed-off-by: Pavitra Jha <jhapavitra98@gmail.com>
Reviewed-by: Viacheslav Dubeyko <Slava.Dubeyko@ibm.com>
Signed-off-by: Ilya Dryomov <idryomov@gmail.com>
This commit is contained in:
parent
4dbc71bcaf
commit
a109a55611
|
|
@ -299,7 +299,7 @@ static int decode_lockers(void **p, void *end, u8 *type, char **tag,
|
|||
if (ret)
|
||||
return ret;
|
||||
|
||||
*num_lockers = ceph_decode_32(p);
|
||||
ceph_decode_32_safe(p, end, *num_lockers, err_inval);
|
||||
*lockers = kzalloc_objs(**lockers, *num_lockers, GFP_NOIO);
|
||||
if (!*lockers)
|
||||
return -ENOMEM;
|
||||
|
|
@ -310,7 +310,8 @@ static int decode_lockers(void **p, void *end, u8 *type, char **tag,
|
|||
goto err_free_lockers;
|
||||
}
|
||||
|
||||
*type = ceph_decode_8(p);
|
||||
ret = -EINVAL;
|
||||
ceph_decode_8_safe(p, end, *type, err_free_lockers);
|
||||
s = ceph_extract_encoded_string(p, end, NULL, GFP_NOIO);
|
||||
if (IS_ERR(s)) {
|
||||
ret = PTR_ERR(s);
|
||||
|
|
@ -320,6 +321,9 @@ static int decode_lockers(void **p, void *end, u8 *type, char **tag,
|
|||
*tag = s;
|
||||
return 0;
|
||||
|
||||
err_inval:
|
||||
return -EINVAL;
|
||||
|
||||
err_free_lockers:
|
||||
ceph_free_lockers(*lockers, *num_lockers);
|
||||
return ret;
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user