* [patch] afs: potential null dereference
@ 2010-03-20 11:19 Dan Carpenter
2010-03-22 12:05 ` David Howells
0 siblings, 1 reply; 5+ messages in thread
From: Dan Carpenter @ 2010-03-20 11:19 UTC (permalink / raw)
To: David Howells; +Cc: linux-afs, linux-kernel, kernel-janitors
It seems clear from the surrounding code that xpermits is allowed to be
NULL here.
Signed-off-by: Dan Carpenter <error27@gmail.com>
diff --git a/fs/afs/security.c b/fs/afs/security.c
index 3ef5043..bb4ed14 100644
--- a/fs/afs/security.c
+++ b/fs/afs/security.c
@@ -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);
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [patch] afs: potential null dereference
2010-03-20 11:19 [patch] afs: potential null dereference Dan Carpenter
@ 2010-03-22 12:05 ` David Howells
2010-03-22 12:56 ` Dan Carpenter
2010-03-22 13:05 ` David Howells
0 siblings, 2 replies; 5+ messages in thread
From: David Howells @ 2010-03-22 12:05 UTC (permalink / raw)
To: Dan Carpenter; +Cc: dhowells, kernel-janitors, linux-afs, linux-kernel
Dan Carpenter <error27@gmail.com> wrote:
> 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch] afs: potential null dereference
2010-03-22 12:05 ` David Howells
@ 2010-03-22 12:56 ` Dan Carpenter
2010-03-22 13:05 ` David Howells
1 sibling, 0 replies; 5+ messages in thread
From: Dan Carpenter @ 2010-03-22 12:56 UTC (permalink / raw)
To: David Howells; +Cc: kernel-janitors, linux-afs, linux-kernel
On Mon, Mar 22, 2010 at 12:05:20PM +0000, David Howells wrote:
> Dan Carpenter <error27@gmail.com> wrote:
>
> > 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
> David
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [patch] afs: potential null dereference
2010-03-22 12:05 ` David Howells
2010-03-22 12:56 ` Dan Carpenter
@ 2010-03-22 13:05 ` David Howells
1 sibling, 0 replies; 5+ messages in thread
From: David Howells @ 2010-03-22 13:05 UTC (permalink / raw)
To: Dan Carpenter; +Cc: dhowells, kernel-janitors, linux-afs, linux-kernel
Dan Carpenter <error27@gmail.com> wrote:
> I was concerned about the dereference here:
>
> + 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
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH] AFS: Potential null dereference
@ 2010-03-22 13:07 David Howells
0 siblings, 0 replies; 5+ messages in thread
From: David Howells @ 2010-03-22 13:07 UTC (permalink / raw)
To: torvalds, akpm
Cc: kernel-janitors, linux-afs, linux-kernel, Dan Carpenter,
David Howells
From: Dan Carpenter <error27@gmail.com>
It seems clear from the surrounding code that xpermits is allowed to be NULL
here.
Signed-off-by: Dan Carpenter <error27@gmail.com>
Signed-off-by: David Howells <dhowells@redhat.com>
---
fs/afs/security.c | 5 +++--
1 files changed, 3 insertions(+), 2 deletions(-)
diff --git a/fs/afs/security.c b/fs/afs/security.c
index 3ef5043..bb4ed14 100644
--- a/fs/afs/security.c
+++ b/fs/afs/security.c
@@ -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);
^ permalink raw reply related [flat|nested] 5+ messages in thread
end of thread, other threads:[~2010-03-22 13:07 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-03-20 11:19 [patch] afs: potential null dereference Dan Carpenter
2010-03-22 12:05 ` David Howells
2010-03-22 12:56 ` Dan Carpenter
2010-03-22 13:05 ` David Howells
-- strict thread matches above, loose matches on Subject: below --
2010-03-22 13:07 [PATCH] AFS: Potential " David Howells
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).