Re: [PATCH v2 3/3] pppoatm: protect against freeing of vcc

On Tue, Oct 30, 2012 at 09:39:22AM +0000, David Woodhouse wrote:

On Mon, 2012-10-22 at 19:14 +0200, Krzysztof Mazur wrote:

quoted

The pppoatm gets a reference to atmvcc, but does not increment vcc
usage count. The vcc uses vcc->sk socket for reference counting,
so sock_hold() and sock_put() should be used by pppoatm.

Signed-off-by: Krzysztof Mazur <redacted>
Cc: David Woodhouse <dwmw2@infradead.org>

Acked-By: David Woodhouse <redacted>

This patch is not needed, because vcc_destroy_socket()
calls pppoatm_push(vcc, NULL) to indicate that vcc is now closed,
before vcc_release() calls sock_put() and it's properly handled
by pppoatm.

I will drop this patch.

But did you spot what's in the end of the context of the first hunk...?

quoted

--- a/net/atm/pppoatm.c
+++ b/net/atm/pppoatm.c

@@ -154,6 +154,7 @@ static void pppoatm_unassign_vcc(struct atm_vcc

*atmvcc)
        tasklet_kill(&pvcc->wakeup_tasklet);
        ppp_unregister_channel(&pvcc->chan);
        atmvcc->user_back = NULL;
+       sock_put(sk_atm(pvcc->atmvcc));
        kfree(pvcc);
        /* Gee, I hope we have the big kernel lock here... */
        module_put(THIS_MODULE);

Fairly sure that hope is unfounded these days... :)

Yes, I saw that.

Thanks.

Krzysiek