[PATCH] net: don't free uninitialized sockets in dns

[PATCH] net: don't free uninitialized sockets in dns

[Josef Bacik]

If we cannot open a connection to our dns server we will have NULL sockets in
our array, so don't do the cleanup on any sockets that didn't get created.

Signed-off-by: Josef Bacik <jbacik@fb.com>
---
 grub-core/net/dns.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/grub-core/net/dns.c b/grub-core/net/dns.c
index 9d0c8fc..c356318 100644
--- a/grub-core/net/dns.c
+++ b/grub-core/net/dns.c
@@ -601,7 +601,10 @@ grub_net_dns_lookup (const char *name,
   grub_free (data.name);
   grub_netbuff_free (nb);
   for (j = 0; j < send_servers; j++)
-    grub_net_udp_close (sockets[j]);
+    {
+      if (sockets[j])
+	grub_net_udp_close (sockets[j]);
+    }
   
   grub_free (sockets);
 
-- 
1.8.1

Re: [PATCH] net: don't free uninitialized sockets in dns

[Andrei Borzenkov]

[-- Attachment #1: Type: text/plain, Size: 874 bytes --]

18.08.2015 21:03, Josef Bacik пишет:
> If we cannot open a connection to our dns server we will have NULL sockets in
> our array, so don't do the cleanup on any sockets that didn't get created.
>

We can avoid having holes to start with. Could you test attached patch?

> Signed-off-by: Josef Bacik <jbacik@fb.com>
> ---
>   grub-core/net/dns.c | 5 ++++-
>   1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/grub-core/net/dns.c b/grub-core/net/dns.c
> index 9d0c8fc..c356318 100644
> --- a/grub-core/net/dns.c
> +++ b/grub-core/net/dns.c
> @@ -601,7 +601,10 @@ grub_net_dns_lookup (const char *name,
>     grub_free (data.name);
>     grub_netbuff_free (nb);
>     for (j = 0; j < send_servers; j++)
> -    grub_net_udp_close (sockets[j]);
> +    {
> +      if (sockets[j])
> +	grub_net_udp_close (sockets[j]);
> +    }
>
>     grub_free (sockets);
>
>


[-- Attachment #2: dns-null-free.patch --]
[-- Type: text/x-patch, Size: 1812 bytes --]

From: Andrei Borzenkov <arvidjaar@gmail.com>
Subject: [PATCH] net: avoid closing NULL socket in DNS lookup

Refactor code so that we do not store NULL pointers in array
of in-flight DNS servers.

Reported-By: Josef Bacik <jbacik@fb.com>


---
 grub-core/net/dns.c | 16 +++++++---------
 1 file changed, 7 insertions(+), 9 deletions(-)

diff --git a/grub-core/net/dns.c b/grub-core/net/dns.c
index 9d0c8fc..953f3be 100644
--- a/grub-core/net/dns.c
+++ b/grub-core/net/dns.c
@@ -437,7 +437,7 @@ grub_net_dns_lookup (const char *name,
   struct recv_data data = {naddresses, addresses, cache,
 			   grub_cpu_to_be16 (id++), 0, 0, name, 0};
   grub_uint8_t *nbd;
-  int have_server = 0;
+  grub_size_t try_server = 0;
 
   if (!servers)
     {
@@ -543,33 +543,31 @@ grub_net_dns_lookup (const char *name,
   for (i = 0; i < n_servers * 4; i++)
     {
       /* Connect to a next server.  */
-      while (!(i & 1) && send_servers < n_servers)
+      while (!(i & 1) && try_server < n_servers)
 	{
-	  sockets[send_servers] = grub_net_udp_open (servers[send_servers],
+	  sockets[send_servers] = grub_net_udp_open (servers[try_server++],
 						     DNS_PORT,
 						     recv_hook,
 						     &data);
-	  send_servers++;
-	  if (!sockets[send_servers - 1])
+	  if (!sockets[send_servers])
 	    {
 	      err = grub_errno;
 	      grub_errno = GRUB_ERR_NONE;
 	    }
 	  else
 	    {
-	      have_server = 1;
+	      send_servers++;
 	      break;
 	    }
 	}
-      if (!have_server)
+      if (!send_servers)
 	goto out;
       if (*data.naddresses)
 	goto out;
       for (j = 0; j < send_servers; j++)
 	{
           grub_err_t err2;
-          if (!sockets[j])
-            continue;
+
           nb->data = nbd;
 
           grub_size_t t = 0;
-- 
tg: (ba218c1..) u/dns-NULL-dereference (depends on: master)

Re: [PATCH] net: don't free uninitialized sockets in dns

[Andrei Borzenkov]

ping?

19.08.2015 19:36, Andrei Borzenkov пишет:
> 18.08.2015 21:03, Josef Bacik пишет:
>> If we cannot open a connection to our dns server we will have NULL
>> sockets in
>> our array, so don't do the cleanup on any sockets that didn't get
>> created.
>>
>
> We can avoid having holes to start with. Could you test attached patch?
>
>> Signed-off-by: Josef Bacik <jbacik@fb.com>
>> ---
>>   grub-core/net/dns.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/grub-core/net/dns.c b/grub-core/net/dns.c
>> index 9d0c8fc..c356318 100644
>> --- a/grub-core/net/dns.c
>> +++ b/grub-core/net/dns.c
>> @@ -601,7 +601,10 @@ grub_net_dns_lookup (const char *name,
>>     grub_free (data.name);
>>     grub_netbuff_free (nb);
>>     for (j = 0; j < send_servers; j++)
>> -    grub_net_udp_close (sockets[j]);
>> +    {
>> +      if (sockets[j])
>> +    grub_net_udp_close (sockets[j]);
>> +    }
>>
>>     grub_free (sockets);
>>
>>
>

Re: [PATCH] net: don't free uninitialized sockets in dns

[Josef Bacik]

On 10/10/2015 03:03 AM, Andrei Borzenkov wrote:
> ping?

Oops sorry forgot to tell you this patch worked as well.  Thanks,

Josef