Messages in this thread Patch in this message | | | Date | Thu, 7 Jul 2005 14:59:29 -0400 | From | Dave Jones <> | Subject | [PATCH] fix use after free in smbfs. |
| |
From code inspection it seems that after freeing 'req', in smb_request() we continue to dereference it a dozen or so times.
That whole area of code looks suspect. ... smb_lock_server(server); if (!(req->rq_flags & SMB_REQ_RECEIVED)) { list_del_init(&req->rq_queue); smb_rput(req); }
smb_rput() also does a list_del_init, but only if its safe to do so (ie, the refcount has dropped to 0). To my not-smbfs-savvy eyes, it would seem that we could potentially nuke the ->rq_queue list, and return from smb_rput() if the request was still in use. What the rest of the code does with such a buggered-up request, I've no idea, but it probably isn't pretty.
Perhaps smb_rput should be taking a pointer to a request that can be null'd on success ?
Signed-off-by: Dave Jones <davej@redhat.com>
--- linux-2.6.12/fs/smbfs/request.c~ 2005-07-07 14:41:11.000000000 -0400 +++ linux-2.6.12/fs/smbfs/request.c 2005-07-07 14:41:22.000000000 -0400 @@ -348,6 +348,7 @@ int smb_add_request(struct smb_request * smb_rput(req); } smb_unlock_server(server); + return -EINTR; } if (!timeleft) {
Looking further, we do exactly the same thing in smb_request_recv()
smb_rput(req); wake_up_interruptible(&req->rq_wait); } ditto in smbiod.c..
What am I missing here? smb_rput() -> smb_free_request() does a kmem_cache_free(req_cachep, req); making further use of that cache item invalid.
Dave
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |