From 95b72eb0bdef6476b7e73061f0382adf46c5495a Mon Sep 17 00:00:00 2001 From: Trond Myklebust Date: Fri, 20 Apr 2012 19:24:51 -0400 Subject: NFSv4: Ensure we do not reuse open owner names The NFSv4 spec is ambiguous about whether or not it is permissible to reuse open owner names, so play it safe. This patch adds a timestamp to the state_owner structure, and combines that with the IDA based uniquifier. Fixes a regression whereby the Linux server returns NFS4ERR_BAD_SEQID. Signed-off-by: Trond Myklebust --- fs/nfs/nfs4xdr.c | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) (limited to 'fs/nfs/nfs4xdr.c') diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c index c74fdb114b4..77fc5f959c4 100644 --- a/fs/nfs/nfs4xdr.c +++ b/fs/nfs/nfs4xdr.c @@ -74,7 +74,7 @@ static int nfs4_stat_to_errno(int); /* lock,open owner id: * we currently use size 2 (u64) out of (NFS4_OPAQUE_LIMIT >> 2) */ -#define open_owner_id_maxsz (1 + 1 + 4) +#define open_owner_id_maxsz (1 + 2 + 1 + 1 + 2) #define lock_owner_id_maxsz (1 + 1 + 4) #define decode_lockowner_maxsz (1 + XDR_QUADLEN(IDMAP_NAMESZ)) #define compound_encode_hdr_maxsz (3 + (NFS4_MAXTAGLEN >> 2)) @@ -1340,12 +1340,13 @@ static inline void encode_openhdr(struct xdr_stream *xdr, const struct nfs_opena */ encode_nfs4_seqid(xdr, arg->seqid); encode_share_access(xdr, arg->fmode); - p = reserve_space(xdr, 32); + p = reserve_space(xdr, 36); p = xdr_encode_hyper(p, arg->clientid); - *p++ = cpu_to_be32(20); + *p++ = cpu_to_be32(24); p = xdr_encode_opaque_fixed(p, "open id:", 8); *p++ = cpu_to_be32(arg->server->s_dev); - xdr_encode_hyper(p, arg->id); + *p++ = cpu_to_be32(arg->id.uniquifier); + xdr_encode_hyper(p, arg->id.create_time); } static inline void encode_createmode(struct xdr_stream *xdr, const struct nfs_openargs *arg) -- cgit v1.2.3-70-g09d2 From 5a00689930ab975fdd1b37b034475017e460cf2a Mon Sep 17 00:00:00 2001 From: Sachin Prabhu Date: Tue, 17 Apr 2012 14:35:39 +0100 Subject: Avoid reading past buffer when calling GETACL Bug noticed in commit bf118a342f10dafe44b14451a1392c3254629a1f When calling GETACL, if the size of the bitmap array, the length attribute and the acl returned by the server is greater than the allocated buffer(args.acl_len), we can Oops with a General Protection fault at _copy_from_pages() when we attempt to read past the pages allocated. This patch allocates an extra PAGE for the bitmap and checks to see that the bitmap + attribute_length + ACLs don't exceed the buffer space allocated to it. Signed-off-by: Sachin Prabhu Reported-by: Jian Li [Trond: Fixed a size_t vs unsigned int printk() warning] Signed-off-by: Trond Myklebust --- fs/nfs/nfs4proc.c | 16 ++++++++++------ fs/nfs/nfs4xdr.c | 18 +++++++++++------- 2 files changed, 21 insertions(+), 13 deletions(-) (limited to 'fs/nfs/nfs4xdr.c') diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 60d5f4c26dd..f5f125fdae1 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -3684,19 +3684,23 @@ static ssize_t __nfs4_get_acl_uncached(struct inode *inode, void *buf, size_t bu if (npages == 0) npages = 1; + /* Add an extra page to handle the bitmap returned */ + npages++; + for (i = 0; i < npages; i++) { pages[i] = alloc_page(GFP_KERNEL); if (!pages[i]) goto out_free; } - if (npages > 1) { - /* for decoding across pages */ - res.acl_scratch = alloc_page(GFP_KERNEL); - if (!res.acl_scratch) - goto out_free; - } + + /* for decoding across pages */ + res.acl_scratch = alloc_page(GFP_KERNEL); + if (!res.acl_scratch) + goto out_free; + args.acl_len = npages * PAGE_SIZE; args.acl_pgbase = 0; + /* Let decode_getfacl know not to fail if the ACL data is larger than * the page we send as a guess */ if (buf == NULL) diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c index 77fc5f959c4..9312dd78d34 100644 --- a/fs/nfs/nfs4xdr.c +++ b/fs/nfs/nfs4xdr.c @@ -4902,11 +4902,19 @@ static int decode_getacl(struct xdr_stream *xdr, struct rpc_rqst *req, bitmap[3] = {0}; struct kvec *iov = req->rq_rcv_buf.head; int status; + size_t page_len = xdr->buf->page_len; res->acl_len = 0; if ((status = decode_op_hdr(xdr, OP_GETATTR)) != 0) goto out; + bm_p = xdr->p; + res->acl_data_offset = be32_to_cpup(bm_p) + 2; + res->acl_data_offset <<= 2; + /* Check if the acl data starts beyond the allocated buffer */ + if (res->acl_data_offset > page_len) + return -ERANGE; + if ((status = decode_attr_bitmap(xdr, bitmap)) != 0) goto out; if ((status = decode_attr_length(xdr, &attrlen, &savep)) != 0) @@ -4916,28 +4924,24 @@ static int decode_getacl(struct xdr_stream *xdr, struct rpc_rqst *req, return -EIO; if (likely(bitmap[0] & FATTR4_WORD0_ACL)) { size_t hdrlen; - u32 recvd; /* The bitmap (xdr len + bitmaps) and the attr xdr len words * are stored with the acl data to handle the problem of * variable length bitmaps.*/ xdr->p = bm_p; - res->acl_data_offset = be32_to_cpup(bm_p) + 2; - res->acl_data_offset <<= 2; /* We ignore &savep and don't do consistency checks on * the attr length. Let userspace figure it out.... */ hdrlen = (u8 *)xdr->p - (u8 *)iov->iov_base; attrlen += res->acl_data_offset; - recvd = req->rq_rcv_buf.len - hdrlen; - if (attrlen > recvd) { + if (attrlen > page_len) { if (res->acl_flags & NFS4_ACL_LEN_REQUEST) { /* getxattr interface called with a NULL buf */ res->acl_len = attrlen; goto out; } - dprintk("NFS: acl reply: attrlen %u > recvd %u\n", - attrlen, recvd); + dprintk("NFS: acl reply: attrlen %zu > page_len %u\n", + attrlen, page_len); return -EINVAL; } xdr_read_pages(xdr, attrlen); -- cgit v1.2.3-70-g09d2 From 5794d21ef4639f0e33440927bb903f9598c21e92 Mon Sep 17 00:00:00 2001 From: Sachin Prabhu Date: Tue, 17 Apr 2012 14:36:40 +0100 Subject: Avoid beyond bounds copy while caching ACL When attempting to cache ACLs returned from the server, if the bitmap size + the ACL size is greater than a PAGE_SIZE but the ACL size itself is smaller than a PAGE_SIZE, we can read past the buffer page boundary. Signed-off-by: Sachin Prabhu Reported-by: Jian Li Signed-off-by: Trond Myklebust --- fs/nfs/nfs4proc.c | 12 +++++------- fs/nfs/nfs4xdr.c | 2 +- 2 files changed, 6 insertions(+), 8 deletions(-) (limited to 'fs/nfs/nfs4xdr.c') diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index f5f125fdae1..2ce069880d6 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -3628,16 +3628,16 @@ out: return ret; } -static void nfs4_write_cached_acl(struct inode *inode, const char *buf, size_t acl_len) +static void nfs4_write_cached_acl(struct inode *inode, struct page **pages, size_t pgbase, size_t acl_len) { struct nfs4_cached_acl *acl; - if (buf && acl_len <= PAGE_SIZE) { + if (pages && acl_len <= PAGE_SIZE) { acl = kmalloc(sizeof(*acl) + acl_len, GFP_KERNEL); if (acl == NULL) goto out; acl->cached = 1; - memcpy(acl->data, buf, acl_len); + _copy_from_pages(acl->data, pages, pgbase, acl_len); } else { acl = kmalloc(sizeof(*acl), GFP_KERNEL); if (acl == NULL) @@ -3670,7 +3670,6 @@ static ssize_t __nfs4_get_acl_uncached(struct inode *inode, void *buf, size_t bu struct nfs_getaclres res = { .acl_len = buflen, }; - void *resp_buf; struct rpc_message msg = { .rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_GETACL], .rpc_argp = &args, @@ -3705,7 +3704,6 @@ static ssize_t __nfs4_get_acl_uncached(struct inode *inode, void *buf, size_t bu * the page we send as a guess */ if (buf == NULL) res.acl_flags |= NFS4_ACL_LEN_REQUEST; - resp_buf = page_address(pages[0]); dprintk("%s buf %p buflen %zu npages %d args.acl_len %zu\n", __func__, buf, buflen, npages, args.acl_len); @@ -3716,9 +3714,9 @@ static ssize_t __nfs4_get_acl_uncached(struct inode *inode, void *buf, size_t bu acl_len = res.acl_len - res.acl_data_offset; if (acl_len > args.acl_len) - nfs4_write_cached_acl(inode, NULL, acl_len); + nfs4_write_cached_acl(inode, NULL, 0, acl_len); else - nfs4_write_cached_acl(inode, resp_buf + res.acl_data_offset, + nfs4_write_cached_acl(inode, pages, res.acl_data_offset, acl_len); if (buf) { ret = -ERANGE; diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c index 9312dd78d34..203c0967451 100644 --- a/fs/nfs/nfs4xdr.c +++ b/fs/nfs/nfs4xdr.c @@ -4940,7 +4940,7 @@ static int decode_getacl(struct xdr_stream *xdr, struct rpc_rqst *req, res->acl_len = attrlen; goto out; } - dprintk("NFS: acl reply: attrlen %zu > page_len %u\n", + dprintk("NFS: acl reply: attrlen %u > page_len %zu\n", attrlen, page_len); return -EINVAL; } -- cgit v1.2.3-70-g09d2 From 31e4dda4747713de13889f7c79c7aec341fea61b Mon Sep 17 00:00:00 2001 From: Bryan Schumaker Date: Fri, 27 Apr 2012 13:27:38 -0400 Subject: NFS: Fix SECINFO_NO_NAME I was using the same decoder function for SECINFO and SECINFO_NO_NAME, so it was returning an error when it tried to decode an OP_SECINFO_NO_NAME header as OP_SECINFO. Signed-off-by: Bryan Schumaker Signed-off-by: Trond Myklebust --- fs/nfs/nfs4xdr.c | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) (limited to 'fs/nfs/nfs4xdr.c') diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c index 203c0967451..c77e802db73 100644 --- a/fs/nfs/nfs4xdr.c +++ b/fs/nfs/nfs4xdr.c @@ -5094,16 +5094,13 @@ out_err: return -EINVAL; } -static int decode_secinfo(struct xdr_stream *xdr, struct nfs4_secinfo_res *res) +static int decode_secinfo_common(struct xdr_stream *xdr, struct nfs4_secinfo_res *res) { struct nfs4_secinfo_flavor *sec_flavor; int status; __be32 *p; int i, num_flavors; - status = decode_op_hdr(xdr, OP_SECINFO); - if (status) - goto out; p = xdr_inline_decode(xdr, 4); if (unlikely(!p)) goto out_overflow; @@ -5129,6 +5126,7 @@ static int decode_secinfo(struct xdr_stream *xdr, struct nfs4_secinfo_res *res) res->flavors->num_flavors++; } + status = 0; out: return status; out_overflow: @@ -5136,7 +5134,23 @@ out_overflow: return -EIO; } +static int decode_secinfo(struct xdr_stream *xdr, struct nfs4_secinfo_res *res) +{ + int status = decode_op_hdr(xdr, OP_SECINFO); + if (status) + return status; + return decode_secinfo_common(xdr, res); +} + #if defined(CONFIG_NFS_V4_1) +static int decode_secinfo_no_name(struct xdr_stream *xdr, struct nfs4_secinfo_res *res) +{ + int status = decode_op_hdr(xdr, OP_SECINFO_NO_NAME); + if (status) + return status; + return decode_secinfo_common(xdr, res); +} + static int decode_exchange_id(struct xdr_stream *xdr, struct nfs41_exchange_id_res *res) { @@ -6821,7 +6835,7 @@ static int nfs4_xdr_dec_secinfo_no_name(struct rpc_rqst *rqstp, status = decode_putrootfh(xdr); if (status) goto out; - status = decode_secinfo(xdr, res); + status = decode_secinfo_no_name(xdr, res); out: return status; } -- cgit v1.2.3-70-g09d2 From 72de53ec4bca39c26709122a8f78bfefe7b6bca4 Mon Sep 17 00:00:00 2001 From: Bryan Schumaker Date: Fri, 27 Apr 2012 13:27:40 -0400 Subject: NFS: Do secinfo as part of lookup Whenever lookup sees wrongsec do a secinfo and retry the lookup to find attributes of the file or directory, such as "is this a referral mountpoint?". This also allows me to remove handling -NFS4ERR_WRONSEC as part of getattr xdr decoding. Signed-off-by: Bryan Schumaker Signed-off-by: Trond Myklebust --- fs/nfs/internal.h | 1 - fs/nfs/nfs4_fs.h | 4 ++++ fs/nfs/nfs4namespace.c | 52 ++++++++++++++++++++++++++++++++++++++++ fs/nfs/nfs4proc.c | 64 ++++++++++++++++++++++++++++++++++++-------------- fs/nfs/nfs4xdr.c | 2 -- 5 files changed, 103 insertions(+), 20 deletions(-) (limited to 'fs/nfs/nfs4xdr.c') diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h index 2476dc69365..45966d95316 100644 --- a/fs/nfs/internal.h +++ b/fs/nfs/internal.h @@ -234,7 +234,6 @@ extern const u32 nfs41_maxwrite_overhead; /* nfs4proc.c */ #ifdef CONFIG_NFS_V4 extern struct rpc_procinfo nfs4_procedures[]; -void nfs_fixup_secinfo_attributes(struct nfs_fattr *, struct nfs_fh *); #endif extern int nfs4_init_ds_session(struct nfs_client *clp); diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h index b6db9e33fb7..c82c2cda3df 100644 --- a/fs/nfs/nfs4_fs.h +++ b/fs/nfs/nfs4_fs.h @@ -205,6 +205,9 @@ struct nfs4_state_maintenance_ops { extern const struct dentry_operations nfs4_dentry_operations; extern const struct inode_operations nfs4_dir_inode_operations; +/* nfs4namespace.c */ +struct rpc_clnt *nfs4_create_sec_client(struct rpc_clnt *, struct inode *, struct qstr *); + /* nfs4proc.c */ extern int nfs4_proc_setclientid(struct nfs_client *, u32, unsigned short, struct rpc_cred *, struct nfs4_setclientid_res *); extern int nfs4_proc_setclientid_confirm(struct nfs_client *, struct nfs4_setclientid_res *arg, struct rpc_cred *); @@ -215,6 +218,7 @@ extern int nfs4_do_close(struct nfs4_state *state, gfp_t gfp_mask, int wait, boo extern int nfs4_server_capabilities(struct nfs_server *server, struct nfs_fh *fhandle); extern int nfs4_proc_fs_locations(struct inode *dir, const struct qstr *name, struct nfs4_fs_locations *fs_locations, struct page *page); +extern int nfs4_proc_secinfo(struct inode *, const struct qstr *, struct nfs4_secinfo_flavors *); extern int nfs4_release_lockowner(struct nfs4_lock_state *); extern const struct xattr_handler *nfs4_xattr_handlers[]; diff --git a/fs/nfs/nfs4namespace.c b/fs/nfs/nfs4namespace.c index 7483a177dc9..9f8681bf90d 100644 --- a/fs/nfs/nfs4namespace.c +++ b/fs/nfs/nfs4namespace.c @@ -132,6 +132,58 @@ static size_t nfs_parse_server_name(char *string, size_t len, return ret; } +static rpc_authflavor_t nfs4_negotiate_security(struct inode *inode, struct qstr *name) +{ + struct page *page; + struct nfs4_secinfo_flavors *flavors; + rpc_authflavor_t flavor; + int err; + + page = alloc_page(GFP_KERNEL); + if (!page) + return -ENOMEM; + flavors = page_address(page); + + err = nfs4_proc_secinfo(inode, name, flavors); + if (err < 0) { + flavor = err; + goto out; + } + + flavor = nfs_find_best_sec(flavors); + +out: + put_page(page); + return flavor; +} + +/* + * Please call rpc_shutdown_client() when you are done with this client. + */ +struct rpc_clnt *nfs4_create_sec_client(struct rpc_clnt *clnt, struct inode *inode, + struct qstr *name) +{ + struct rpc_clnt *clone; + struct rpc_auth *auth; + rpc_authflavor_t flavor; + + flavor = nfs4_negotiate_security(inode, name); + if (flavor < 0) + return ERR_PTR(flavor); + + clone = rpc_clone_client(clnt); + if (IS_ERR(clone)) + return clone; + + auth = rpcauth_create(flavor, clone); + if (!auth) { + rpc_shutdown_client(clone); + clone = ERR_PTR(-EIO); + } + + return clone; +} + static struct vfsmount *try_location(struct nfs_clone_mount *mountdata, char *page, char *page2, const struct nfs4_fs_location *location) diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index ff7571f12bb..3d92fe6be78 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -2528,39 +2528,69 @@ static int _nfs4_proc_lookup(struct rpc_clnt *clnt, struct inode *dir, return status; } -void nfs_fixup_secinfo_attributes(struct nfs_fattr *fattr, struct nfs_fh *fh) +static void nfs_fixup_secinfo_attributes(struct nfs_fattr *fattr) { - memset(fh, 0, sizeof(struct nfs_fh)); - fattr->fsid.major = 1; fattr->valid |= NFS_ATTR_FATTR_TYPE | NFS_ATTR_FATTR_MODE | - NFS_ATTR_FATTR_NLINK | NFS_ATTR_FATTR_FSID | NFS_ATTR_FATTR_MOUNTPOINT; + NFS_ATTR_FATTR_NLINK | NFS_ATTR_FATTR_MOUNTPOINT; fattr->mode = S_IFDIR | S_IRUGO | S_IXUGO; fattr->nlink = 2; } -static int nfs4_proc_lookup(struct rpc_clnt *clnt, struct inode *dir, struct qstr *name, - struct nfs_fh *fhandle, struct nfs_fattr *fattr) +static int nfs4_proc_lookup_common(struct rpc_clnt **clnt, struct inode *dir, + struct qstr *name, struct nfs_fh *fhandle, + struct nfs_fattr *fattr) { struct nfs4_exception exception = { }; + struct rpc_clnt *client = *clnt; int err; do { - int status; - - status = _nfs4_proc_lookup(clnt, dir, name, fhandle, fattr); - switch (status) { + err = _nfs4_proc_lookup(client, dir, name, fhandle, fattr); + switch (err) { case -NFS4ERR_BADNAME: - return -ENOENT; + err = -ENOENT; + goto out; case -NFS4ERR_MOVED: - return nfs4_get_referral(dir, name, fattr, fhandle); + err = nfs4_get_referral(dir, name, fattr, fhandle); + goto out; case -NFS4ERR_WRONGSEC: - nfs_fixup_secinfo_attributes(fattr, fhandle); + err = -EPERM; + if (client != *clnt) + goto out; + + client = nfs4_create_sec_client(client, dir, name); + if (IS_ERR(client)) + return PTR_ERR(client); + + exception.retry = 1; + break; + default: + err = nfs4_handle_exception(NFS_SERVER(dir), err, &exception); } - err = nfs4_handle_exception(NFS_SERVER(dir), - status, &exception); } while (exception.retry); + +out: + if (err == 0) + *clnt = client; + else if (client != *clnt) + rpc_shutdown_client(client); + return err; } +static int nfs4_proc_lookup(struct rpc_clnt *clnt, struct inode *dir, struct qstr *name, + struct nfs_fh *fhandle, struct nfs_fattr *fattr) +{ + int status; + struct rpc_clnt *client = NFS_CLIENT(dir); + + status = nfs4_proc_lookup_common(&client, dir, name, fhandle, fattr); + if (client != NFS_CLIENT(dir)) { + rpc_shutdown_client(client); + nfs_fixup_secinfo_attributes(fattr); + } + return status; +} + static int _nfs4_proc_access(struct inode *inode, struct nfs_access_entry *entry) { struct nfs_server *server = NFS_SERVER(inode); @@ -4996,8 +5026,8 @@ static int _nfs4_proc_secinfo(struct inode *dir, const struct qstr *name, struct return status; } -static int nfs4_proc_secinfo(struct inode *dir, const struct qstr *name, - struct nfs4_secinfo_flavors *flavors) +int nfs4_proc_secinfo(struct inode *dir, const struct qstr *name, + struct nfs4_secinfo_flavors *flavors) { struct nfs4_exception exception = { }; int err; diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c index c77e802db73..c54aae364be 100644 --- a/fs/nfs/nfs4xdr.c +++ b/fs/nfs/nfs4xdr.c @@ -4258,8 +4258,6 @@ static int decode_getfattr_attrs(struct xdr_stream *xdr, uint32_t *bitmap, status = decode_attr_error(xdr, bitmap, &err); if (status < 0) goto xdr_error; - if (err == -NFS4ERR_WRONGSEC) - nfs_fixup_secinfo_attributes(fattr, fh); status = decode_attr_filehandle(xdr, bitmap, fh); if (status < 0) -- cgit v1.2.3-70-g09d2