afs: potential null dereference
Subsystems (2)
PATCH 1/1 afs: potential null dereference
It seems clear from the surrounding code that xpermits is allowed to be NULL here.
fs/afs/security.c +3 -2
@@ -189,8 +189,9 @@ void afs_cache_permit(struct afs_vnode *vnode, struct key *key, long acl_order)if (!permits) goto out_unlock; -memcpy (permits->permits, xpermits->permits, - count *sizeof (struct afs_permit )); +if (xpermits) +memcpy (permits->permits, xpermits->permits, + count *sizeof (struct afs_permit ));_debug ("key %x access %x" ,key_serial (key), vnode->status.caller_access);
David Howells
maintainerQuoted 2 lines · Dan Carpenter
It seems clear from the surrounding code that xpermits is allowed to be NULL here.
Interesting. The memcpy() won't oops due to this because if it is given a NULL pointer, it will also be given a zero count. I wonder if this means the if-statement your patch adds is actually unnecessary...
David
Quoted 9 lines · David Howells
Quoted 2 lines · Dan Carpenter
It seems clear from the surrounding code that xpermits is allowed to be NULL here.
Interesting. The
memcpy()won't oops due to this because if it is given a NULL pointer, it will also be given a zero count. I wonder if this means the if-statement your patch adds is actually unnecessary...
I was concerned about the dereference here:
+ if (xpermits)
+ memcpy(permits->permits, xpermits->permits,
^^^^^^^^^^^^^^^^^
+ count * sizeof(struct afs_permit));
This code has been there for three years, so yeah, you would think if it were a problem someone would have complained. My theory was "xpermits" was almost always non-null.
regards,
dan carpenter
Quoted 1 line
David
David Howells
maintainerfs/afs/security.c void afs_cache_permit(struct afs_vnode *vnode, struct key *key, long acl_order) · line 194
Quoted 6 lines · Dan Carpenter
I was concerned about the dereference here:
fs/afs/security.c +3
-0+if (xpermits) +memcpy (permits->permits, xpermits->permits, ^^^^^^^^^^^^^^^^^ + count *sizeof (struct afs_permit ));
That's a good point - in which case your patch should definitely go in.
David