[Pkg-freeipmi-devel] Bug#690040: freeipmi: Build with PIE, bindnow, openfiles with O_EXCL and check return status
Albert Chu
chu11 at llnl.gov
Tue Oct 9 18:20:34 UTC 2012
Hey Yaroslav,
comments inlined below
On Tue, 2012-10-09 at 08:58 -0400, Yaroslav Halchenko wrote:
> Hi Dave,
>
> Thanks for forwarding! I wonder if you have upstreamed/discussed
> O_EXCL patch with upstream (CCing upstream to expedite in case if
> not) -- sounds sensible to me (isn't it Albert? see patch below. quoting
> entire message for completeness)
>
> Cheers
>
> On Tue, 09 Oct 2012, Dave Walker (Daviey) wrote:
>
> > Package: freeipmi
> > Version: 1.1.5-3
> > Severity: normal
> > Tags: patch
> > User: ubuntu-devel at lists.ubuntu.com
> > Usertags: origin-ubuntu quantal ubuntu-patch
>
>
>
>
> > In Ubuntu, the attached patch was applied to achieve the following:
>
>
> > * debian/rules: Build with "-pie,-bindnow"
> > * debian/patches/0002_excel_when_opening_tmp.patch: Open files with O_EXCL.
I'm confused by this requirement. Why should it be an error if the file
already exists?
The default location for this library's debug dumps is /tmp. I
admittedly chose it somewhat at random, it just felt like a decent
location. Is there a better place? Perhaps the current working
directory would be more appropriate?
> > * debian/patches/fix-Wunused-result.patch: Resolve
> -Wunused-result's
> > warnings, by checking for non-0 return.
Seems fine. The upstream location to fix this is in
common/toolcommon/tool-daemon-common.c (since there's a common code for
this now). I can fix this upstream.
Al
> > I'm not sure fix-Wunused-result.patch adds any value to the latest experimental package.
>
> > Thanks for considering the patch.
>
>
> > -- System Information:
> > Debian Release: wheezy/sid
> > APT prefers quantal-updates
> > APT policy: (500, 'quantal-updates'), (500, 'quantal-security'), (500, 'quantal')
> > Architecture: amd64 (x86_64)
> > Foreign Architectures: i386
>
> > Kernel: Linux 3.5.0-10-generic (SMP w/2 CPU cores)
> > Locale: LANG=en_GB.UTF-8, LC_CTYPE=en_GB.UTF-8 (charmap=UTF-8)
> > Shell: /bin/sh linked to /bin/dash
>
> > diff -Nru freeipmi-1.1.5/debian/changelog freeipmi-1.1.5/debian/changelog
> > diff -Nru freeipmi-1.1.5/debian/patches/0002_excel_when_opening_tmp.patch freeipmi-1.1.5/debian/patches/0002_excel_when_opening_tmp.patch
> > --- freeipmi-1.1.5/debian/patches/0002_excel_when_opening_tmp.patch 1970-01-01 01:00:00.000000000 +0100
> > +++ freeipmi-1.1.5/debian/patches/0002_excel_when_opening_tmp.patch 2012-10-07 20:17:11.000000000 +0100
> > @@ -0,0 +1,26 @@
> > +Index: freeipmi-1.1.5/libipmiconsole/ipmiconsole_debug.c
> > +===================================================================
> > +--- freeipmi-1.1.5.orig/libipmiconsole/ipmiconsole_debug.c 2012-05-17 15:08:55.000000000 -0400
> > ++++ freeipmi-1.1.5/libipmiconsole/ipmiconsole_debug.c 2012-10-02 10:32:04.755269452 -0400
> > +@@ -84,7 +84,7 @@
> > + IPMICONSOLE_DEBUG_DIRECTORY,
> > + IPMICONSOLE_DEBUG_FILENAME);
> > +
> > +- if ((console_debug_fd = open (filename, O_CREAT | O_APPEND | O_WRONLY, 0600)) < 0)
> > ++ if ((console_debug_fd = open (filename, O_CREAT | O_APPEND | O_WRONLY | O_EXCL, 0600)) < 0)
> > + {
> > + console_debug_flags &= ~IPMICONSOLE_DEBUG_FILE;
> > + IPMICONSOLE_DEBUG (("open: %s", strerror (errno)));
> > +Index: freeipmi-1.1.5/libipmiconsole/ipmiconsole_ctx.c
> > +===================================================================
> > +--- freeipmi-1.1.5.orig/libipmiconsole/ipmiconsole_ctx.c 2012-05-17 15:08:55.000000000 -0400
> > ++++ freeipmi-1.1.5/libipmiconsole/ipmiconsole_ctx.c 2012-10-02 10:34:50.559273698 -0400
> > +@@ -351,7 +351,7 @@
> > + c->config.hostname);
> > +
> > + if ((c->debug.debug_fd = open (filename,
> > +- O_CREAT | O_APPEND | O_WRONLY,
> > ++ O_CREAT | O_APPEND | O_WRONLY | O_EXCL,
> > + 0600)) < 0)
> > + {
> > + c->config.debug_flags &= ~IPMICONSOLE_DEBUG_FILE;
> > diff -Nru freeipmi-1.1.5/debian/patches/fix-Wunused-result.patch freeipmi-1.1.5/debian/patches/fix-Wunused-result.patch
> > --- freeipmi-1.1.5/debian/patches/fix-Wunused-result.patch 1970-01-01 01:00:00.000000000 +0100
> > +++ freeipmi-1.1.5/debian/patches/fix-Wunused-result.patch 2012-10-09 12:27:26.000000000 +0100
> > @@ -0,0 +1,54 @@
> > +Description: Resolve -Wunused-result's warnings, by checking for non-0 return.
> > + Patch not upstreamed, as trunk has refactored this case out.
> > +Author: Dave Walker (Daviey) <DaveWalker at ubuntu.com>
> > +Forwarded: not-needed
> > +
> > +--- a/bmc-watchdog/bmc-watchdog.c
> > ++++ b/bmc-watchdog/bmc-watchdog.c
> > +@@ -1692,7 +1692,8 @@
> > + {
> > + /* parent terminates */
> > + char buf;
> > +- read(fds[0], &buf, 1);
> > ++ if (read(fds[0], &buf, 1) < 0)
> > ++ _err_exit ("read: %s", strerror (errno));
> > + close(fds[1]);
> > + close(fds[0]);
> > + exit (0);
> > +@@ -1718,7 +1719,8 @@
> > +
> > + umask (0);
> > +
> > +- write(fds[1], "a", 1);
> > ++ if (write(fds[1], "a", 1) < 0)
> > ++ _err_exit ("write: %s", strerror (errno));
> > + close(fds[1]);
> > + close(fds[0]);
> > + for (i = 0; i < 64; i++)
> > +--- a/ipmidetectd/ipmidetectd.c
> > ++++ b/ipmidetectd/ipmidetectd.c
> > +@@ -69,7 +69,8 @@
> > + {
> > + /* Terminate Parent */
> > + char buf;
> > +- read(fds[0], &buf, 1);
> > ++ if (read(fds[0], &buf, 1) < 0)
> > ++ IPMIDETECTD_EXIT (("read: %s", strerror (errno)));
> > + close(fds[1]);
> > + close(fds[0]);
> > + exit (0);
> > +@@ -86,10 +87,12 @@
> > + if (pid != 0) /* Terminate 1st Child */
> > + exit (0);
> > +
> > +- chdir ("/");
> > ++ if (chdir ("/") < 0)
> > ++ IPMIDETECTD_EXIT (("chdir: %s", strerror (errno)));
> > +
> > + umask (0);
> > +- write(fds[1], "a", 1);
> > ++ if (write(fds[1], "a", 1) < 0)
> > ++ IPMIDETECTD_EXIT (("write: %s", strerror (errno)));
> > + close(fds[1]);
> > + close(fds[0]);
> > +
> > diff -Nru freeipmi-1.1.5/debian/patches/series freeipmi-1.1.5/debian/patches/series
> > --- freeipmi-1.1.5/debian/patches/series 2012-06-15 02:41:57.000000000 +0100
> > +++ freeipmi-1.1.5/debian/patches/series 2012-10-07 22:17:30.000000000 +0100
> > @@ -1,3 +1,5 @@
> > up_fixmanpages
> > deb_bmc-watchdog_noRUN
> > 0001-Fix-Wformat-security-warnings.patch
> > +0002_excel_when_opening_tmp.patch
> > +fix-Wunused-result.patch
> > diff -Nru freeipmi-1.1.5/debian/rules freeipmi-1.1.5/debian/rules
> > --- freeipmi-1.1.5/debian/rules 2012-06-15 02:41:57.000000000 +0100
> > +++ freeipmi-1.1.5/debian/rules 2012-10-07 20:17:11.000000000 +0100
> > @@ -4,6 +4,8 @@
> > # We use some bashisms
> > SHELL=/bin/bash
>
> > +export DEB_BUILD_MAINT_OPTIONS=hardening=+pie,+bindnow
> > +
> > # mega rule -- Joey knows how to do the rest
> > %:
> > dh $@ --with autotools_dev
>
> > _______________________________________________
> > Pkg-freeipmi-devel mailing list
> > Pkg-freeipmi-devel at lists.alioth.debian.org
> > http://lists.alioth.debian.org/mailman/listinfo/pkg-freeipmi-devel
>
>
--
Albert Chu
chu11 at llnl.gov
Computer Scientist
High Performance Systems Division
Lawrence Livermore National Laboratory
More information about the Pkg-freeipmi-devel
mailing list