From: Jesper Juhl 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. Signed-off-by: Jesper Juhl Signed-off-by: Andrew Morton --- fs/nfsd/nfs2acl.c | 9 +++++---- 1 files changed, 5 insertions(+), 4 deletions(-) diff -puN fs/nfsd/nfs2acl.c~nfs-nfsaclsvc_encode_getaclres-fix-potential-null-deref-and-tiny-optimization fs/nfsd/nfs2acl.c --- a/fs/nfsd/nfs2acl.c~nfs-nfsaclsvc_encode_getaclres-fix-potential-null-deref-and-tiny-optimization +++ a/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++]) _