Hello!
Next patches fix two NFSv4 client vulnerabilities.
First (memory corruption) can be raises anytime whereas second
(dereference ->lookup and call NULL pointer) can be raised during mount.
Repeatability 100%
--
Thanks,
Vitaliy Gusev
There is a remote vulnerability for nfs4-client. That allows
a remote NFSv4 server or attacker in middle rewrite memory on
NFSv4 client.
Of course if encryption is used then only server can do corruption.
1. Client send GETATTR request to server, but there are not
checks for bitmask in reply from server. Therefore server may return
more attributes than were asked.
2. Pointer to struct nfs_fattr is passed to decode_getfattr(), but
that parameter is considered as pointer to struct nfs4_fs_locations
in case set FATTR4_WORD0_FS_LOCATIONS in reply from server.
But there is no check for pointer type that was passed!
3. Remote side can set FATTR4_WORD0_FS_LOCATIONS and fill
datas fsattr4_fs_locations in reply to client.
4. sizeof(nfs4_fs_locations) is about 92080 bytes in kernel 2.6.38
So ~ 90 Kbytes can be overwritten in memory of NFSv4 client.
Fortunately, nfs_fattr is allocated via kmalloc and
is not stored in stack. So generally kmalloc-192 is corrupted.
Affected kernels: all from v2.6.18
Commit 683b57b435326eb512c7305892683b6205669448
"NFSv4: Implement the fs_locations function call"
5.
Simple testcase for NFSv4 server: add next code at begin of
nfsd4_encode_fattr()
if (!strcmp("filexxx", dentry->d_name.name)) {
bmval[0] |= FATTR4_WORD0_FS_LOCATIONS;
bmval0 |= FATTR4_WORD0_FS_LOCATIONS;
}
NFSv4 client that tries to touch 'filexxx' will catch kernel panic.
Signed-off-by: Vitaliy Gusev <[email protected]>
---
fs/nfs/nfs4xdr.c | 16 ++++++++++++----
1 files changed, 12 insertions(+), 4 deletions(-)
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index 242b392..8dbfd9a 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -4301,7 +4301,8 @@ xdr_error:
}
static int decode_getfattr_generic(struct xdr_stream *xdr, struct
nfs_fattr *fattr,
- struct nfs_fh *fh, const struct nfs_server *server, int may_sleep)
+ struct nfs_fh *fh, const struct nfs_server *server, int may_sleep,
+ int expect_fsloc)
{
__be32 *savep;
uint32_t attrlen,
@@ -4316,6 +4317,9 @@ static int decode_getfattr_generic(struct
xdr_stream *xdr, struct nfs_fattr *fat
if (status < 0)
goto xdr_error;
+ if (!expect_fsloc)
+ bitmap[0] &= ~FATTR4_WORD0_FS_LOCATIONS;
+
status = decode_attr_length(xdr, &attrlen, &savep);
if (status < 0)
goto xdr_error;
@@ -4333,7 +4337,7 @@ xdr_error:
static int decode_getfattr(struct xdr_stream *xdr, struct nfs_fattr
*fattr,
const struct nfs_server *server, int may_sleep)
{
- return decode_getfattr_generic(xdr, fattr, NULL, server, may_sleep);
+ return decode_getfattr_generic(xdr, fattr, NULL, server, may_sleep,
0);
}
/*
@@ -6338,9 +6342,11 @@ static int nfs4_xdr_dec_fs_locations(struct
rpc_rqst *req,
if (status)
goto out;
xdr_enter_page(xdr, PAGE_SIZE);
- status = decode_getfattr(xdr, &res->fs_locations->fattr,
+ status = decode_getfattr_generic(xdr, &res->fs_locations->fattr,
+ NULL,
res->fs_locations->server,
- !RPC_IS_ASYNC(req->rq_task));
+ !RPC_IS_ASYNC(req->rq_task),
+ 1);
out:
return status;
}
@@ -6677,6 +6683,8 @@ int nfs4_decode_dirent(struct xdr_stream *xdr,
struct nfs_entry *entry,
if (decode_attr_bitmap(xdr, bitmap) < 0)
goto out_overflow;
+ bitmap[0] &= ~FATTR4_WORD0_FS_LOCATIONS;
+
if (decode_attr_length(xdr, &len, &p) < 0)
goto out_overflow;
--
1.7.1
During mount if rootfh changes fsid then fs-core layer
dereferences and calls NULL pointer.
nfs_fhget() sets rootinode->i_op to nfs_mountpoint_inode_operations.
Then d_alloc_and_lookup() calls i_op->lookup() that is NULL.
The problem is: rpc_ops->getroot() and rpc_ops->getattr()
return different fsid due to server replies.
So just refresh fsid, as RFC3530 doesn't specify behavior
in case of rootfh changes fsid.
Oops:
BUG: unable to handle kernel NULL pointer dereference at (null)
IP: [< (null)>] (null)
stack trace:
d_alloc_and_lookup+0x4c/0x74
do_lookup+0x1e3/0x280
link_path_walk+0x12e/0xab0
nfs4_remote_get_sb+0x56/0x2c0 [nfs]
path_walk+0x67/0xe0
vfs_path_lookup+0x8e/0x100
nfs_follow_remote_path+0x16f/0x3e0 [nfs]
nfs4_try_mount+0x6f/0xd0 [nfs]
nfs_get_sb+0x269/0x400 [nfs]
vfs_kern_mount+0x8a/0x1f0
do_kern_mount+0x52/0x130
do_mount+0x20a/0x260
sys_mount+0x90/0xe0
system_call_fastpath+0x16/0x1b
Signed-off-by: Vitaliy Gusev <[email protected]>
---
fs/nfs/getroot.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/fs/nfs/getroot.c b/fs/nfs/getroot.c
index b5ffe8f..7979652 100644
--- a/fs/nfs/getroot.c
+++ b/fs/nfs/getroot.c
@@ -199,6 +199,10 @@ struct dentry *nfs4_get_root(struct super_block
*sb, struct nfs_fh *mntfh)
goto out;
}
+ if (fattr->valid & NFS_ATTR_FATTR_FSID &&
+ !nfs_fsid_equal(&server->fsid, &fattr->fsid))
+ memcpy(&server->fsid, &fattr->fsid, sizeof(server->fsid));
+
inode = nfs_fhget(sb, mntfh, fattr);
if (IS_ERR(inode)) {
dprintk("nfs_get_root: get root inode failed\n");
--
1.7.1
Changes with v2 version: when not expected fs_locations occurs:
1. Throw print message "unexpected fs_locations"
2. Stop parsing xdr and return EIO error
----
There is a remote vulnerability for nfs4-client. That allows
a remote NFSv4 server or attacker in middle rewrite memory on
NFSv4 client.
Of course if encryption is used then only server can do corruption.
1. Client send GETATTR request to server, but there are not
checks for bitmask in reply from server. Therefore server may return
more attributes than were asked.
2. Pointer to struct nfs_fattr is passed to decode_getfattr(), but
that parameter is considered as pointer to struct nfs4_fs_locations
in case set FATTR4_WORD0_FS_LOCATIONS in reply from server.
But there is no check for pointer type that was passed!
3. Remote side can set FATTR4_WORD0_FS_LOCATIONS and fill
datas fsattr4_fs_locations in reply to client.
4. sizeof(nfs4_fs_locations) is about 92080 bytes in kernel 2.6.38
So ~ 90 Kbytes can be overwritten in memory of NFSv4 client.
Fortunately, nfs_fattr is allocated via kmalloc and
is not stored on stack. So generally kmalloc-192 is corrupted.
Affected kernels: all from v2.6.18
Commit 683b57b435326eb512c7305892683b6205669448
"NFSv4: Implement the fs_locations function call"
Signed-off-by: Vitaliy Gusev <[email protected]>
---
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index c87e543..6fea9c9 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -3451,7 +3451,8 @@ out_overflow:
return -EIO;
}
-static int decode_attr_fs_locations(struct xdr_stream *xdr, uint32_t
*bitmap, struct nfs4_fs_locations *res)
+static int decode_attr_fs_locations(struct xdr_stream *xdr, uint32_t
*bitmap, struct nfs4_fs_locations *res,
+ uint32_t *deny_bitmap)
{
int n;
__be32 *p;
@@ -3462,6 +3463,13 @@ static int decode_attr_fs_locations(struct
xdr_stream *xdr, uint32_t *bitmap, st
status = 0;
if (unlikely(!(bitmap[0] & FATTR4_WORD0_FS_LOCATIONS)))
goto out;
+
+ if (unlikely(deny_bitmap[0] & FATTR4_WORD0_FS_LOCATIONS)) {
+ status = -EIO;
+ printk(KERN_WARNING "%s: Unexpected fs_locations\n", __func__);
+ goto out;
+ }
+
dprintk("%s: fsroot ", __func__);
status = decode_pathname(xdr, &res->fs_path);
if (unlikely(status != 0))
@@ -4186,7 +4194,8 @@ xdr_error:
static int decode_getfattr_attrs(struct xdr_stream *xdr, uint32_t
*bitmap,
struct nfs_fattr *fattr, struct nfs_fh *fh,
- const struct nfs_server *server, int may_sleep)
+ const struct nfs_server *server, int may_sleep,
+ uint32_t *deny_bitmap)
{
int status;
umode_t fmode = 0;
@@ -4232,7 +4241,7 @@ static int decode_getfattr_attrs(struct xdr_stream
*xdr, uint32_t *bitmap,
status = decode_attr_fs_locations(xdr, bitmap, container_of(fattr,
struct nfs4_fs_locations,
- fattr));
+ fattr), deny_bitmap);
if (status < 0)
goto xdr_error;
fattr->valid |= status;
@@ -4301,11 +4310,13 @@ xdr_error:
}
static int decode_getfattr_generic(struct xdr_stream *xdr, struct
nfs_fattr *fattr,
- struct nfs_fh *fh, const struct nfs_server *server, int may_sleep)
+ struct nfs_fh *fh, const struct nfs_server *server, int may_sleep,
+ int expect_fsloc)
{
__be32 *savep;
uint32_t attrlen,
bitmap[3] = {0};
+ uint32_t deny_bitmap[3] = {0};
int status;
status = decode_op_hdr(xdr, OP_GETATTR);
@@ -4316,11 +4327,15 @@ static int decode_getfattr_generic(struct
xdr_stream *xdr, struct nfs_fattr *fat
if (status < 0)
goto xdr_error;
+ if (!expect_fsloc)
+ deny_bitmap[0] |= FATTR4_WORD0_FS_LOCATIONS;
+
status = decode_attr_length(xdr, &attrlen, &savep);
if (status < 0)
goto xdr_error;
- status = decode_getfattr_attrs(xdr, bitmap, fattr, fh, server,
may_sleep);
+ status = decode_getfattr_attrs(xdr, bitmap, fattr, fh, server,
may_sleep,
+ deny_bitmap);
if (status < 0)
goto xdr_error;
@@ -4333,7 +4348,7 @@ xdr_error:
static int decode_getfattr(struct xdr_stream *xdr, struct nfs_fattr
*fattr,
const struct nfs_server *server, int may_sleep)
{
- return decode_getfattr_generic(xdr, fattr, NULL, server, may_sleep);
+ return decode_getfattr_generic(xdr, fattr, NULL, server, may_sleep,
0);
}
/*
@@ -6338,9 +6353,11 @@ static int nfs4_xdr_dec_fs_locations(struct
rpc_rqst *req,
if (status)
goto out;
xdr_enter_page(xdr, PAGE_SIZE);
- status = decode_getfattr(xdr, &res->fs_locations->fattr,
+ status = decode_getfattr_generic(xdr, &res->fs_locations->fattr,
+ NULL,
res->fs_locations->server,
- !RPC_IS_ASYNC(req->rq_task));
+ !RPC_IS_ASYNC(req->rq_task),
+ 1);
out:
return status;
}
@@ -6639,7 +6656,7 @@ out:
int nfs4_decode_dirent(struct xdr_stream *xdr, struct nfs_entry *entry,
int plus)
{
- uint32_t bitmap[3] = {0};
+ uint32_t bitmap[3] = {0}, deny_bitmap[3] = {0};
uint32_t len;
__be32 *p = xdr_inline_decode(xdr, 4);
if (unlikely(!p))
@@ -6680,8 +6697,10 @@ int nfs4_decode_dirent(struct xdr_stream *xdr,
struct nfs_entry *entry,
if (decode_attr_length(xdr, &len, &p) < 0)
goto out_overflow;
+ deny_bitmap[0] |= FATTR4_WORD0_FS_LOCATIONS;
+
if (decode_getfattr_attrs(xdr, bitmap, entry->fattr, entry->fh,
- entry->server, 1) < 0)
+ entry->server, 1, deny_bitmap) < 0)
goto out_overflow;
if (entry->fattr->valid & NFS_ATTR_FATTR_FILEID)
entry->ino = entry->fattr->fileid;