Merge pull request #2729 from KlaraSystems/des/leak-on-fatal

Don't leak memory on fatal error

(cherry picked from commit 372e709c1a143c08281fef76edaf84db42327559)

diff --git a/libarchive/archive_write.c b/libarchive/archive_write.c
index a8e7b63b5bfed86b577c35d42199685056914602..9b9cb196f0f9c0d9bde2cdeec17d675760cafd02 100644 (file)
--- a/libarchive/archive_write.c
+++ b/libarchive/archive_write.c
@@ -360,7 +360,6 @@ archive_write_client_open(struct archive_write_filter *f)
        struct archive_none *state;
        void *buffer;
        size_t buffer_size;
-       int ret;
 
        f->bytes_per_block = archive_write_get_bytes_per_block(f->archive);
        f->bytes_in_last_block =
@@ -385,13 +384,7 @@ archive_write_client_open(struct archive_write_filter *f)
 
        if (a->client_opener == NULL)
                return (ARCHIVE_OK);
-       ret = a->client_opener(f->archive, a->client_data);
-       if (ret != ARCHIVE_OK) {
-               free(state->buffer);
-               free(state);
-               f->data = NULL;
-       }
-       return (ret);
+       return (a->client_opener(f->archive, a->client_data));
 }
 
 static int
@@ -480,6 +473,7 @@ static int
 archive_write_client_free(struct archive_write_filter *f)
 {
        struct archive_write *a = (struct archive_write *)f->archive;
+       struct archive_none *state = (struct archive_none *)f->data;
 
        if (a->client_freer)
                (*a->client_freer)(&a->archive, a->client_data);
@@ -492,6 +486,13 @@ archive_write_client_free(struct archive_write_filter *f)
                a->passphrase = NULL;
        }
 
+       /* Free state. */
+       if (state != NULL) {
+               free(state->buffer);
+               free(state);
+               f->data = NULL;
+       }
+
        return (ARCHIVE_OK);
 }
 
@@ -548,8 +549,6 @@ archive_write_client_close(struct archive_write_filter *f)
        }
        if (a->client_closer)
                (*a->client_closer)(&a->archive, a->client_data);
-       free(state->buffer);
-       free(state);
 
        /* Clear the close handler myself not to be called again. */
        f->state = ARCHIVE_WRITE_FILTER_STATE_CLOSED;
@@ -807,7 +806,10 @@ _archive_write_finish_entry(struct archive *_a)
        if (a->archive.state & ARCHIVE_STATE_DATA
            && a->format_finish_entry != NULL)
                ret = (a->format_finish_entry)(a);
-       a->archive.state = ARCHIVE_STATE_HEADER;
+       if (ret == ARCHIVE_FATAL)
+               a->archive.state = ARCHIVE_STATE_FATAL;
+       else
+               a->archive.state = ARCHIVE_STATE_HEADER;
        return (ret);
 }
 
@@ -819,6 +821,7 @@ _archive_write_data(struct archive *_a, const void *buff, size_t s)
 {
        struct archive_write *a = (struct archive_write *)_a;
        const size_t max_write = INT_MAX;
+       int ret;
 
        archive_check_magic(&a->archive, ARCHIVE_WRITE_MAGIC,
            ARCHIVE_STATE_DATA, "archive_write_data");
@@ -826,7 +829,10 @@ _archive_write_data(struct archive *_a, const void *buff, size_t s)
        if (s > max_write)
                s = max_write;
        archive_clear_error(&a->archive);
-       return ((a->format_write_data)(a, buff, s));
+       ret = (a->format_write_data)(a, buff, s);
+       if (ret == ARCHIVE_FATAL)
+               a->archive.state = ARCHIVE_STATE_FATAL;
+       return (ret);
 }
 
 static struct archive_write_filter *

diff --git a/libarchive/archive_write_add_filter_bzip2.c b/libarchive/archive_write_add_filter_bzip2.c
index 0726f08936ec1d9180f2a5a604488dafb8bd7629..2434528d5133b6ed33f31fdb0fa3e0760131180c 100644 (file)
--- a/libarchive/archive_write_add_filter_bzip2.c
+++ b/libarchive/archive_write_add_filter_bzip2.c
@@ -281,6 +281,10 @@ static int
 archive_compressor_bzip2_free(struct archive_write_filter *f)
 {
        struct private_data *data = (struct private_data *)f->data;
+
+       /* May already have been called, but not necessarily. */
+       (void)BZ2_bzCompressEnd(&(data->stream));
+
        free(data->compressed);
        free(data);
        f->data = NULL;

diff --git a/libarchive/test/test_write_filter_bzip2.c b/libarchive/test/test_write_filter_bzip2.c
index 20ca0d9a7b224d71c4eeea572e2184fb6f815782..7b2e4f857a75cd20530964b18cded72adddcc1ed 100644 (file)
--- a/libarchive/test/test_write_filter_bzip2.c
+++ b/libarchive/test/test_write_filter_bzip2.c
@@ -267,6 +267,35 @@ DEFINE_TEST(test_write_filter_bzip2)
        assertEqualInt(ARCHIVE_OK, archive_write_close(a));
        assertEqualInt(ARCHIVE_OK, archive_write_free(a));
 
+       /*
+        * Test behavior after a fatal error (triggered by giving
+        * archive_write_open_memory() a very small buffer).
+        */
+       if (!use_prog) {
+               used1 = 0;
+               assert((a = archive_write_new()) != NULL);
+               assertEqualIntA(a, ARCHIVE_OK,
+                   archive_write_set_format_ustar(a));
+               assertEqualIntA(a, ARCHIVE_OK,
+                   archive_write_add_filter_bzip2(a));
+               assertEqualIntA(a, ARCHIVE_OK,
+                   archive_write_open_memory(a, buff, 100, &used1));
+               assert((ae = archive_entry_new()) != NULL);
+               archive_entry_set_filetype(ae, AE_IFREG);
+               archive_entry_set_size(ae, 4000000);
+               archive_entry_copy_pathname(ae, "file");
+               assertEqualIntA(a, ARCHIVE_OK,
+                   archive_write_header(a, ae));
+               for (i = 0; i < 1000000; i++) {
+                       r = archive_write_data(a, &i, 4);
+                       if (r == ARCHIVE_FATAL)
+                               break;
+               }
+               assertEqualIntA(a, ARCHIVE_FATAL, r);
+               archive_entry_free(ae);
+               assertEqualInt(ARCHIVE_OK, archive_write_free(a));
+       }
+
        /*
         * Clean up.
         */