netdev - Re: [PATCH net-next v2 5/6] gtp: add socket to pdp context

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 2 Feb 2017 15:38:07 +0100 (CET)
From:   Andreas Schultz <aschultz@...p.net>
To:     pablo <pablo@...filter.org>
Cc:     netdev <netdev@...r.kernel.org>, laforge <laforge@...monks.org>,
        Lionel Gauthier <Lionel.Gauthier@...ecom.fr>,
        openbsc <openbsc@...ts.osmocom.org>
Subject: Re: [PATCH net-next v2 5/6] gtp: add socket to pdp context



----- On Feb 2, 2017, at 3:28 PM, pablo pablo@...filter.org wrote:

> On Thu, Feb 02, 2017 at 03:12:55PM +0100, Andreas Schultz wrote:
>> Hi,
>> 
>> ----- On Feb 2, 2017, at 2:56 PM, pablo pablo@...filter.org wrote:
>> 
>> > On Mon, Jan 30, 2017 at 05:37:12PM +0100, Andreas Schultz wrote:
>> >> Having the socket present in context simplifies the sending logic.
>> >> It also fixes the invalid assumption that we have to use the same
>> >> sending socket for all client IP's on a specific gtp interface.
>> >> 
>> >> Signed-off-by: Andreas Schultz <aschultz@...p.net>
>> >> ---
>> >>  drivers/net/gtp.c | 72 ++++++++++++++++++++++++++++++-------------------------
>> >>  1 file changed, 39 insertions(+), 33 deletions(-)
>> >> 
>> >> diff --git a/drivers/net/gtp.c b/drivers/net/gtp.c
>> >> index 68c6c9b..ff00597 100644
>> >> --- a/drivers/net/gtp.c
>> >> +++ b/drivers/net/gtp.c
>> > [...]
>> >> @@ -984,16 +966,26 @@ static int ipv4_pdp_add(struct gtp_dev *gtp, struct
>> >> genl_info *info)
>> >>  	return 0;
>> >>  }
>> >>  
>> >> +static void pdp_context_free(struct rcu_head *head)
>> >> +{
>> >> +	struct pdp_ctx *pctx = container_of(head, struct pdp_ctx, rcu_head);
>> >> +
>> >> +	sock_put(pctx->sk);
>> >> +	kfree(pctx);
>> >> +}
>> >> +
>> >>  static void pdp_context_delete(struct pdp_ctx *pctx)
>> >>  {
>> >>  	hlist_del_rcu(&pctx->hlist_tid);
>> >>  	hlist_del_rcu(&pctx->hlist_addr);
>> >> -	kfree(pctx);
>> >> +	call_rcu(&pctx->rcu_head, pdp_context_free);
>> > 
>> > This is fixing incorrect rcu conversion in 4/6. Please, fix this there.
>> 
>> Ehm, right, but the kfree in 4/6 could have been a kfree_rcu instead without
>> the call_rcu.
>> 
>> Do you prefer to introduce the call_rcu in 4/6 and then just add the sock_put
>> in this patch or should I change 4/6 to kfree_rcu and do the call_rcu
>> conversion here?
> 
> I suggest you just call kfree_rcu() from 4/6.
> 
> Regarding holding socket reference, see my comment for patch 1/6.

This is going to be a problem at this stage of the changes.

The final goal is to have a reference from the socket to the pdp context.
Then, when the socket is closed, the pdp context can be destroyed.

However, at this point, only the netdevice knows about the contexts. So
when the socket is closed, the pdp context would have a dangling reference
to the socket.

I can integrate this change into the later one. But it will make that
change larger and more difficult to review.

I guess, I'm going to drop this change from this series and see if can
reorder the next part so that it still makes and I easy to review....

Andreas