Message ID | 1487349854-9732-4-git-send-email-bfields@redhat.com (mailing list archive) |
---|---|
State | New, archived |
Headers | show |
Hi Bruce, [auto build test ERROR on nfs/linux-next] [also build test ERROR on v4.10-rc8 next-20170217] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/J-Bruce-Fields/getacl-fixes/20170218-022320 base: git://git.linux-nfs.org/projects/trondmy/linux-nfs.git linux-next config: x86_64-randconfig-x015-201707 (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=x86_64 All error/warnings (new ones prefixed by >>): fs/nfs/nfs4proc.c: In function 'nfs4_do_get_acl': >> fs/nfs/nfs4proc.c:5095:2: error: 'ret' undeclared (first use in this function) ret = nfs4_call_sync(NFS_SERVER(inode)->client, NFS_SERVER(inode), ^~~ fs/nfs/nfs4proc.c:5095:2: note: each undeclared identifier is reported only once for each function it appears in >> fs/nfs/nfs4proc.c:5104:1: warning: control reaches end of non-void function [-Wreturn-type] } ^ vim +/ret +5095 fs/nfs/nfs4proc.c cfc94798 Weston Andros Adamson 2017-02-17 5089 return -ENOMEM; 5a006899 Sachin Prabhu 2012-04-17 5090 cfc94798 Weston Andros Adamson 2017-02-17 5091 args.acl_len = ARRAY_SIZE(pages) << PAGE_SHIFT; 5a006899 Sachin Prabhu 2012-04-17 5092 cfc94798 Weston Andros Adamson 2017-02-17 5093 dprintk("%s buf %p buflen %zu args.acl_len %zu\n", cfc94798 Weston Andros Adamson 2017-02-17 5094 __func__, buf, buflen, args.acl_len); bf118a34 Andy Adamson 2011-12-07 @5095 ret = nfs4_call_sync(NFS_SERVER(inode)->client, NFS_SERVER(inode), bf118a34 Andy Adamson 2011-12-07 5096 &msg, &args.seq_args, &res.seq_res, 0); 942bf94d J. Bruce Fields 2017-02-17 5097 if (ret == 0) 1f1ea6c2 Trond Myklebust 2012-08-26 5098 ret = res.acl_len; 942bf94d J. Bruce Fields 2017-02-17 5099 cfc94798 Weston Andros Adamson 2017-02-17 5100 for (i = 0; i < ARRAY_SIZE(pages) && pages[i]; i++) bf118a34 Andy Adamson 2011-12-07 5101 __free_page(pages[i]); 331818f1 Trond Myklebust 2012-02-03 5102 __free_page(res.acl_scratch); e50a1c2e J. Bruce Fields 2005-06-22 5103 return ret; e50a1c2e J. Bruce Fields 2005-06-22 @5104 } e50a1c2e J. Bruce Fields 2005-06-22 5105 942bf94d J. Bruce Fields 2017-02-17 5106 /* 942bf94d J. Bruce Fields 2017-02-17 5107 * The getxattr API returns the required buffer length when called with a :::::: The code at line 5095 was first introduced by commit :::::: bf118a342f10dafe44b14451a1392c3254629a1f NFSv4: include bitmap in nfsv4 get acl data :::::: TO: Andy Adamson <andros@netapp.com> :::::: CC: Trond Myklebust <Trond.Myklebust@netapp.com> --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Whoops, forgot to retst after a last minute change! Fixed locally. Also fixed up the subject lines that refer to "nfsd". --b. On Sat, Feb 18, 2017 at 03:15:11AM +0800, kbuild test robot wrote: > Hi Bruce, > > [auto build test ERROR on nfs/linux-next] > [also build test ERROR on v4.10-rc8 next-20170217] > [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] > > url: https://github.com/0day-ci/linux/commits/J-Bruce-Fields/getacl-fixes/20170218-022320 > base: git://git.linux-nfs.org/projects/trondmy/linux-nfs.git linux-next > config: x86_64-randconfig-x015-201707 (attached as .config) > compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 > reproduce: > # save the attached .config to linux build tree > make ARCH=x86_64 > > All error/warnings (new ones prefixed by >>): > > fs/nfs/nfs4proc.c: In function 'nfs4_do_get_acl': > >> fs/nfs/nfs4proc.c:5095:2: error: 'ret' undeclared (first use in this function) > ret = nfs4_call_sync(NFS_SERVER(inode)->client, NFS_SERVER(inode), > ^~~ > fs/nfs/nfs4proc.c:5095:2: note: each undeclared identifier is reported only once for each function it appears in > >> fs/nfs/nfs4proc.c:5104:1: warning: control reaches end of non-void function [-Wreturn-type] > } > ^ > > vim +/ret +5095 fs/nfs/nfs4proc.c > > cfc94798 Weston Andros Adamson 2017-02-17 5089 return -ENOMEM; > 5a006899 Sachin Prabhu 2012-04-17 5090 > cfc94798 Weston Andros Adamson 2017-02-17 5091 args.acl_len = ARRAY_SIZE(pages) << PAGE_SHIFT; > 5a006899 Sachin Prabhu 2012-04-17 5092 > cfc94798 Weston Andros Adamson 2017-02-17 5093 dprintk("%s buf %p buflen %zu args.acl_len %zu\n", > cfc94798 Weston Andros Adamson 2017-02-17 5094 __func__, buf, buflen, args.acl_len); > bf118a34 Andy Adamson 2011-12-07 @5095 ret = nfs4_call_sync(NFS_SERVER(inode)->client, NFS_SERVER(inode), > bf118a34 Andy Adamson 2011-12-07 5096 &msg, &args.seq_args, &res.seq_res, 0); > 942bf94d J. Bruce Fields 2017-02-17 5097 if (ret == 0) > 1f1ea6c2 Trond Myklebust 2012-08-26 5098 ret = res.acl_len; > 942bf94d J. Bruce Fields 2017-02-17 5099 > cfc94798 Weston Andros Adamson 2017-02-17 5100 for (i = 0; i < ARRAY_SIZE(pages) && pages[i]; i++) > bf118a34 Andy Adamson 2011-12-07 5101 __free_page(pages[i]); > 331818f1 Trond Myklebust 2012-02-03 5102 __free_page(res.acl_scratch); > e50a1c2e J. Bruce Fields 2005-06-22 5103 return ret; > e50a1c2e J. Bruce Fields 2005-06-22 @5104 } > e50a1c2e J. Bruce Fields 2005-06-22 5105 > 942bf94d J. Bruce Fields 2017-02-17 5106 /* > 942bf94d J. Bruce Fields 2017-02-17 5107 * The getxattr API returns the required buffer length when called with a > > :::::: The code at line 5095 was first introduced by commit > :::::: bf118a342f10dafe44b14451a1392c3254629a1f NFSv4: include bitmap in nfsv4 get acl data > > :::::: TO: Andy Adamson <andros@netapp.com> > :::::: CC: Trond Myklebust <Trond.Myklebust@netapp.com> > > --- > 0-DAY kernel test infrastructure Open Source Technology Center > https://lists.01.org/pipermail/kbuild-all Intel Corporation -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Hi Bruce,
[auto build test WARNING on nfs/linux-next]
[also build test WARNING on v4.10-rc8 next-20170217]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]
url: https://github.com/0day-ci/linux/commits/J-Bruce-Fields/getacl-fixes/20170218-022320
base: git://git.linux-nfs.org/projects/trondmy/linux-nfs.git linux-next
coccinelle warnings: (new ones prefixed by >>)
>> fs/nfs/nfs4proc.c:5135:2-7: WARNING: NULL check before freeing functions like kfree, debugfs_remove, debugfs_remove_recursive or usb_free_urb is not needed. Maybe consider reorganizing relevant code to avoid passing NULL values.
Please review and possibly fold the followup patch.
---
0-DAY kernel test infrastructure Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c index 2d485dab343e..6262b2955d0e 100644 --- a/fs/nfs/nfs4proc.c +++ b/fs/nfs/nfs4proc.c @@ -5035,17 +5035,23 @@ static inline ssize_t nfs4_read_cached_acl(struct inode *inode, char *buf, size_ return ret; } -static void nfs4_write_cached_acl(struct inode *inode, struct page **pages, size_t pgbase, size_t acl_len) +/* + * We don't cache ACLs over a certain size. I don't know if we have any + * real reason for this policy: + */ +#define NFS4_MAX_CACHED_ACL PAGE_SIZE + +static void nfs4_write_cached_acl(struct inode *inode, void *buf, size_t acl_len) { struct nfs4_cached_acl *acl; size_t buflen = sizeof(*acl) + acl_len; - if (buflen <= PAGE_SIZE) { + if (buflen <= NFS4_MAX_CACHED_ACL) { acl = kmalloc(buflen, GFP_KERNEL); if (acl == NULL) goto out; acl->cached = 1; - _copy_from_pages(acl->data, pages, pgbase, acl_len); + memcpy(acl->data, buf, acl_len); } else { acl = kmalloc(sizeof(*acl), GFP_KERNEL); if (acl == NULL) @@ -5057,13 +5063,7 @@ static void nfs4_write_cached_acl(struct inode *inode, struct page **pages, size nfs4_set_cached_acl(inode, acl); } -/* - * The getxattr API returns the required buffer length when called with a - * NULL buf. The NFSv4 acl tool then calls getxattr again after allocating - * the required buf. Cache the result from the first getxattr call to avoid - * sending another RPC. - */ -static ssize_t __nfs4_get_acl_uncached(struct inode *inode, void *buf, size_t buflen) +static ssize_t nfs4_do_get_acl(struct inode *inode, void *buf, size_t buflen) { /* enough pages to hold ACL data plus the bitmap and acl length */ struct page *pages[NFS4ACL_MAXPAGES + 1] = {NULL, }; @@ -5074,13 +5074,14 @@ static ssize_t __nfs4_get_acl_uncached(struct inode *inode, void *buf, size_t bu }; struct nfs_getaclres res = { .acl_len = buflen, + .buf = buf, }; struct rpc_message msg = { .rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_GETACL], .rpc_argp = &args, .rpc_resp = &res, }; - int ret = -ENOMEM, i; + int i; /* for decoding across pages */ res.acl_scratch = alloc_page(GFP_KERNEL); @@ -5093,34 +5094,48 @@ static ssize_t __nfs4_get_acl_uncached(struct inode *inode, void *buf, size_t bu __func__, buf, buflen, args.acl_len); ret = nfs4_call_sync(NFS_SERVER(inode)->client, NFS_SERVER(inode), &msg, &args.seq_args, &res.seq_res, 0); - if (ret) - goto out_free; + if (ret == 0) + ret = res.acl_len; - /* Handle the case where the passed-in buffer is too short */ - if (res.acl_flags & NFS4_ACL_TRUNC) { - /* Did the user only issue a request for the acl length? */ - if (buf == NULL) - goto out_ok; - ret = -ERANGE; - goto out_free; - } - nfs4_write_cached_acl(inode, pages, res.acl_data_offset, res.acl_len); - if (buf) { - if (res.acl_len > buflen) { - ret = -ERANGE; - goto out_free; - } - _copy_from_pages(buf, pages, res.acl_data_offset, res.acl_len); - } -out_ok: - ret = res.acl_len; -out_free: for (i = 0; i < ARRAY_SIZE(pages) && pages[i]; i++) __free_page(pages[i]); __free_page(res.acl_scratch); return ret; } +/* + * The getxattr API returns the required buffer length when called with a + * NULL buf. The NFSv4 acl tool then calls getxattr again after allocating + * the required buf. Cache the result from the first getxattr call to avoid + * sending another RPC. + */ +static ssize_t __nfs4_get_acl_uncached(struct inode *inode, void *buf, size_t buflen) +{ + void *priv_buf = NULL; + void *our_buf = buf; + int our_buflen = buflen; + static ssize_t ret = -ENOMEM; + + if (!buf) { + priv_buf = kmalloc(NFS4_MAX_CACHED_ACL, GFP_KERNEL); + if (!priv_buf) + goto out; + our_buf = priv_buf; + our_buflen = NFS4_MAX_CACHED_ACL; + } + ret = nfs4_do_get_acl(inode, our_buf, our_buflen); + if (ret < 0) + goto out; + if (ret <= our_buflen) + nfs4_write_cached_acl(inode, our_buf, ret); + if (buf && ret > buflen) + ret = -ERANGE; +out: + if (priv_buf) + kfree(priv_buf); + return ret; +} + static ssize_t nfs4_get_acl_uncached(struct inode *inode, void *buf, size_t buflen) { struct nfs4_exception exception = { }; diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c index bb95dd2edeef..534b377084fb 100644 --- a/fs/nfs/nfs4xdr.c +++ b/fs/nfs/nfs4xdr.c @@ -5353,39 +5353,24 @@ static int decode_getacl(struct xdr_stream *xdr, struct rpc_rqst *req, uint32_t attrlen, bitmap[3] = {0}; int status; - unsigned int pg_offset; - res->acl_len = 0; if ((status = decode_op_hdr(xdr, OP_GETATTR)) != 0) goto out; - - xdr_enter_page(xdr, xdr->buf->page_len); - - /* Calculate the offset of the page data */ - pg_offset = xdr->buf->head[0].iov_len; - if ((status = decode_attr_bitmap(xdr, bitmap)) != 0) goto out; if ((status = decode_attr_length(xdr, &attrlen, &savep)) != 0) goto out; - + if (unlikely(attrlen > (xdr->nwords << 2))) + return -EIO; if (unlikely(bitmap[0] & (FATTR4_WORD0_ACL - 1U))) return -EIO; if (likely(bitmap[0] & FATTR4_WORD0_ACL)) { + unsigned int offset = xdr_stream_pos(xdr); - /* 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.*/ - res->acl_data_offset = xdr_stream_pos(xdr) - pg_offset; + if (attrlen <= res->acl_len) + read_bytes_from_xdr_buf(xdr->buf, offset, + res->buf, attrlen); res->acl_len = attrlen; - - /* Check for receive buffer overflow */ - if (res->acl_len > (xdr->nwords << 2) || - res->acl_len + res->acl_data_offset > xdr->buf->page_len) { - res->acl_flags |= NFS4_ACL_TRUNC; - dprintk("NFS: acl reply: attrlen %u > page_len %u\n", - attrlen, xdr->nwords << 2); - } } else status = -EOPNOTSUPP; diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h index 348f7c158084..418116d62740 100644 --- a/include/linux/nfs_xdr.h +++ b/include/linux/nfs_xdr.h @@ -747,13 +747,11 @@ struct nfs_getaclargs { struct page ** acl_pages; }; -/* getxattr ACL interface flags */ -#define NFS4_ACL_TRUNC 0x0001 /* ACL was truncated */ struct nfs_getaclres { struct nfs4_sequence_res seq_res; + void * buf; size_t acl_len; size_t acl_data_offset; - int acl_flags; struct page * acl_scratch; };