9pfs: fix integer overflow issue in xattr read/write
The v9fs_xattr_read() and v9fs_xattr_write() are passed a guest originated offset: they must ensure this offset does not go beyond the size of the extended attribute that was set in v9fs_xattrcreate(). Unfortunately, the current code implement these checks with unsafe calculations on 32 and 64 bit values, which may allow a malicious guest to cause OOB access anyway. Fix this by comparing the offset and the xattr size, which are both uint64_t, before trying to compute the effective number of bytes to read or write. Suggested-by: Greg Kurz <groug@kaod.org> Signed-off-by: Li Qiang <liqiang6-s@360.cn> Reviewed-by: Greg Kurz <groug@kaod.org> Reviewed-By: Guido Günther <agx@sigxcpu.org> Signed-off-by: Greg Kurz <groug@kaod.org>
This commit is contained in:
parent
8495f9ad26
commit
7e55d65c56
32
hw/9pfs/9p.c
32
hw/9pfs/9p.c
|
@ -1637,20 +1637,17 @@ static int v9fs_xattr_read(V9fsState *s, V9fsPDU *pdu, V9fsFidState *fidp,
|
||||||
{
|
{
|
||||||
ssize_t err;
|
ssize_t err;
|
||||||
size_t offset = 7;
|
size_t offset = 7;
|
||||||
int read_count;
|
uint64_t read_count;
|
||||||
int64_t xattr_len;
|
|
||||||
V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
|
V9fsVirtioState *v = container_of(s, V9fsVirtioState, state);
|
||||||
VirtQueueElement *elem = v->elems[pdu->idx];
|
VirtQueueElement *elem = v->elems[pdu->idx];
|
||||||
|
|
||||||
xattr_len = fidp->fs.xattr.len;
|
if (fidp->fs.xattr.len < off) {
|
||||||
read_count = xattr_len - off;
|
read_count = 0;
|
||||||
|
} else {
|
||||||
|
read_count = fidp->fs.xattr.len - off;
|
||||||
|
}
|
||||||
if (read_count > max_count) {
|
if (read_count > max_count) {
|
||||||
read_count = max_count;
|
read_count = max_count;
|
||||||
} else if (read_count < 0) {
|
|
||||||
/*
|
|
||||||
* read beyond XATTR value
|
|
||||||
*/
|
|
||||||
read_count = 0;
|
|
||||||
}
|
}
|
||||||
err = pdu_marshal(pdu, offset, "d", read_count);
|
err = pdu_marshal(pdu, offset, "d", read_count);
|
||||||
if (err < 0) {
|
if (err < 0) {
|
||||||
|
@ -1979,23 +1976,18 @@ static int v9fs_xattr_write(V9fsState *s, V9fsPDU *pdu, V9fsFidState *fidp,
|
||||||
{
|
{
|
||||||
int i, to_copy;
|
int i, to_copy;
|
||||||
ssize_t err = 0;
|
ssize_t err = 0;
|
||||||
int write_count;
|
uint64_t write_count;
|
||||||
int64_t xattr_len;
|
|
||||||
size_t offset = 7;
|
size_t offset = 7;
|
||||||
|
|
||||||
|
|
||||||
xattr_len = fidp->fs.xattr.len;
|
if (fidp->fs.xattr.len < off) {
|
||||||
write_count = xattr_len - off;
|
|
||||||
if (write_count > count) {
|
|
||||||
write_count = count;
|
|
||||||
} else if (write_count < 0) {
|
|
||||||
/*
|
|
||||||
* write beyond XATTR value len specified in
|
|
||||||
* xattrcreate
|
|
||||||
*/
|
|
||||||
err = -ENOSPC;
|
err = -ENOSPC;
|
||||||
goto out;
|
goto out;
|
||||||
}
|
}
|
||||||
|
write_count = fidp->fs.xattr.len - off;
|
||||||
|
if (write_count > count) {
|
||||||
|
write_count = count;
|
||||||
|
}
|
||||||
err = pdu_marshal(pdu, offset, "d", write_count);
|
err = pdu_marshal(pdu, offset, "d", write_count);
|
||||||
if (err < 0) {
|
if (err < 0) {
|
||||||
return err;
|
return err;
|
||||||
|
|
Loading…
Reference in a new issue