Bug 254204 - devel/libtar: update to 1.2.20
Summary: devel/libtar: update to 1.2.20
Status: Closed FIXED
Alias: None
Product: Ports & Packages
Classification: Unclassified
Component: Individual Port(s) (show other bugs)
Version: Latest
Hardware: Any Any
: --- Affects Only Me
Assignee: Yuri Victorovich
URL:
Keywords: buildisok
Depends on:
Blocks:
 
Reported: 2021-03-10 20:10 UTC by Steve Wills
Modified: 2023-04-16 06:40 UTC (History)
3 users (show)

See Also:
bugzilla: maintainer-feedback? (manuelj.munoz)


Attachments
patch to update port (2.84 KB, patch)
2021-03-10 20:10 UTC, Steve Wills
swills: maintainer-approval? (manuelj.munoz)
Details | Diff
Test program (745 bytes, text/plain)
2021-03-24 20:20 UTC, Manuel Muñoz
no flags Details
updated patch (2.83 KB, patch)
2023-01-02 20:07 UTC, Yuri Victorovich
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Steve Wills freebsd_committer freebsd_triage 2021-03-10 20:10:17 UTC
Created attachment 223171 [details]
patch to update port
Comment 1 Automation User 2021-03-10 20:25:59 UTC
Build and package info is available at https://gitlab.com/swills/freebsd-ports/pipelines/268516198
Comment 2 Fernando Apesteguía freebsd_committer freebsd_triage 2021-03-11 08:59:45 UTC
^Triage: Reporter is committer, assign accordingly.

^Triage: If there is a changelog or release notes URL available for this version, please add it to the URL field.

Thanks!
Comment 3 Manuel Muñoz 2021-03-23 15:18:03 UTC
Hello guys,

I am the maintainer, apologies for the delay.
It seems 1.2.20 was out 7 years ago.

I wrote an email in order to introduce myself to Mark Roth to his email account at University of Illinois but the account is gone and he seems to be working at Google, I cannot find his current address. 

So I am right now trying to build libtar from the port (after upgrading to 12.2) using the suggested Makefile.
Comment 4 Manuel Muñoz 2021-03-24 20:20:20 UTC
Created attachment 223551 [details]
Test program
Comment 5 Manuel Muñoz 2021-03-24 20:22:29 UTC
Thanks Steve...

First of all, sorry if I say or do anything stupid, I am pretty new here.

