On Wed, 2012-08-01 at 14:29 +0100, Sachin Prabhu wrote:
> Rename flag ACL_LEN_REQEST to NFS4_ACL_LEN_ONLY. Apart from using this
> flag during a request to indicate that the user is only interested in
> the length of the ACL, we now also use it in the response stage to
> indicate that the data returned holds the complete ACL or just the
> length.
>
> Signed-off-by: Sachin Prabhu <sprabhu@xxxxxxxxxx>
> ---
> fs/nfs/nfs4proc.c | 4 ++--
> fs/nfs/nfs4xdr.c | 4 +++-
> include/linux/nfs_xdr.h | 2 +-
> 3 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index be03b95..aaa98f0 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -3760,7 +3760,7 @@ static ssize_t __nfs4_get_acl_uncached(struct inode *inode, void *buf, size_t bu
> /* Let decode_getfacl know not to fail if the ACL data is larger than
> * the page we send as a guess */
> if (buf == NULL)
> - res.acl_flags |= NFS4_ACL_LEN_REQUEST;
> + res.acl_flags |= NFS4_ACL_LEN_ONLY;
>
> dprintk("%s buf %p buflen %zu npages %d args.acl_len %zu\n",
> __func__, buf, buflen, npages, args.acl_len);
> @@ -3770,7 +3770,7 @@ static ssize_t __nfs4_get_acl_uncached(struct inode *inode, void *buf, size_t bu
> goto out_free;
>
> acl_len = res.acl_len - res.acl_data_offset;
> - if (res.acl_len > args.acl_len)
> + if (res.acl_flags & NFS4_ACL_LEN_ONLY)
Is this a bugfix or a cleanup? If the former, then it belongs in the
first patch.
> nfs4_write_cached_acl(inode, NULL, 0, acl_len);
> else
> nfs4_write_cached_acl(inode, pages, res.acl_data_offset,
Now if NFS4_ACL_LEN_ONLY is set, we appear to unconditionally write the
acl, even if the buffer overflows...
> diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
> index 18fae29..a88fcd0 100644
> --- a/fs/nfs/nfs4xdr.c
> +++ b/fs/nfs/nfs4xdr.c
> @@ -5101,7 +5101,7 @@ static int decode_getacl(struct xdr_stream *xdr, struct rpc_rqst *req,
> hdrlen = (u8 *)xdr->p - (u8 *)iov->iov_base;
> attrlen += res->acl_data_offset;
> if (attrlen > page_len) {
> - if (res->acl_flags & NFS4_ACL_LEN_REQUEST) {
> + if (res->acl_flags & NFS4_ACL_LEN_ONLY) {
The logic here still looks incorrect. If the user is only requesting the
acl length, then we shouldn't care about the 'attrlen > page_len' test.
There also appears to remain a lot of illegal deferencing of xdr->p even
after this patch. We should be able to fix that by replacing
xdr_read_pages with a call to xdr_enter_page() before we call
decode_attr_bitmap()...
> /* getxattr interface called with a NULL buf */
> res->acl_len = attrlen;
> goto out;
> @@ -5110,6 +5110,8 @@ static int decode_getacl(struct xdr_stream *xdr, struct rpc_rqst *req,
> attrlen, page_len);
> return -EINVAL;
> }
> + /* At this stage we have the complete ACL */
> + res->acl_flags &= ~NFS4_ACL_LEN_ONLY;
> xdr_read_pages(xdr, attrlen);
We should just replace that check of 'attrlen > page_len' with a check
on the return value of xdr_read_pages().
> res->acl_len = attrlen;
> } else
> diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h
> index d3b7c18..c2d77d5 100644
> --- a/include/linux/nfs_xdr.h
> +++ b/include/linux/nfs_xdr.h
> @@ -648,7 +648,7 @@ struct nfs_getaclargs {
> };
>
> /* getxattr ACL interface flags */
> -#define NFS4_ACL_LEN_REQUEST 0x0001 /* zero length getxattr buffer */
> +#define NFS4_ACL_LEN_ONLY 0x0001 /* zero length getxattr buffer */
> struct nfs_getaclres {
> size_t acl_len;
> size_t acl_data_offset;
��.n��������+%������w��{.n�����{��w��� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
