[Openvpn-devel] Add missing check for nl_socket_alloc failure

Message ID 20230214135658.1034787-1-arne@rfc2549.org
State Changes Requested
Headers show
Series [Openvpn-devel] Add missing check for nl_socket_alloc failure | expand

Commit Message

Arne Schwabe Feb. 14, 2023, 1:56 p.m. UTC
This can happen if the memory alloc fails.

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 src/openvpn/dco_linux.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

Comments

Gert Doering Feb. 14, 2023, 2:01 p.m. UTC | #1
Hi,

On Tue, Feb 14, 2023 at 02:56:58PM +0100, Arne Schwabe wrote:
>  resolve_ovpn_netlink_id(int msglevel)
>  {
> -    int ret;
>      struct nl_sock *nl_sock = nl_socket_alloc();
>  
> -    ret = genl_connect(nl_sock);
> +    if (!nl_sock)
> +    {
> +        msg(msglevel, "Allocating net link socket failed");
> +    }

... shouldn't we abort in this case, aka, M_FATAL (or "return -errno")?

gert
Arne Schwabe March 20, 2023, 5:02 p.m. UTC | #2
Am 14.02.23 um 15:01 schrieb Gert Doering:
> Hi,
> 
> On Tue, Feb 14, 2023 at 02:56:58PM +0100, Arne Schwabe wrote:
>>   resolve_ovpn_netlink_id(int msglevel)
>>   {
>> -    int ret;
>>       struct nl_sock *nl_sock = nl_socket_alloc();
>>   
>> -    ret = genl_connect(nl_sock);
>> +    if (!nl_sock)
>> +    {
>> +        msg(msglevel, "Allocating net link socket failed");
>> +    }
> 
> ... shouldn't we abort in this case, aka, M_FATAL (or "return -errno")?

I don't think so. This function is called from two code paths:

- dco_available: I don't think if detection of dco fails, we should quit 
openvpn completely.

- ovpn_dco_init_netlink: Here we already call it with M_ERR, which is 
   (M_FATAL | M_ERRNO)

Arne

Patch

diff --git a/src/openvpn/dco_linux.c b/src/openvpn/dco_linux.c
index c84f9cfe1..b1103c8d5 100644
--- a/src/openvpn/dco_linux.c
+++ b/src/openvpn/dco_linux.c
@@ -79,10 +79,14 @@  typedef int (*ovpn_nl_cb)(struct nl_msg *msg, void *arg);
 static int
 resolve_ovpn_netlink_id(int msglevel)
 {
-    int ret;
     struct nl_sock *nl_sock = nl_socket_alloc();
 
-    ret = genl_connect(nl_sock);
+    if (!nl_sock)
+    {
+        msg(msglevel, "Allocating net link socket failed");
+    }
+
+    int ret = genl_connect(nl_sock);
     if (ret)
     {
         msg(msglevel, "Cannot connect to generic netlink: %s",