Re: [PATCH net-next v11 07/23] ovpn: introduce the ovpn_socket object

2024-11-22 Thread Sergey Ryazanov
On November 21, 2024 11:36:19 PM, Antonio Quartulli  wrote:
>On 21/11/2024 00:58, Sergey Ryazanov wrote:
>> On 15.11.2024 16:28, Antonio Quartulli wrote:
>>> On 10/11/2024 19:26, Sergey Ryazanov wrote:
 On 29.10.2024 12:47, Antonio Quartulli wrote:
>> 
>> [...]
>> 
> +static bool ovpn_socket_hold(struct ovpn_socket *sock)
> +{
> +    return kref_get_unless_zero(&sock->refcount);
 
 Why do we need to wrap this kref acquiring call into the function. Why we 
 cannot simply call kref_get_unless_zero() from ovpn_socket_get()?
>>> 
>>> Generally I prefer to keep the API among objects consistent.
>>> In this specific case, it means having hold() and put() helpers in order to 
>>> avoid calling kref_* functions directly in the code.
>>> 
>>> This is a pretty simple case because hold() is called only once, but I 
>>> still like to be consistent.
>> 
>> Make sense. The counterpart ovpn_socket_hold() function declared in the 
>> header file. Probably that's why I missed it. Shall we move the holding 
>> routine there as well?
>
>I prefer not to, because that function is used only in socket.c. 
>Moving/declaring it in socket.h would export a symbols that is not used 
>anywhere else.
>
>The _put() variant is instead use in peer.c, thus it is exported.

Technically, inline function is not exported. On another hand, it makes sense 
to keep header file clean. Agree.

[...]
 
> +int ovpn_udp_socket_attach(struct socket *sock, struct ovpn_struct *ovpn)
> +{
> +    struct ovpn_socket *old_data;
> +    int ret = 0;
> +
> +    /* sanity check */
> +    if (sock->sk->sk_protocol != IPPROTO_UDP) {
 
 The function will be called only for a UDP socket. The caller makes sure 
 this is truth. So, why do we need this check?
>>> 
>>> To avoid this function being copied/called somewhere else in the future and 
>>> we forget about this critical assumption.
>> 
>> Shall we do the same for all other functions in this file? E.g. 
>> ovpn_udp_socket_detach/ovpn_udp_send_skb?
>
>Those functions work on a socket that is already owned, thus it already passed 
>this precheck, while _attach() is the one seeing the new socket for the first 
>time.
>
>If this check is triggered it would only be due to a bug.
>Hence the DEBUG_NET_WARN_ON_ONCE().
>
>> And who is giving guarantee that the code will be copied together with the 
>> check?
>
>No guarantee is given :)
>
>> 
>>> Indeed it's a just sanity check.
>> 
>> Shall we check for pointers validity before dereferencing them?
>> 
>> if (!ovpn || !sock || !sock->sk || !sock->sk->sk_protocol != IPPROTO_UDP) {
>> 
>> With the above questions I would like to show that it's endless number of 
>> possible mistakes. And no matter how much do we check, a creative engineer 
>> will find a way to ruin the kernel.
>> 
>> So, is it worth to spend code lines for checking socket for being UDP inside 
>> a function that has '_udp_' in its name and is called only inside the module?
>
>Are you suggesting we should drop any kind of check for functions called only 
>within the module? I am not sure I follow..

Sanity checks in the internal functions, yes. I'm afraid, they give a false 
feel of safety. Short a clear code for me is more preferable, especially when I 
know in advance who and how going to call a function.

>Anyway, I am dropping the check at the beginning in the function.




Re: [PATCH net-next v11 07/23] ovpn: introduce the ovpn_socket object

2024-11-21 Thread Antonio Quartulli

On 21/11/2024 00:58, Sergey Ryazanov wrote:

On 15.11.2024 16:28, Antonio Quartulli wrote:

On 10/11/2024 19:26, Sergey Ryazanov wrote:

On 29.10.2024 12:47, Antonio Quartulli wrote:


[...]


+static bool ovpn_socket_hold(struct ovpn_socket *sock)
+{
+    return kref_get_unless_zero(&sock->refcount);


Why do we need to wrap this kref acquiring call into the function. 
Why we cannot simply call kref_get_unless_zero() from ovpn_socket_get()?


Generally I prefer to keep the API among objects consistent.
In this specific case, it means having hold() and put() helpers in 
order to avoid calling kref_* functions directly in the code.


This is a pretty simple case because hold() is called only once, but I 
still like to be consistent.


Make sense. The counterpart ovpn_socket_hold() function declared in the 
header file. Probably that's why I missed it. Shall we move the holding 
routine there as well?


I prefer not to, because that function is used only in socket.c. 
Moving/declaring it in socket.h would export a symbols that is not used 
anywhere else.


The _put() variant is instead use in peer.c, thus it is exported.



[...]

+int ovpn_udp_socket_attach(struct socket *sock, struct ovpn_struct 
*ovpn)

+{
+    struct ovpn_socket *old_data;
+    int ret = 0;
+
+    /* sanity check */
+    if (sock->sk->sk_protocol != IPPROTO_UDP) {


The function will be called only for a UDP socket. The caller makes 
sure this is truth. So, why do we need this check?


To avoid this function being copied/called somewhere else in the 
future and we forget about this critical assumption.


Shall we do the same for all other functions in this file? E.g. 
ovpn_udp_socket_detach/ovpn_udp_send_skb?


Those functions work on a socket that is already owned, thus it already 
passed this precheck, while _attach() is the one seeing the new socket 
for the first time.


If this check is triggered it would only be due to a bug.
Hence the DEBUG_NET_WARN_ON_ONCE().

And who is giving guarantee 
that the code will be copied together with the check?


No guarantee is given :)




Indeed it's a just sanity check.


Shall we check for pointers validity before dereferencing them?

if (!ovpn || !sock || !sock->sk || !sock->sk->sk_protocol != IPPROTO_UDP) {

With the above questions I would like to show that it's endless number 
of possible mistakes. And no matter how much do we check, a creative 
engineer will find a way to ruin the kernel.


So, is it worth to spend code lines for checking socket for being UDP 
inside a function that has '_udp_' in its name and is called only inside 
the module?


Are you suggesting we should drop any kind of check for functions called 
only within the module? I am not sure I follow..


Anyway, I am dropping the check at the beginning in the function.

Regards,





+    DEBUG_NET_WARN_ON_ONCE(1);
+    return -EINVAL;
+    }


--
Sergey


--
Antonio Quartulli
OpenVPN Inc.




Re: [PATCH net-next v11 07/23] ovpn: introduce the ovpn_socket object

2024-11-21 Thread Antonio Quartulli

On 21/11/2024 00:34, Sergey Ryazanov wrote:

On 19.11.2024 15:44, Antonio Quartulli wrote:

On 15/11/2024 15:28, Antonio Quartulli wrote:
[...]

+}
+
+static struct ovpn_socket *ovpn_socket_get(struct socket *sock)
+{
+    struct ovpn_socket *ovpn_sock;
+
+    rcu_read_lock();
+    ovpn_sock = rcu_dereference_sk_user_data(sock->sk);
+    if (!ovpn_socket_hold(ovpn_sock)) {
+    pr_warn("%s: found ovpn_socket with ref = 0\n", __func__);


Should we be more specific here and print warning with 
netdev_warn(ovpn_sock->ovpn->dev, ...)?


ACK must be an unnoticed leftover


I take this back.
If refcounter is zero, I'd avoid accessing any field of the ovpn_sock 
object, thus the pr_warn() without any reference to the device.


If it's such unlikely scenario, then should it be:

if (WARN_ON(!ovpn_socket_hold(ovpn_sock)))
     ovpn_sock = NULL;

?


Yeah, makes sense.

Thanks!



--
Sergey


--
Antonio Quartulli
OpenVPN Inc.




Re: [PATCH net-next v11 07/23] ovpn: introduce the ovpn_socket object

2024-11-20 Thread Sergey Ryazanov

On 15.11.2024 16:28, Antonio Quartulli wrote:

On 10/11/2024 19:26, Sergey Ryazanov wrote:

On 29.10.2024 12:47, Antonio Quartulli wrote:


[...]


+static bool ovpn_socket_hold(struct ovpn_socket *sock)
+{
+    return kref_get_unless_zero(&sock->refcount);


Why do we need to wrap this kref acquiring call into the function. Why 
we cannot simply call kref_get_unless_zero() from ovpn_socket_get()?


Generally I prefer to keep the API among objects consistent.
In this specific case, it means having hold() and put() helpers in order 
to avoid calling kref_* functions directly in the code.


This is a pretty simple case because hold() is called only once, but I 
still like to be consistent.


Make sense. The counterpart ovpn_socket_hold() function declared in the 
header file. Probably that's why I missed it. Shall we move the holding 
routine there as well?


[...]

+int ovpn_udp_socket_attach(struct socket *sock, struct ovpn_struct 
*ovpn)

+{
+    struct ovpn_socket *old_data;
+    int ret = 0;
+
+    /* sanity check */
+    if (sock->sk->sk_protocol != IPPROTO_UDP) {


The function will be called only for a UDP socket. The caller makes 
sure this is truth. So, why do we need this check?


To avoid this function being copied/called somewhere else in the future 
and we forget about this critical assumption.


Shall we do the same for all other functions in this file? E.g. 
ovpn_udp_socket_detach/ovpn_udp_send_skb? And who is giving guarantee 
that the code will be copied together with the check?



Indeed it's a just sanity check.


Shall we check for pointers validity before dereferencing them?

if (!ovpn || !sock || !sock->sk || !sock->sk->sk_protocol != IPPROTO_UDP) {

With the above questions I would like to show that it's endless number 
of possible mistakes. And no matter how much do we check, a creative 
engineer will find a way to ruin the kernel.


So, is it worth to spend code lines for checking socket for being UDP 
inside a function that has '_udp_' in its name and is called only inside 
the module?



+    DEBUG_NET_WARN_ON_ONCE(1);
+    return -EINVAL;
+    }


--
Sergey



Re: [PATCH net-next v11 07/23] ovpn: introduce the ovpn_socket object

2024-11-20 Thread Sergey Ryazanov

On 19.11.2024 15:44, Antonio Quartulli wrote:

On 15/11/2024 15:28, Antonio Quartulli wrote:
[...]

+}
+
+static struct ovpn_socket *ovpn_socket_get(struct socket *sock)
+{
+    struct ovpn_socket *ovpn_sock;
+
+    rcu_read_lock();
+    ovpn_sock = rcu_dereference_sk_user_data(sock->sk);
+    if (!ovpn_socket_hold(ovpn_sock)) {
+    pr_warn("%s: found ovpn_socket with ref = 0\n", __func__);


Should we be more specific here and print warning with 
netdev_warn(ovpn_sock->ovpn->dev, ...)?


ACK must be an unnoticed leftover


I take this back.
If refcounter is zero, I'd avoid accessing any field of the ovpn_sock 
object, thus the pr_warn() without any reference to the device.


If it's such unlikely scenario, then should it be:

if (WARN_ON(!ovpn_socket_hold(ovpn_sock)))
ovpn_sock = NULL;

?

--
Sergey



Re: [PATCH net-next v11 07/23] ovpn: introduce the ovpn_socket object

2024-11-19 Thread Antonio Quartulli

On 15/11/2024 15:28, Antonio Quartulli wrote:
[...]

+}
+
+static struct ovpn_socket *ovpn_socket_get(struct socket *sock)
+{
+    struct ovpn_socket *ovpn_sock;
+
+    rcu_read_lock();
+    ovpn_sock = rcu_dereference_sk_user_data(sock->sk);
+    if (!ovpn_socket_hold(ovpn_sock)) {
+    pr_warn("%s: found ovpn_socket with ref = 0\n", __func__);


Should we be more specific here and print warning with 
netdev_warn(ovpn_sock->ovpn->dev, ...)?


ACK must be an unnoticed leftover


I take this back.
If refcounter is zero, I'd avoid accessing any field of the ovpn_sock 
object, thus the pr_warn() without any reference to the device.


Regards,

--
Antonio Quartulli
OpenVPN Inc.




Re: [PATCH net-next v11 07/23] ovpn: introduce the ovpn_socket object

2024-11-15 Thread Antonio Quartulli

On 10/11/2024 19:26, Sergey Ryazanov wrote:

On 29.10.2024 12:47, Antonio Quartulli wrote:

This specific structure is used in the ovpn kernel module
to wrap and carry around a standard kernel socket.

ovpn takes ownership of passed sockets and therefore an ovpn
specific objects is attached to them for status tracking
purposes.

Initially only UDP support is introduced. TCP will come in a later
patch.

Signed-off-by: Antonio Quartulli 


[...]


diff --git a/drivers/net/ovpn/socket.c b/drivers/net/ovpn/socket.c
new file mode 100644
index 
..090a3232ab0ec19702110f1a90f45c7f10889f6f

--- /dev/null
+++ b/drivers/net/ovpn/socket.c
@@ -0,0 +1,120 @@
+// SPDX-License-Identifier: GPL-2.0
+/*  OpenVPN data channel offload
+ *
+ *  Copyright (C) 2020-2024 OpenVPN, Inc.
+ *
+ *  Author:    James Yonan 
+ *    Antonio Quartulli 
+ */
+
+#include 
+#include 
+
+#include "ovpnstruct.h"
+#include "main.h"
+#include "io.h"
+#include "peer.h"
+#include "socket.h"
+#include "udp.h"
+
+static void ovpn_socket_detach(struct socket *sock)
+{
+    if (!sock)
+    return;
+
+    sockfd_put(sock);
+}
+
+/**
+ * ovpn_socket_release_kref - kref_put callback
+ * @kref: the kref object
+ */
+void ovpn_socket_release_kref(struct kref *kref)
+{
+    struct ovpn_socket *sock = container_of(kref, struct ovpn_socket,
+    refcount);
+
+    ovpn_socket_detach(sock->sock);
+    kfree_rcu(sock, rcu);
+}
+
+static bool ovpn_socket_hold(struct ovpn_socket *sock)
+{
+    return kref_get_unless_zero(&sock->refcount);


Why do we need to wrap this kref acquiring call into the function. Why 
we cannot simply call kref_get_unless_zero() from ovpn_socket_get()?


Generally I prefer to keep the API among objects consistent.
In this specific case, it means having hold() and put() helpers in order 
to avoid calling kref_* functions directly in the code.


This is a pretty simple case because hold() is called only once, but I 
still like to be consistent.





+}
+
+static struct ovpn_socket *ovpn_socket_get(struct socket *sock)
+{
+    struct ovpn_socket *ovpn_sock;
+
+    rcu_read_lock();
+    ovpn_sock = rcu_dereference_sk_user_data(sock->sk);
+    if (!ovpn_socket_hold(ovpn_sock)) {
+    pr_warn("%s: found ovpn_socket with ref = 0\n", __func__);


Should we be more specific here and print warning with 
netdev_warn(ovpn_sock->ovpn->dev, ...)?


ACK must be an unnoticed leftover



And, BTW, how we can pick-up a half-destroyed socket?


I don't think this can happen under basic conditions.
But I am pretty sure in case of bugs this *could* happen quite easily.

[...]


+/**
+ * ovpn_udp_socket_attach - set udp-tunnel CBs on socket and link it 
to ovpn

+ * @sock: socket to configure
+ * @ovpn: the openvp instance to link
+ *
+ * After invoking this function, the sock will be controlled by ovpn 
so that

+ * any incoming packet may be processed by ovpn first.
+ *
+ * Return: 0 on success or a negative error code otherwise
+ */
+int ovpn_udp_socket_attach(struct socket *sock, struct ovpn_struct 
*ovpn)

+{
+    struct ovpn_socket *old_data;
+    int ret = 0;
+
+    /* sanity check */
+    if (sock->sk->sk_protocol != IPPROTO_UDP) {


The function will be called only for a UDP socket. The caller makes sure 
this is truth. So, why do we need this check?


To avoid this function being copied/called somewhere else in the future 
and we forget about this critical assumption.


Indeed it's a just sanity check.




+    DEBUG_NET_WARN_ON_ONCE(1);
+    return -EINVAL;
+    }
+
+    /* make sure no pre-existing encapsulation handler exists */
+    rcu_read_lock();
+    old_data = rcu_dereference_sk_user_data(sock->sk);
+    if (!old_data) {
+    /* socket is currently unused - we can take it */
+    rcu_read_unlock();
+    return 0;
+    }
+
+    /* socket is in use. We need to understand if it's owned by this 
ovpn

+ * instance or by something else.
+ * In the former case, we can increase the refcounter and happily
+ * use it, because the same UDP socket is expected to be shared 
among

+ * different peers.
+ *
+ * Unlikely TCP, a single UDP socket can be used to talk to many 
remote
+ * hosts and therefore openvpn instantiates one only for all its 
peers

+ */
+    if ((READ_ONCE(udp_sk(sock->sk)->encap_type) == 
UDP_ENCAP_OVPNINUDP) &&

+    old_data->ovpn == ovpn) {
+    netdev_dbg(ovpn->dev,
+   "%s: provided socket already owned by this interface\n",
+   __func__);


Why do we need the function name being printed here?


leftover, will fix, thanks!




+    ret = -EALREADY;
+    } else {
+    netdev_err(ovpn->dev,
+   "%s: provided socket already taken by other user\n",
+   __func__);


The same comment regarding the function name printing.


ACK



And why 'error' level? There is a few ways to fall into this case and 
each of them implies a user-space screw up. But why

Re: [PATCH net-next v11 07/23] ovpn: introduce the ovpn_socket object

2024-11-10 Thread Sergey Ryazanov

On 29.10.2024 12:47, Antonio Quartulli wrote:

This specific structure is used in the ovpn kernel module
to wrap and carry around a standard kernel socket.

ovpn takes ownership of passed sockets and therefore an ovpn
specific objects is attached to them for status tracking
purposes.

Initially only UDP support is introduced. TCP will come in a later
patch.

Signed-off-by: Antonio Quartulli 


[...]


diff --git a/drivers/net/ovpn/socket.c b/drivers/net/ovpn/socket.c
new file mode 100644
index 
..090a3232ab0ec19702110f1a90f45c7f10889f6f
--- /dev/null
+++ b/drivers/net/ovpn/socket.c
@@ -0,0 +1,120 @@
+// SPDX-License-Identifier: GPL-2.0
+/*  OpenVPN data channel offload
+ *
+ *  Copyright (C) 2020-2024 OpenVPN, Inc.
+ *
+ *  Author:James Yonan 
+ * Antonio Quartulli 
+ */
+
+#include 
+#include 
+
+#include "ovpnstruct.h"
+#include "main.h"
+#include "io.h"
+#include "peer.h"
+#include "socket.h"
+#include "udp.h"
+
+static void ovpn_socket_detach(struct socket *sock)
+{
+   if (!sock)
+   return;
+
+   sockfd_put(sock);
+}
+
+/**
+ * ovpn_socket_release_kref - kref_put callback
+ * @kref: the kref object
+ */
+void ovpn_socket_release_kref(struct kref *kref)
+{
+   struct ovpn_socket *sock = container_of(kref, struct ovpn_socket,
+   refcount);
+
+   ovpn_socket_detach(sock->sock);
+   kfree_rcu(sock, rcu);
+}
+
+static bool ovpn_socket_hold(struct ovpn_socket *sock)
+{
+   return kref_get_unless_zero(&sock->refcount);


Why do we need to wrap this kref acquiring call into the function. Why 
we cannot simply call kref_get_unless_zero() from ovpn_socket_get()?



+}
+
+static struct ovpn_socket *ovpn_socket_get(struct socket *sock)
+{
+   struct ovpn_socket *ovpn_sock;
+
+   rcu_read_lock();
+   ovpn_sock = rcu_dereference_sk_user_data(sock->sk);
+   if (!ovpn_socket_hold(ovpn_sock)) {
+   pr_warn("%s: found ovpn_socket with ref = 0\n", __func__);


Should we be more specific here and print warning with 
netdev_warn(ovpn_sock->ovpn->dev, ...)?


And, BTW, how we can pick-up a half-destroyed socket?


+   ovpn_sock = NULL;
+   }
+   rcu_read_unlock();
+
+   return ovpn_sock;
+}
+
+static int ovpn_socket_attach(struct socket *sock, struct ovpn_peer *peer)
+{
+   int ret = -EOPNOTSUPP;
+
+   if (!sock || !peer)
+   return -EINVAL;
+
+   if (sock->sk->sk_protocol == IPPROTO_UDP)
+   ret = ovpn_udp_socket_attach(sock, peer->ovpn);
+
+   return ret;
+}
+
+/**
+ * ovpn_socket_new - create a new socket and initialize it
+ * @sock: the kernel socket to embed
+ * @peer: the peer reachable via this socket
+ *
+ * Return: an openvpn socket on success or a negative error code otherwise
+ */
+struct ovpn_socket *ovpn_socket_new(struct socket *sock, struct ovpn_peer 
*peer)
+{
+   struct ovpn_socket *ovpn_sock;
+   int ret;
+
+   ret = ovpn_socket_attach(sock, peer);
+   if (ret < 0 && ret != -EALREADY)
+   return ERR_PTR(ret);
+
+   /* if this socket is already owned by this interface, just increase the
+* refcounter and use it as expected.
+*
+* Since UDP sockets can be used to talk to multiple remote endpoints,
+* openvpn normally instantiates only one socket and shares it among all
+* its peers. For this reason, when we find out that a socket is already
+* used for some other peer in *this* instance, we can happily increase
+* its refcounter and use it normally.
+*/
+   if (ret == -EALREADY) {
+   /* caller is expected to increase the sock refcounter before
+* passing it to this function. For this reason we drop it if
+* not needed, like when this socket is already owned.
+*/
+   ovpn_sock = ovpn_socket_get(sock);
+   sockfd_put(sock);
+   return ovpn_sock;
+   }
+
+   ovpn_sock = kzalloc(sizeof(*ovpn_sock), GFP_KERNEL);
+   if (!ovpn_sock)
+   return ERR_PTR(-ENOMEM);
+
+   ovpn_sock->ovpn = peer->ovpn;
+   ovpn_sock->sock = sock;
+   kref_init(&ovpn_sock->refcount);
+
+   rcu_assign_sk_user_data(sock->sk, ovpn_sock);
+
+   return ovpn_sock;
+}
diff --git a/drivers/net/ovpn/socket.h b/drivers/net/ovpn/socket.h
new file mode 100644
index 
..5ad9c5073b085482da95ee8ebf40acf20bf2e4b3
--- /dev/null
+++ b/drivers/net/ovpn/socket.h
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*  OpenVPN data channel offload
+ *
+ *  Copyright (C) 2020-2024 OpenVPN, Inc.
+ *
+ *  Author:James Yonan 
+ * Antonio Quartulli 
+ */
+
+#ifndef _NET_OVPN_SOCK_H_
+#define _NET_OVPN_SOCK_H_
+
+#include 
+#include 
+#include 
+
+struct ovpn_struct;
+struct ovpn_peer;
+
+/**
+ * struct ovpn_socket - a kernel socket ref

[PATCH net-next v11 07/23] ovpn: introduce the ovpn_socket object

2024-10-29 Thread Antonio Quartulli
This specific structure is used in the ovpn kernel module
to wrap and carry around a standard kernel socket.

ovpn takes ownership of passed sockets and therefore an ovpn
specific objects is attached to them for status tracking
purposes.

Initially only UDP support is introduced. TCP will come in a later
patch.

Signed-off-by: Antonio Quartulli 
---
 drivers/net/ovpn/Makefile |   2 +
 drivers/net/ovpn/socket.c | 120 ++
 drivers/net/ovpn/socket.h |  48 +++
 drivers/net/ovpn/udp.c|  72 
 drivers/net/ovpn/udp.h|  17 +++
 5 files changed, 259 insertions(+)

diff --git a/drivers/net/ovpn/Makefile b/drivers/net/ovpn/Makefile
index 
ce13499b3e1775a7f2a9ce16c6cb0aa088f93685..56bddc9bef83e0befde6af3c3565bb91731d7b22
 100644
--- a/drivers/net/ovpn/Makefile
+++ b/drivers/net/ovpn/Makefile
@@ -13,3 +13,5 @@ ovpn-y += io.o
 ovpn-y += netlink.o
 ovpn-y += netlink-gen.o
 ovpn-y += peer.o
+ovpn-y += socket.o
+ovpn-y += udp.o
diff --git a/drivers/net/ovpn/socket.c b/drivers/net/ovpn/socket.c
new file mode 100644
index 
..090a3232ab0ec19702110f1a90f45c7f10889f6f
--- /dev/null
+++ b/drivers/net/ovpn/socket.c
@@ -0,0 +1,120 @@
+// SPDX-License-Identifier: GPL-2.0
+/*  OpenVPN data channel offload
+ *
+ *  Copyright (C) 2020-2024 OpenVPN, Inc.
+ *
+ *  Author:James Yonan 
+ * Antonio Quartulli 
+ */
+
+#include 
+#include 
+
+#include "ovpnstruct.h"
+#include "main.h"
+#include "io.h"
+#include "peer.h"
+#include "socket.h"
+#include "udp.h"
+
+static void ovpn_socket_detach(struct socket *sock)
+{
+   if (!sock)
+   return;
+
+   sockfd_put(sock);
+}
+
+/**
+ * ovpn_socket_release_kref - kref_put callback
+ * @kref: the kref object
+ */
+void ovpn_socket_release_kref(struct kref *kref)
+{
+   struct ovpn_socket *sock = container_of(kref, struct ovpn_socket,
+   refcount);
+
+   ovpn_socket_detach(sock->sock);
+   kfree_rcu(sock, rcu);
+}
+
+static bool ovpn_socket_hold(struct ovpn_socket *sock)
+{
+   return kref_get_unless_zero(&sock->refcount);
+}
+
+static struct ovpn_socket *ovpn_socket_get(struct socket *sock)
+{
+   struct ovpn_socket *ovpn_sock;
+
+   rcu_read_lock();
+   ovpn_sock = rcu_dereference_sk_user_data(sock->sk);
+   if (!ovpn_socket_hold(ovpn_sock)) {
+   pr_warn("%s: found ovpn_socket with ref = 0\n", __func__);
+   ovpn_sock = NULL;
+   }
+   rcu_read_unlock();
+
+   return ovpn_sock;
+}
+
+static int ovpn_socket_attach(struct socket *sock, struct ovpn_peer *peer)
+{
+   int ret = -EOPNOTSUPP;
+
+   if (!sock || !peer)
+   return -EINVAL;
+
+   if (sock->sk->sk_protocol == IPPROTO_UDP)
+   ret = ovpn_udp_socket_attach(sock, peer->ovpn);
+
+   return ret;
+}
+
+/**
+ * ovpn_socket_new - create a new socket and initialize it
+ * @sock: the kernel socket to embed
+ * @peer: the peer reachable via this socket
+ *
+ * Return: an openvpn socket on success or a negative error code otherwise
+ */
+struct ovpn_socket *ovpn_socket_new(struct socket *sock, struct ovpn_peer 
*peer)
+{
+   struct ovpn_socket *ovpn_sock;
+   int ret;
+
+   ret = ovpn_socket_attach(sock, peer);
+   if (ret < 0 && ret != -EALREADY)
+   return ERR_PTR(ret);
+
+   /* if this socket is already owned by this interface, just increase the
+* refcounter and use it as expected.
+*
+* Since UDP sockets can be used to talk to multiple remote endpoints,
+* openvpn normally instantiates only one socket and shares it among all
+* its peers. For this reason, when we find out that a socket is already
+* used for some other peer in *this* instance, we can happily increase
+* its refcounter and use it normally.
+*/
+   if (ret == -EALREADY) {
+   /* caller is expected to increase the sock refcounter before
+* passing it to this function. For this reason we drop it if
+* not needed, like when this socket is already owned.
+*/
+   ovpn_sock = ovpn_socket_get(sock);
+   sockfd_put(sock);
+   return ovpn_sock;
+   }
+
+   ovpn_sock = kzalloc(sizeof(*ovpn_sock), GFP_KERNEL);
+   if (!ovpn_sock)
+   return ERR_PTR(-ENOMEM);
+
+   ovpn_sock->ovpn = peer->ovpn;
+   ovpn_sock->sock = sock;
+   kref_init(&ovpn_sock->refcount);
+
+   rcu_assign_sk_user_data(sock->sk, ovpn_sock);
+
+   return ovpn_sock;
+}
diff --git a/drivers/net/ovpn/socket.h b/drivers/net/ovpn/socket.h
new file mode 100644
index 
..5ad9c5073b085482da95ee8ebf40acf20bf2e4b3
--- /dev/null
+++ b/drivers/net/ovpn/socket.h
@@ -0,0 +1,48 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+/*  OpenVPN data chann