mem-map leak in loc_database_free()

mem-map leak in loc_database_free()

[Gisle Vanem]

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

Hello list.

In my Windows program I use the 'libloc' library
and the compressed 'location.db.xz' file. I also have
some functions to download, decompress to a temp-file.
Then finally using 'CopyFile()' to update to the final
.db-file.

But my problem was that 'CopyFile()' failed with error
'ERROR_USER_MAPPED_FILE' since some regions to the file
was not closed.

I saw that 'libloc' does 5 calls to 'mmap()' but only
4 calls to 'munmap()' in 'loc_database_free()'; the freeing
of 'db->countries_v1' is missing.

So with this patch my program works fine:

--- a/database.c 2020-12-03 15:04:38
+++ b/database.c 2021-03-21 17:21:04

@@ -446,6 +446,13 @@
                         ERROR(db->ctx, "Could not unmap network nodes section: %s\n", strerror(errno));
         }

+       // Remove mapped countries nodes section
+       if (db->countries_v1) {
+               r = munmap(db->countries_v1, db->countries_count * sizeof(*db->countries_v1));
+               if (r)
+                       ERROR(db->ctx, "Could not unmap countries nodes section: %s\n", strerror(errno));
+       }
+
         if (db->pool)
                 loc_stringpool_unref(db->pool);
---------

Not sure how 'libloc' could work w/o this on Linux.
What does a 'cp /tmp/location.db location-in-use.db'
say when running one of the test programs in parallel?

-- 
--gv

Re: mem-map leak in loc_database_free()

[Michael Tremer]

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

Hello Gisle,

Thank you for reporting this problem.

Copying files under Linux works even when they are left open like this.

However, I merged your patch.

-Michael

> On 21 Mar 2021, at 16:35, Gisle Vanem <gisle.vanem(a)gmail.com> wrote:
> 
> Hello list.
> 
> In my Windows program I use the 'libloc' library
> and the compressed 'location.db.xz' file. I also have
> some functions to download, decompress to a temp-file.
> Then finally using 'CopyFile()' to update to the final
> .db-file.
> 
> But my problem was that 'CopyFile()' failed with error
> 'ERROR_USER_MAPPED_FILE' since some regions to the file
> was not closed.
> 
> I saw that 'libloc' does 5 calls to 'mmap()' but only
> 4 calls to 'munmap()' in 'loc_database_free()'; the freeing
> of 'db->countries_v1' is missing.
> 
> So with this patch my program works fine:
> 
> --- a/database.c 2020-12-03 15:04:38
> +++ b/database.c 2021-03-21 17:21:04
> 
> @@ -446,6 +446,13 @@
>                        ERROR(db->ctx, "Could not unmap network nodes section: %s\n", strerror(errno));
>        }
> 
> +       // Remove mapped countries nodes section
> +       if (db->countries_v1) {
> +               r = munmap(db->countries_v1, db->countries_count * sizeof(*db->countries_v1));
> +               if (r)
> +                       ERROR(db->ctx, "Could not unmap countries nodes section: %s\n", strerror(errno));
> +       }
> +
>        if (db->pool)
>                loc_stringpool_unref(db->pool);
> ---------
> 
> Not sure how 'libloc' could work w/o this on Linux.
> What does a 'cp /tmp/location.db location-in-use.db'
> say when running one of the test programs in parallel?
> 
> -- 
> --gv