Bug 1335812 - tmux: can't create socket after upgrade
Summary: tmux: can't create socket after upgrade
Keywords:
Status: CLOSED WONTFIX
Alias: None
Product: Red Hat Enterprise Linux 7
Classification: Red Hat
Component: tmux
Version: 7.2
Hardware: Unspecified
OS: Unspecified
high
urgent
Target Milestone: rc
: ---
Assignee: David Cantrell
QA Contact: BaseOS QE - Apps
URL:
Whiteboard:
: 1336291 (view as bug list)
Depends On:
Blocks: 1256306
TreeView+ depends on / blocked
 
Reported: 2016-05-13 09:21 UTC by Dalibor Pospíšil
Modified: 2017-02-09 08:56 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2016-05-23 14:25:53 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)

Description Dalibor Pospíšil 2016-05-13 09:21:27 UTC
Description of problem:
cannot start tmux
This is regression against tmux-1.8-4.el7

Version-Release number of selected component (if applicable):
tmux-1.8-6.el7

How reproducible:
always

Steps to Reproduce:
# tmux
can't create socket

Comment 1 David Cantrell 2016-05-16 13:08:39 UTC
I think this is because the -6 build was missing the tmpfiles.d config file, so tmux didn't have /var/run/tmux to write to.  That's been fixed in the most recent build.  Can you try tmux-1.8-8.el7?

Comment 2 David Cantrell 2016-05-16 13:10:17 UTC
*** Bug 1336291 has been marked as a duplicate of this bug. ***

