In fs/nfsd/nfs2acl.c::nfsaclsvc_encode_getaclres() I see a few issues.
1) At the top of the function we assign to the 'inode' variable by
dereferencing 'dentry', but further down we test 'dentry' for NULL. So, if
'dentry' (which is really 'resp->fh.fh_dentry') can be NULL, then either
we have a potential NULL pointer deref bug or we have a superflous test.
2) In the case of resp->fh.fh_dentry != NULL we have a duplicate
assignment to 'inode' - just one would do.
3) There are two locations in the function where we may return before we
use the value of the variable 'w', but we compute it at the very top of the
function. So in the case where we return early we have wasted a few cycles
computing a value that was never used.
This patch solves these issues by :
1) Moving the computation of 'w' to just before it is used, after the two
potential returns.
2) Remove the initial assignment of 'dentry->d_inode'
(aka resp->fh.fh_dentry->d_inode) to 'inode' so that the assignment only
happens once and happens after the NULL test.
So we get 3 benefits from this patch :
1) We avoid a potential NULL pointer deref.
2) We save a few CPU cycles from not computing 'w' in the case of an early
return as well as saving the assignment to 'inode' in the dentry == NULL
case, and there's only a single assignment to 'inode' ever.
3) We save a few bytes of .text in the object file.
before:
text data bss dec hex filename
2502 204 0 2706 a92 fs/nfsd/nfs2acl.o
after:
text data bss dec hex filename
2489 204 0 2693 a85 fs/nfsd/nfs2acl.o
Patch is compile tested only since I don't currently have a setup where I
can meaningfully test it further.
Please comment and/or apply.
Signed-off-by: Jesper Juhl <[email protected]>
---
fs/nfsd/nfs2acl.c | 9 +++++----
1 files changed, 5 insertions(+), 4 deletions(-)
diff --git a/fs/nfsd/nfs2acl.c b/fs/nfsd/nfs2acl.c
index e3eca08..d89d63f 100644
--- a/fs/nfsd/nfs2acl.c
+++ b/fs/nfsd/nfs2acl.c
@@ -221,12 +221,10 @@ static int nfsaclsvc_encode_getaclres(st
struct nfsd3_getaclres *resp)
{
struct dentry *dentry = resp->fh.fh_dentry;
- struct inode *inode = dentry->d_inode;
- int w = nfsacl_size(
- (resp->mask & NFS_ACL) ? resp->acl_access : NULL,
- (resp->mask & NFS_DFACL) ? resp->acl_default : NULL);
struct kvec *head = rqstp->rq_res.head;
+ struct inode *inode;
unsigned int base;
+ int w;
int n;
if (dentry == NULL || dentry->d_inode == NULL)
@@ -239,6 +237,9 @@ static int nfsaclsvc_encode_getaclres(st
return 0;
base = (char *)p - (char *)head->iov_base;
+ w = nfsacl_size(
+ (resp->mask & NFS_ACL) ? resp->acl_access : NULL,
+ (resp->mask & NFS_DFACL) ? resp->acl_default : NULL);
rqstp->rq_res.page_len = w;
while (w > 0) {
if (!rqstp->rq_respages[rqstp->rq_resused++])
--
Jesper Juhl <[email protected]>
Don't top-post http://www.catb.org/~esr/jargon/html/T/top-post.html
Plain text mails only, please http://www.expita.com/nomime.html
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
