The patch titled
NFS: nfsaclsvc_encode_getaclres() - Fix potential NULL deref and tiny optimization.
has been removed from the -mm tree. Its filename was
nfs-nfsaclsvc_encode_getaclres-fix-potential-null-deref-and-tiny-optimization.patch
This patch was dropped because it was nacked by the maintainer
------------------------------------------------------
Subject: NFS: nfsaclsvc_encode_getaclres() - Fix potential NULL deref and tiny optimization.
From: Jesper Juhl <jesper.juhl@xxxxxxxxx>
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 <jesper.juhl@xxxxxxxxx>
Signed-off-by: Andrew Morton <akpm@xxxxxxxx>
---
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++])
_
Patches currently in -mm which might be from jesper.juhl@xxxxxxxxx are
nfs-kill-obsolete-nfs_paranoia.patch
nfs-nfsaclsvc_encode_getaclres-fix-potential-null-deref-and-tiny-optimization.patch
isdn-avoid-a-potential-null-ptr-deref-in-ippp.patch
video-sis-remove-unnecessary-variables-in-sis_ddc2delay.patch
debug-shared-irqs.patch
-
To unsubscribe from this list: send the line "unsubscribe mm-commits" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