Comment 3 Michal Sekletar 2016-05-17 12:00:49 UTC
(In reply to David Cantrell from comment #1)
> I think this is because the -6 build was missing the tmpfiles.d config file,
> so tmux didn't have /var/run/tmux to write to.  That's been fixed in the
> most recent build.  Can you try tmux-1.8-8.el7?

Note, that having tmpfiles snippet in the package doesn't mean that the path mentioned in that snippet is usable after package installation. 

From Fedora packaging guidelines,

"In the spec file, the packager needs to install the tmpfiles.d conf file into the %{_tmpfilesdir} directory and also make sure the directory is included in the rpm."

Comment 4 Michal Sekletar 2016-05-17 12:01:42 UTC
https://fedoraproject.org/wiki/Packaging:Tmpfiles.d

Comment 5 Martin Banas 2016-05-17 12:10:07 UTC
David,
since this is not created automatically, installations still fails. Marking this as TestBlocker.

Comment 7 David Cantrell 2016-05-17 13:00:11 UTC
I have not really used the tmpfiles.d stuff, so here's what I have in the tmpfiles.d tmux.conf file:

d /run/tmux 1777 root root -

I've also patched the source to use /run rather than /var/run since that seems to be the norm these days:

diff -up tmux-1.8/compat.h.orig tmux-1.8/compat.h
--- tmux-1.8/compat.h.orig	2016-05-17 08:53:49.338644507 -0400
+++ tmux-1.8/compat.h	2016-05-17 08:54:48.712245806 -0400
@@ -39,7 +39,7 @@ typedef uint64_t u_int64_t;
 
 #ifndef HAVE_PATHS_H
 #define	_PATH_BSHELL	"/bin/sh"
-#define _PATH_VARRUN	"/var/run/"
+#define _PATH_VARRUN	"/run/"
 #define _PATH_DEVNULL	"/dev/null"
 #define _PATH_TTY	"/dev/tty"
 #define _PATH_DEV	"/dev/"

And finally the changes to the spec file:

diff --git a/tmux.spec b/tmux.spec
index 33df591..92a1eec 100644
--- a/tmux.spec
+++ b/tmux.spec
@@ -1,6 +1,6 @@
 Name:           tmux
 Version:        1.8
-Release:        8%{?dist}
+Release:        9%{?dist}
 Summary:        A terminal multiplexer
 
 Group:          Applications/System
@@ -9,11 +9,12 @@ Group:          Applications/System
 License:        ISC and BSD
 URL:            http://sourceforge.net/projects/tmux
 Source0:        http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.gz
-Source1:        tmux.conf
+Source1:        tmux-tmpfiles.d.conf
 
 Patch0:         tmux-1.8-bz1036136.patch
 Patch1:         tmux-1.8-CMSG_DATA.patch
 Patch2:         tmux-1.8-bzero.patch
+Patch3:         tmux-1.8-varrun.patch
 
 BuildRequires:  ncurses-devel
 BuildRequires:  libevent-devel
@@ -38,6 +39,9 @@ as GNU Screen.
 # bzero() is provided
 %patch2 -p1
 
+# Sockets and pid files to /run
+%patch3 -p1
+
 %build
 %configure
 make %{?_smp_mflags} LDFLAGS="%{optflags}"
@@ -45,7 +49,10 @@ make %{?_smp_mflags} LDFLAGS="%{optflags}"
 %install
 rm -rf %{buildroot}
 make install DESTDIR=%{buildroot} INSTALLBIN="install -p -m 755" INSTALLMAN="install -p -m 644"
-install -D -m 0644 %{SOURCE1} %{buildroot}%{_prefix}/lib/tmpfiles.d/tmux.conf
+install -D -m 0644 %{SOURCE1} %{buildroot}%{_tmpfilesdir}/%{name}.conf
+mkdir -p %{buildroot}/run
+install -d -m 0755 %{buildroot}/run/%{name}/
+install -m 0644 %{buildroot}/run/%{name}.pid
 
 %post
 if [ ! -f %{_sysconfdir}/shells ] ; then
@@ -65,6 +72,9 @@ fi
 %{_bindir}/tmux
 %{_mandir}/man1/tmux.1.*
 %{_prefix}/lib/tmpfiles.d/tmux.conf
+%dir /run/%{name}
+%verify(not size mtime md5) /run/%{name}.pid
+%{_tmpfilesdir}/%{name}.conf
 
 %changelog
 * Fri May 13 2016 David Cantrell <dcantrell> - 1.8-8


msekleta, can you let me know if these changes are correct for the tmpfiles.d usage?  If so, I'll get a new build of tmux going.

Comment 8 Michal Sekletar 2016-05-17 13:44:20 UTC
(In reply to David Cantrell from comment #7)
> I have not really used the tmpfiles.d stuff, so here's what I have in the
> tmpfiles.d tmux.conf file:
> 
> d /run/tmux 1777 root root -
> 
> I've also patched the source to use /run rather than /var/run since that
> seems to be the norm these days:
> 

Yeah, using /run is preferred these days as /var/run is only compat symlink.

Note that if directory  is created using 'd' then tmpfiles.d clean up logic applies to this path. By default, i.e. if Age paramater is not specified, all files which were not touched in more than 10 days are removed.

In order to prevent that you need to put following line into your tmpfiles.d snippet

x /run/tmux

But if only file tmux puts there is unix socket then you to need that because systemd-tmpfiles skips live (listed in /proc/net/unix) unix sockets on cleanup.


> diff -up tmux-1.8/compat.h.orig tmux-1.8/compat.h
> --- tmux-1.8/compat.h.orig	2016-05-17 08:53:49.338644507 -0400
> +++ tmux-1.8/compat.h	2016-05-17 08:54:48.712245806 -0400
> @@ -39,7 +39,7 @@ typedef uint64_t u_int64_t;
>  
>  #ifndef HAVE_PATHS_H
>  #define	_PATH_BSHELL	"/bin/sh"
> -#define _PATH_VARRUN	"/var/run/"
> +#define _PATH_VARRUN	"/run/"
>  #define _PATH_DEVNULL	"/dev/null"
>  #define _PATH_TTY	"/dev/tty"
>  #define _PATH_DEV	"/dev/"
> 
> And finally the changes to the spec file:
> 
> diff --git a/tmux.spec b/tmux.spec
> index 33df591..92a1eec 100644
> --- a/tmux.spec
> +++ b/tmux.spec
> @@ -1,6 +1,6 @@
>  Name:           tmux
>  Version:        1.8
> -Release:        8%{?dist}
> +Release:        9%{?dist}
>  Summary:        A terminal multiplexer
>  
>  Group:          Applications/System
> @@ -9,11 +9,12 @@ Group:          Applications/System
>  License:        ISC and BSD
>  URL:            http://sourceforge.net/projects/tmux
>  Source0:       
> http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.gz
> -Source1:        tmux.conf
> +Source1:        tmux-tmpfiles.d.conf
>  
>  Patch0:         tmux-1.8-bz1036136.patch
>  Patch1:         tmux-1.8-CMSG_DATA.patch
>  Patch2:         tmux-1.8-bzero.patch
> +Patch3:         tmux-1.8-varrun.patch
>  
>  BuildRequires:  ncurses-devel
>  BuildRequires:  libevent-devel
> @@ -38,6 +39,9 @@ as GNU Screen.
>  # bzero() is provided
>  %patch2 -p1
>  
> +# Sockets and pid files to /run
> +%patch3 -p1
> +
>  %build
>  %configure
>  make %{?_smp_mflags} LDFLAGS="%{optflags}"
> @@ -45,7 +49,10 @@ make %{?_smp_mflags} LDFLAGS="%{optflags}"
>  %install
>  rm -rf %{buildroot}
>  make install DESTDIR=%{buildroot} INSTALLBIN="install -p -m 755"
> INSTALLMAN="install -p -m 644"
> -install -D -m 0644 %{SOURCE1}
> %{buildroot}%{_prefix}/lib/tmpfiles.d/tmux.conf
> +install -D -m 0644 %{SOURCE1} %{buildroot}%{_tmpfilesdir}/%{name}.conf
> +mkdir -p %{buildroot}/run
> +install -d -m 0755 %{buildroot}/run/%{name}/
> +install -m 0644 %{buildroot}/run/%{name}.pid

This line *seems* redundant, because from quick grep of tmux source code it looks like that pid file is not written anywhere, hence no need to reference it in rpm. Also I guess there can be more tmux servers running at the same time for different users so single pid file doesn't even make sense in that case. Not 100% sure on this one.

>  %post
>  if [ ! -f %{_sysconfdir}/shells ] ; then
> @@ -65,6 +72,9 @@ fi
>  %{_bindir}/tmux
>  %{_mandir}/man1/tmux.1.*
>  %{_prefix}/lib/tmpfiles.d/tmux.conf
> +%dir /run/%{name}
> +%verify(not size mtime md5) /run/%{name}.pid

If my assumption is true then above line is redundant too.

> +%{_tmpfilesdir}/%{name}.conf
>  
>  %changelog
>  * Fri May 13 2016 David Cantrell <dcantrell> - 1.8-8
> 
> 
> msekleta, can you let me know if these changes are correct for the
> tmpfiles.d usage?  If so, I'll get a new build of tmux going.

Looking at this patch I can't resist the feeling that this change breaks tmux for non privileged user. Probably for normal users their tmux socket should be created under user's XDG_RUNTIME_DIR. At any rate...Before you push, please double check that with the patch applied tmux is still usable for both root and normal users.

Comment 9 David Cantrell 2016-05-17 19:28:13 UTC
(In reply to Michal Sekletar from comment #8)
> (In reply to David Cantrell from comment #7)
> > I have not really used the tmpfiles.d stuff, so here's what I have in the
> > tmpfiles.d tmux.conf file:
> > 
> > d /run/tmux 1777 root root -
> > 
> > I've also patched the source to use /run rather than /var/run since that
> > seems to be the norm these days:
> > 
> 
> Yeah, using /run is preferred these days as /var/run is only compat symlink.
> 
> Note that if directory  is created using 'd' then tmpfiles.d clean up logic
> applies to this path. By default, i.e. if Age paramater is not specified,
> all files which were not touched in more than 10 days are removed.

I will stick with using /run

> In order to prevent that you need to put following line into your tmpfiles.d
> snippet
> 
> x /run/tmux
> 
> But if only file tmux puts there is unix socket then you to need that
> because systemd-tmpfiles skips live (listed in /proc/net/unix) unix sockets
> on cleanup.

Is "x /run/tmux" the entirety of the additional tmpfiles.d line?  So the tmpfiles.d file would look like:

    d /run/tmux 1777 root root -
    x /run/tmux

?

> > diff -up tmux-1.8/compat.h.orig tmux-1.8/compat.h
> > --- tmux-1.8/compat.h.orig	2016-05-17 08:53:49.338644507 -0400
> > +++ tmux-1.8/compat.h	2016-05-17 08:54:48.712245806 -0400
> > @@ -39,7 +39,7 @@ typedef uint64_t u_int64_t;
> >  
> >  #ifndef HAVE_PATHS_H
> >  #define	_PATH_BSHELL	"/bin/sh"
> > -#define _PATH_VARRUN	"/var/run/"
> > +#define _PATH_VARRUN	"/run/"
> >  #define _PATH_DEVNULL	"/dev/null"
> >  #define _PATH_TTY	"/dev/tty"
> >  #define _PATH_DEV	"/dev/"
> > 
> > And finally the changes to the spec file:
> > 
> > diff --git a/tmux.spec b/tmux.spec
> > index 33df591..92a1eec 100644
> > --- a/tmux.spec
> > +++ b/tmux.spec
> > @@ -1,6 +1,6 @@
> >  Name:           tmux
> >  Version:        1.8
> > -Release:        8%{?dist}
> > +Release:        9%{?dist}
> >  Summary:        A terminal multiplexer
> >  
> >  Group:          Applications/System
> > @@ -9,11 +9,12 @@ Group:          Applications/System
> >  License:        ISC and BSD
> >  URL:            http://sourceforge.net/projects/tmux
> >  Source0:       
> > http://downloads.sourceforge.net/%{name}/%{name}-%{version}.tar.gz
> > -Source1:        tmux.conf
> > +Source1:        tmux-tmpfiles.d.conf
> >  
> >  Patch0:         tmux-1.8-bz1036136.patch
> >  Patch1:         tmux-1.8-CMSG_DATA.patch
> >  Patch2:         tmux-1.8-bzero.patch
> > +Patch3:         tmux-1.8-varrun.patch
> >  
> >  BuildRequires:  ncurses-devel
> >  BuildRequires:  libevent-devel
> > @@ -38,6 +39,9 @@ as GNU Screen.
> >  # bzero() is provided
> >  %patch2 -p1
> >  
> > +# Sockets and pid files to /run
> > +%patch3 -p1
> > +
> >  %build
> >  %configure
> >  make %{?_smp_mflags} LDFLAGS="%{optflags}"
> > @@ -45,7 +49,10 @@ make %{?_smp_mflags} LDFLAGS="%{optflags}"
> >  %install
> >  rm -rf %{buildroot}
> >  make install DESTDIR=%{buildroot} INSTALLBIN="install -p -m 755"
> > INSTALLMAN="install -p -m 644"
> > -install -D -m 0644 %{SOURCE1}
> > %{buildroot}%{_prefix}/lib/tmpfiles.d/tmux.conf
> > +install -D -m 0644 %{SOURCE1} %{buildroot}%{_tmpfilesdir}/%{name}.conf
> > +mkdir -p %{buildroot}/run
> > +install -d -m 0755 %{buildroot}/run/%{name}/
> > +install -m 0644 %{buildroot}/run/%{name}.pid
> 
> This line *seems* redundant, because from quick grep of tmux source code it
> looks like that pid file is not written anywhere, hence no need to reference
> it in rpm. Also I guess there can be more tmux servers running at the same
> time for different users so single pid file doesn't even make sense in that
> case. Not 100% sure on this one.

It's not necessary for tmux to run, no.  I was following the guidelines and didn't know if there needed to be a placeholder pid file if something (systemd?) somehow would use it.  Dunno, did not bother to look that up.

I will remove the pid line.

> >  %post
> >  if [ ! -f %{_sysconfdir}/shells ] ; then
> > @@ -65,6 +72,9 @@ fi
> >  %{_bindir}/tmux
> >  %{_mandir}/man1/tmux.1.*
> >  %{_prefix}/lib/tmpfiles.d/tmux.conf
> > +%dir /run/%{name}
> > +%verify(not size mtime md5) /run/%{name}.pid
> 
> If my assumption is true then above line is redundant too.

Removed.

> > +%{_tmpfilesdir}/%{name}.conf
> >  
> >  %changelog
> >  * Fri May 13 2016 David Cantrell <dcantrell> - 1.8-8
> > 
> > 
> > msekleta, can you let me know if these changes are correct for the
> > tmpfiles.d usage?  If so, I'll get a new build of tmux going.
> 
> Looking at this patch I can't resist the feeling that this change breaks
> tmux for non privileged user. Probably for normal users their tmux socket
> should be created under user's XDG_RUNTIME_DIR. At any rate...Before you
> push, please double check that with the patch applied tmux is still usable
> for both root and normal users.

It shouldn't.  But of course I will check it out before updating the errata.

Thanks.

Comment 10 David Cantrell 2016-05-17 19:29:54 UTC
Can we get a qa_ack+ on this?

Comment 11 Michal Sekletar 2016-05-17 20:48:42 UTC
(In reply to David Cantrell from comment #9)
> Is "x /run/tmux" the entirety of the additional tmpfiles.d line?  So the
> tmpfiles.d file would look like:
> 
>     d /run/tmux 1777 root root -
>     x /run/tmux
> 
> ?

Meh. I just looked up the man page once more just to be sure and of course it is exactly the opposite to I've said before. Omitting the Age parameter or setting it to "-" disables cleanup logic for the path, i.e. no second "x" line is necessary in this case. Sorry for the noise.

Comment 12 David Cantrell 2016-05-18 16:48:45 UTC
I've dropped the 'x /run/tmux' line.  The -9 build of the package works on 7.3 nightly.  However, the package creates /run/tmux with 0755 permissions and that does fail for users other than root.  Permissions of 1777 work fine, obviously.  This change in the spec:

-install -d -m 0755 %{buildroot}/run/%{name}/
+install -d -m 1777 %{buildroot}/run/%{name}/

The question I have is does this feel sufficient?  Really, the use of /run/tmux here is really no different than what tmux had been doing using /tmp, with the exception of /run being under the control of systemd and effectively being reaped periodically.  Should /run/tmux be owned by a tmux group, for example, and carry 1770 permissions and have /usr/bin/tmux installed setgid to tmux?  Thoughts?

Comment 13 Michal Sekletar 2016-05-18 20:28:02 UTC
(In reply to David Cantrell from comment #12)
> I've dropped the 'x /run/tmux' line.  The -9 build of the package works on
> 7.3 nightly.  However, the package creates /run/tmux with 0755 permissions
> and that does fail for users other than root.  Permissions of 1777 work
> fine, obviously.  This change in the spec:
> 
> -install -d -m 0755 %{buildroot}/run/%{name}/
> +install -d -m 1777 %{buildroot}/run/%{name}/

1777 is no different from using /tmp.

> 
> The question I have is does this feel sufficient?  Really, the use of
> /run/tmux here is really no different than what tmux had been doing using
> /tmp, with the exception of /run being under the control of systemd and
> effectively being reaped periodically.  Should /run/tmux be owned by a tmux
> group, for example, and carry 1770 permissions and have /usr/bin/tmux
> installed setgid to tmux?  Thoughts?

IIRC screen is doing the same thing, I mean root:screen on /run/screen and /usr/bin/screen being setgid. I think tmux can do too. But I guess you should ping some security people about this. Most likely you will need to do that anyway, because rpmdiff probably doesn't allow introducing new setgid binary without waiver from security team.

Comment 14 David Cantrell 2016-05-19 16:51:31 UTC
Can someone in QE please provide a qa_ack+ on this so that I can actually build changes in brew?  Fix for this is queued, just need the ack to push.

Comment 15 Dalibor Pospíšil 2016-05-20 14:03:42 UTC
(In reply to David Cantrell from comment #1)
> I think this is because the -6 build was missing the tmpfiles.d config file,
> so tmux didn't have /var/run/tmux to write to.  That's been fixed in the
> most recent build.  Can you try tmux-1.8-8.el7?

This version does _NOT_ fix the problem.

Comment 16 David Cantrell 2016-05-20 18:32:45 UTC
(In reply to Dalibor Pospíšil from comment #15)
> (In reply to David Cantrell from comment #1)
> > I think this is because the -6 build was missing the tmpfiles.d config file,
> > so tmux didn't have /var/run/tmux to write to.  That's been fixed in the
> > most recent build.  Can you try tmux-1.8-8.el7?
> 
> This version does _NOT_ fix the problem.

Yep, well past that build.  The latest scratch build I did (-10) implements the setgid model for tmux's sockets in /run/tmux, but this was something that was implemented in other distributions and dropped due to opening it up to other security problems.  Still investigating, but at this point, I'm not sure anything needs to change in tmux and both and the other bug can close out as WONTFIX.

Comment 19 David Cantrell 2016-05-23 14:25:53 UTC
Following upstream and other major distributions and not installing tmux as setgid.


Note You need to log in before you can comment on or make changes to this bug.