I have replace all Makefile, distinfo, pkg-desc, pkg-plist files inside /usr/ports/devel/libtar, and got 1.2.20 installed (# make install clean).

After that, I have checked by running a small .c program that makes use of libtar.

When running 1.2.20 I get the correct output but a "Core dump (Segmentation fault)" at the end.
Comment 6 Fernando Apesteguía freebsd_committer freebsd_triage 2021-03-30 14:19:20 UTC
Hi Manuel, Steve,

I can confirm the segfault. I had a look and the problem arises trying to free(filename). According to the ChangeLog:

     Fixed thread-safe bug in th_get_pathname() (Sergey Zhitomirsky)

But they are returning the addres of a static alocated array:

char *
th_get_pathname(TAR *t)
{
        static TLS_THREAD char filename[MAXPATHLEN];

        if (t->th_buf.gnu_longname)
                return t->th_buf.gnu_longname;

        if (t->th_buf.prefix[0] != '\0')
        {
                snprintf(filename, sizeof(filename), "%.155s/%.100s",
                         t->th_buf.prefix, t->th_buf.name);
                return filename;
        }

        snprintf(filename, sizeof(filename), "%.100s", t->th_buf.name);
        return filename;
}

So it can not be freed from outside. I added a patch that just strdups the string before returning it so it can be freed. 

With the new version:

fernape@beastie:~/tmp/libtar$ touch one two three
fernape@beastie:~/tmp/libtar$ tar cvf mytar.tar one two three 
a one
a two
a three
fernape@beastie:~/tmp/libtar$ clang -ggdb -Wall -ltar -I /usr/local/include/ -L /usr/local/lib libtar-list.c 
fernape@beastie:~/tmp/libtar$ ./a.out mytar.tar 
one
two
three

Before the change, it segfaulted after "one".

There is this patch from a few days after the last release in 2013: https://github.com/tklauser/libtar/commit/ec613af2e9371d7a3e1f7c7a6822164a4255b4d1

th_get_pathname is used internally in the library assuming it returns a static allocated buffer and they never free filename. So I think that the above patch will trade segfaults for memory leaks.



Let me know what you think.

What surprises me is that I don't see that patch in OpenBSD or other Linux distributions...
Comment 7 Manuel Muñoz 2021-04-02 20:19:13 UTC
Hey Fernando,

Why do you say there could be a memory leak?
I can see in lib/handle.c they have added a "free(t->th_pathname)" statement.

/* close tarfile handle */
int
tar_close(TAR *t)
{
	int i;

	i = (*(t->type->closefunc))(t->fd);

	if (t->h != NULL)
		libtar_hash_free(t->h, ((t->oflags & O_ACCMODE) == O_RDONLY
					? free
					: (libtar_freefunc_t)tar_dev_free));
	free(t->th_pathname);
	free(t);

	return i;
}
Comment 8 Fernando Apesteguía freebsd_committer freebsd_triage 2021-04-04 16:51:02 UTC
(In reply to Manuel Muñoz from comment #7)

I mean that by strduping the string returned from th_get_pathname, the function returns a _different_ string every time. For example, when called from tar_set_file_perms():

filename = (realname ? realname : th_get_pathname(t));

But filename is never freed(). So that string will be lost/leaked. The code as it is, assumes that the string returned by th_get_pathname is always the same. Similar problems are encountered in functions like tar_extract_regfile().

I would need to run a program through valgrind to confirm this.
Comment 9 Yuri Victorovich freebsd_committer freebsd_triage 2023-01-02 20:07:10 UTC
Created attachment 239217 [details]
updated patch

Updated patch uses the GitHub project.
Comment 10 Yuri Victorovich freebsd_committer freebsd_triage 2023-01-02 20:10:05 UTC
Testcase doesn't SEGV now.

manuelj.munoz@gmail.com, can this be committed?
Comment 11 Rene Ladan freebsd_committer freebsd_triage 2023-04-11 20:20:27 UTC
Maintainer reset.
Comment 12 Fernando Apesteguía freebsd_committer freebsd_triage 2023-04-12 15:23:19 UTC
(In reply to Yuri Victorovich from comment #9)
Yuri, can you go and update this one?
Comment 13 Yuri Victorovich freebsd_committer freebsd_triage 2023-04-12 16:57:34 UTC
Ok.
Comment 14 Yuri Victorovich freebsd_committer freebsd_triage 2023-04-16 04:58:26 UTC
Committed.
Comment 15 commit-hook freebsd_committer freebsd_triage 2023-04-16 04:59:09 UTC
A commit in branch main references this bug:

URL: https://cgit.FreeBSD.org/ports/commit/?id=e3e6da1c73a0a4ccacec18b43df6e0700601bb68

commit e3e6da1c73a0a4ccacec18b43df6e0700601bb68
Author:     Yuri Victorovich <yuri@FreeBSD.org>
AuthorDate: 2023-04-16 04:56:02 +0000
Commit:     Yuri Victorovich <yuri@FreeBSD.org>
CommitDate: 2023-04-16 04:58:09 +0000

    devel/libtar: Update 1.2.11 → 1.2.20

    PR:     254204
    Approved by:    manuelj.munoz@gmail.com (maintainer's timeout; 2 years 1 months+)

 devel/libtar/Makefile                           | 23 +++++++++++++++--------
 devel/libtar/distinfo                           |  5 +++--
 devel/libtar/files/patch-lib_Makefile.in (gone) | 25 -------------------------
 devel/libtar/pkg-plist                          |  1 +
 devel/ticcutils/Makefile                        |  2 +-
 5 files changed, 20 insertions(+), 36 deletions(-)
Comment 16 Fernando Apesteguía freebsd_committer freebsd_triage 2023-04-16 06:40:11 UTC
Thanks!