Re: [PATCH] avoid use after free

[Geraldo Netto]

Hello Waldek/All,

Sorry Waldek, I should have specified it better
Actually, I used the clang static analiser to uncover this use after free
(the report i have uploaded):
http://exdev.sourceforge.net/osv/clang-20180122/report-a87eab.html#EndPath

This is a piece of the code, apparently, we use if_freenameindex(ii) on
line 195, but then, on line 236, we use the function fill_mac_addrs() with
ii
Actually, double-checking it now, I see I still need to free ii in other 2
points:

libc/network/getifaddrs.c:
177 int getifaddrs(struct ifaddrs **ifap)
178 {
179 stor *list = 0, *head = 0;
180 struct if_nameindex* ii = if_nameindex();
181 if(!ii) return -1;
182 int addr_count;
183 size_t i;
184 // allocate and add to the list twice the number of
185 // interface of ifaddr storage in order to have enough
186 // for MAC HW addresses that require their own location.
187 for (i = 0; i < 2; i++) {
188 addr_count =
189 allocate_and_add_ifaddrs(, , ii);
190 if (!addr_count) {
191 if_freenameindex(ii);
192 goto err2;
193 }
194 }
195 if_freenameindex(ii);
196
...
236 if (-1 == fill_mac_addrs(sock, head, ii)) {  // Use of memory after it
is freed
237 goto err;
238 }
239 close(sock);
XXX   if_freenameindex(ii); // patch moves the line 195 to here
240 void* last = 0;
241 for(head = list; head; head=(stor*)head->next) last=head;
242 head = last;
243 dealwithipv6(, );
244 *ifap = (struct ifaddrs*) list;
245 return 0;
246 err:
247 close(sock);
XXX   if_freenameindex(ii);  // I think we may need to add another
if_freenameindex(ii) here
248 err2:
XXX   if_freenameindex(ii);  // it seems that we could move the line 191 to
here
249 freeifaddrs((struct ifaddrs*) list);
250 return -1;
251 }

Also, I was checking musl source code and they have a totally different
code now:
https://git.musl-libc.org/cgit/musl/tree/src/network/getifaddrs.c
int getifaddrs(struct ifaddrs **ifap)
{
struct ifaddrs_ctx _ctx, *ctx = &_ctx;
int r;
memset(ctx, 0, sizeof *ctx);
r = __rtnetlink_enumerate(AF_UNSPEC, AF_UNSPEC, netlink_msg_to_ifaddr, ctx);
if (r == 0) *ifap = >first->ifa;
else freeifaddrs(>first->ifa);
return r;
}


Kind Regards,

Geraldo Netto
Sapere Aude => Non dvcor, dvco
http://exdev.sf.net/

On 11 February 2018 at 16:13, Waldek Kozaczuk  wrote:
> I am not really familiar with this code but I do not see where ii is used
> after original call to if_freenameindex. Do you have specific scenario
when
> something would fail before your patch and now passes after it?
Concluding I
> am not convinced this patch is correct. @nyh what do you think?
>
> Also could you please elaborate with at least one sentence why and what
this
> patch fixes instead of "please, review this patch, worked okay for me"?
>
> Thanks,
> Waldek
>
> On Thursday, February 8, 2018 at 8:54:30 PM UTC-5, Geraldo Netto wrote:
>>
>> please, review this patch, worked okay for me
>>
>> Signed-off-by: geraldo netto 
>> ---
>>  libc/network/getifaddrs.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/libc/network/getifaddrs.c b/libc/network/getifaddrs.c
>> index 4cdf4d6..38f5dde 100644
>> --- a/libc/network/getifaddrs.c
>> +++ b/libc/network/getifaddrs.c
>> @@ -192,7 +192,6 @@ int getifaddrs(struct ifaddrs **ifap)
>>  goto err2;
>>  }
>>  }
>> -if_freenameindex(ii);
>>
>>  int sock = socket(PF_INET, SOCK_DGRAM|SOCK_CLOEXEC, IPPROTO_IP);
>>  if(sock == -1) goto err2;
>> @@ -237,6 +236,7 @@ int getifaddrs(struct ifaddrs **ifap)
>>  goto err;
>>  }
>>  close(sock);
>> +if_freenameindex(ii);
>>  void* last = 0;
>>  for(head = list; head; head=(stor*)head->next) last=head;
>>  head = last;
>> --
>> 2.7.4
>>
> --
> You received this message because you are subscribed to the Google Groups
> "OSv Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to osv-dev+unsubscr...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH] reduce hostname length in order to follow procfs hostname implementation

[Waldek Kozaczuk]

I am a little bit reluctant to apply this patch. I do agree that we should 
be consistent (on other hand core/dhcp.cc uses 256 char long buffer - 
another place where it is different than 65). But I am afraid that some 
times especially in cloud environment what gets passed through cloud init 
may be full FQDN 
(https://en.wikipedia.org/wiki/Fully_qualified_domain_name) and OSv does 
not correctly differentiate between hostname and FQDN and 65 may not be 
enough. But maybe 65 is hard limit so we should be OK.

I also found something here 
- 
https://stackoverflow.com/questions/8724954/what-is-the-maximum-number-of-characters-for-a-host-name-in-unix.
  

On Friday, February 9, 2018 at 1:58:49 PM UTC-5, Geraldo Netto wrote:
>
> Signed-off-by: geraldo netto  
> --- 
>  modules/cloud-init/cloud-init.cc | 2 +- 
>  1 file changed, 1 insertion(+), 1 deletion(-) 
>
> diff --git a/modules/cloud-init/cloud-init.cc 
> b/modules/cloud-init/cloud-init.cc 
> index 5e05c82..ddb3a3a 100644 
> --- a/modules/cloud-init/cloud-init.cc 
> +++ b/modules/cloud-init/cloud-init.cc 
> @@ -24,7 +24,7 @@ extern "C" void dhcp_renew(bool wait); 
>  // If hostname changes, try to propagate the change to DHCP server too. 
>  void set_hostname_renew_dhcp(std::string hostname) { 
>  if (hostname.length() > 0) { 
> -char old_hostname[256] = ""; 
> +char old_hostname[65] = ""; 
>  gethostname(old_hostname, sizeof(old_hostname)); 
>  sethostname(hostname.c_str(), hostname.length()); 
>  if (hostname != old_hostname) { 
> -- 
> 2.7.4 
>
>

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: [PATCH] avoid use after free

[Waldek Kozaczuk]

I am not really familiar with this code but I do not see where ii is used 
after original call to if_freenameindex. Do you have specific scenario when 
something would fail before your patch and now passes after it? Concluding 
I am not convinced this patch is correct. @nyh what do you think?

Also could you please elaborate with at least one sentence why and what 
this patch fixes instead of "please, review this patch, worked okay for me"?

Thanks,
Waldek 

On Thursday, February 8, 2018 at 8:54:30 PM UTC-5, Geraldo Netto wrote:
>
> please, review this patch, worked okay for me 
>
> Signed-off-by: geraldo netto  
> --- 
>  libc/network/getifaddrs.c | 2 +- 
>  1 file changed, 1 insertion(+), 1 deletion(-) 
>
> diff --git a/libc/network/getifaddrs.c b/libc/network/getifaddrs.c 
> index 4cdf4d6..38f5dde 100644 
> --- a/libc/network/getifaddrs.c 
> +++ b/libc/network/getifaddrs.c 
> @@ -192,7 +192,6 @@ int getifaddrs(struct ifaddrs **ifap) 
>  goto err2; 
>  } 
>  } 
> -if_freenameindex(ii); 
>   
>  int sock = socket(PF_INET, SOCK_DGRAM|SOCK_CLOEXEC, IPPROTO_IP); 
>  if(sock == -1) goto err2; 
> @@ -237,6 +236,7 @@ int getifaddrs(struct ifaddrs **ifap) 
>  goto err; 
>  } 
>  close(sock); 
> +if_freenameindex(ii); 
>  void* last = 0; 
>  for(head = list; head; head=(stor*)head->next) last=head; 
>  head = last; 
> -- 
> 2.7.4 
>
>

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Re: mysql: failed looking up symbol numa_all_nodes_ptr

[Nadav Har'El]

On Sat, Feb 10, 2018 at 7:31 AM, Geraldo Netto 
wrote:

> Dear Friends,
>
> I was trying to upgrade mysql but in order to upgrade to version 5.6.39
> It seems mysql requires the libraries
> - libnuma.so.1
>

libnuma is a separate shared library, it is not part of glibc, and you
should just
copy it from wherever (e.g., your host) to the image, then the missing
numa_all_nodes_ptr
variable will be defined.

- libcrypt.so.1
>

I think this is not necessary, as OSv includes an implementation of
crypt(), but I don't think
we ever tested this, and it may miss other stuff (like encrypt()). We
should probably choose
one of the following two options, and make things more consistent:
1. If OSv supplies libcrypt.so.1, it should supply all functions (not just
crypt()) and supplied_modules (core/elf.cc) should mention libcrypt.so.1
2. If OSv doesn't supply libcrypt.so.1, it shouldn't include parts of it
(see Makefile doing "musl += crypt/..."

I'm guessing the current mix was useful for something (Java?) which uses
the crypt() function but not anything else from libcrypt.so.1.

-- 
You received this message because you are subscribed to the Google Groups "OSv 
Development" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to osv-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.