Hi Adam

Sorry for the delay. Here is the debdiff you have asked for.
It is a simple backport of the change done in upstream GIT.

Best regards,

// Ola

On Sun, Dec 13, 2009 at 03:21:59PM +0000, Adam D. Barratt wrote:
> [CC list trimmed]
> 
> Hi,
> 
> On Sun, 2009-11-29 at 12:30 +0100, Ola Lundqvist wrote:
> > > A) 
> > > 
> > > * Fix in stable 
> > > (This will probably not break any existing configurations, it will just 
> > > keep links that were removed before)
> > > 
> > > Pro: would help fast
> > > Pro: no side effetcs
> > > Con: does it break anything ? 
> > 
> > Would be a good thing if stable release team accept this kind of update.
> > Added:
> > Con: maybe side effects with 3.0.22 ? Likely not. Just as below.
> 
> Please could you provide a debdiff for your proposed change?
> 
> Regards,
> 
> Adam
> 
> 
> 

-- 
 --------------------- Ola Lundqvist ---------------------------
/  o...@debian.org                     Annebergsslingan 37      \
|  o...@inguza.com                      654 65 KARLSTAD          |
|  http://inguza.com/                  +46 (0)70-332 1551       |
\  gpg/f.p.: 7090 A92B 18FE 7994 0C36  4FE4 18A1 B1CF 0FE5 3DD9 /
 ---------------------------------------------------------------
diff -u vzctl-3.0.22/debian/changelog vzctl-3.0.22/debian/changelog
--- vzctl-3.0.22/debian/changelog
+++ vzctl-3.0.22/debian/changelog
@@ -1,3 +1,11 @@
+vzctl (3.0.22-14lenny1) unstable; urgency=low
+
+  * Correction for destroyed symlink for configuration file backported from
+    upstream GIT. This solves a potential data corruption issue as described
+    in the bug report. Closes: #557785.
+
+ -- Ola Lundqvist <o...@debian.org>  Sun, 20 Dec 2009 19:33:36 +0100
+
 vzctl (3.0.22-14) unstable; urgency=medium
 
   * Correction to make it work with ipv6, closes: #505792. Thanks to
only in patch2:
unchanged:
--- vzctl-3.0.22.orig/src/lib/config.c
+++ vzctl-3.0.22/src/lib/config.c
@@ -15,6 +15,9 @@
  *  along with this program; if not, write to the Free Software
  *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
  */
+#ifndef _GNU_SOURCE
+#define _GNU_SOURCE
+#endif
 #include <stdlib.h>
 #include <string.h>
 #include <ctype.h>
@@ -2166,43 +2169,56 @@
 
 static int write_conf(char *fname, list_head_t *head)
 {
-       char buf[STR_SIZE];
+       char *tmpfile, *file;
        conf_struct *conf;
-       int fd = 2;
-       int len, ret;
-
-       if (fname != NULL) {
-               snprintf(buf, sizeof(buf), "%s.tmp", fname);
-               if ((fd = open(buf, O_CREAT|O_WRONLY|O_TRUNC, 0644)) < 0) {
-                       logger(-1, errno, "Unable to create configuration"
-                               " file %s", buf);
+       FILE * fp;
+       int ret = 1;
+       const char *suffix = ".tmp";
+       char *fmt;
+
+       file = canonicalize_file_name(fname);
+       if (file == NULL) {
+               if (errno != ENOENT)
+               {
+                       logger(-1, errno, "Unable to resolve path %s", fname);
                        return 1;
                }
+               file = strdup(fname);
+       }
+       tmpfile = malloc(strlen(file) + strlen(suffix) + 1);
+       sprintf(tmpfile, "%s%s", file, suffix);
+       if ((fp = fopen(tmpfile, "w")) == NULL) {
+               logger(-1, errno, "Unable to create configuration"
+                       " file %s", tmpfile);
+               goto out;
        }
        list_for_each(conf, head, list) {
                if (conf->val == NULL)
                        continue;
-               len = strlen(conf->val);
-               ret = write(fd, conf->val, len);
-               if (ret < 0) {
-                       logger(-1, errno, "Unable to write %d bytes to %s",
-                                       len, buf);
-                       unlink(buf);
-                       close(fd);
-                       return 1;
-               }
                if (strchr(conf->val, '\n') == NULL)
-                       write(fd, "\n", 1);
-       }
-       if (fname != NULL) {
-               close(fd);
-               if (rename(buf, fname)) {
-                       logger(-1, errno, "Unable to move %s -> %s",
-                               buf, fname);
-                       return 1;
+                       fmt="%s\n";
+               else
+                       fmt="%s";
+               if (fprintf(fp, fmt, conf->val) < 0) {
+                       logger(-1, errno, "Error writing to %s",
+                                       tmpfile);
+                       fclose(fp);
+                       goto out2;
                }
        }
-       return 0;
+       fclose(fp);
+       if (rename(tmpfile, file)) {
+               logger(-1, errno, "Unable to move %s -> %s",
+                       tmpfile, file );
+               goto out2;
+       }
+       ret = 0;
+out2:
+       unlink(tmpfile);
+out:
+       free(tmpfile);
+       free(file);
+       return ret;
 }
 
 static int vps_merge_conf(list_head_t *dst, list_head_t *src)
only in patch2:
unchanged:
--- vzctl-3.0.22.orig/debian/patches/config-symlink-corr.patch
+++ vzctl-3.0.22/debian/patches/config-symlink-corr.patch
@@ -0,0 +1,133 @@
+From: Kir Kolyshkin <k...@openvz.org>
+Date: Fri, 16 Oct 2009 13:05:29 +0000 (+0400)
+Subject: vzctl: refactor write_conf()
+X-Git-Url: 
http://git.openvz.org/?p=vzctl;a=commitdiff_plain;h=35c8a3e3c963446e98b087ea629f32647512af25;hp=acb231d37f8581a31ab98be8deb499e0828f2012
+
+vzctl: refactor write_conf()
+
+There were a few bugs:
+1. If CT config file was a symlink pointing to a file residing
+   on another partition, we failed miserably (bug #1270). To fix,
+   use canonicalize_file_name. Bug reported by xavier martin.
+
+2. Return value from write() syscall was not checked in one place.
+
+In addition to fixing the above bugs, the following improvements were made:
+1. Buffer for tmpfile is allocated dynamically.
+2. Get rid of functionality of writing to stderr, it was never used
+   (maybe only for debug, but it's prehistorical).
+3. Switched from open/write/close to fopen/fprintf/fclose.
+4. Saved (potentially) one syscall for writing '\n'.
+
+Hope I haven't added more bugs...
+
+NOTE1: check (strchr(conf->val, '\n') == NULL) is sufficient here,
+since those lines are read by fgets() in read_conf(), so they can
+only contain '\n' in the end, not in the middle.
+
+NOTE2: we still do not check fprintf() result properly, since it may
+end up writing only part of given string. Not sure if it can hit or not.
+
+http://bugzilla.openvz.org/1270
+
+Signed-off-by: Kir Kolyshkin <k...@openvz.org>
+---
+
+diff --git a/src/lib/config.c b/src/lib/config.c
+index 4c579d7..de8ac09 100644
+--- a/src/lib/config.c
++++ b/src/lib/config.c
+@@ -15,6 +15,9 @@
+  *  along with this program; if not, write to the Free Software
+  *  Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+  */
++#ifndef _GNU_SOURCE
++#define _GNU_SOURCE
++#endif
+ #include <stdlib.h>
+ #include <string.h>
+ #include <ctype.h>
+@@ -2290,43 +2293,56 @@ static int read_conf(char *fname, list_head_t *conf_h)
+ 
+ static int write_conf(char *fname, list_head_t *head)
+ {
+-      char buf[STR_SIZE];
++      char *tmpfile, *file;
+       conf_struct *conf;
+-      int fd = 2;
+-      int len, ret;
+-
+-      if (fname != NULL) {
+-              snprintf(buf, sizeof(buf), "%s.tmp", fname);
+-              if ((fd = open(buf, O_CREAT|O_WRONLY|O_TRUNC, 0644)) < 0) {
+-                      logger(-1, errno, "Unable to create configuration"
+-                              " file %s", buf);
++      FILE * fp;
++      int ret = 1;
++      const char *suffix = ".tmp";
++      char *fmt;
++
++      file = canonicalize_file_name(fname);
++      if (file == NULL) {
++              if (errno != ENOENT)
++              {
++                      logger(-1, errno, "Unable to resolve path %s", fname);
+                       return 1;
+               }
++              file = strdup(fname);
++      }
++      tmpfile = malloc(strlen(file) + strlen(suffix) + 1);
++      sprintf(tmpfile, "%s%s", file, suffix);
++      if ((fp = fopen(tmpfile, "w")) == NULL) {
++              logger(-1, errno, "Unable to create configuration"
++                      " file %s", tmpfile);
++              goto out;
+       }
+       list_for_each(conf, head, list) {
+               if (conf->val == NULL)
+                       continue;
+-              len = strlen(conf->val);
+-              ret = write(fd, conf->val, len);
+-              if (ret < 0) {
+-                      logger(-1, errno, "Unable to write %d bytes to %s",
+-                                      len, buf);
+-                      unlink(buf);
+-                      close(fd);
+-                      return 1;
+-              }
+               if (strchr(conf->val, '\n') == NULL)
+-                      write(fd, "\n", 1);
+-      }
+-      if (fname != NULL) {
+-              close(fd);
+-              if (rename(buf, fname)) {
+-                      logger(-1, errno, "Unable to move %s -> %s",
+-                              buf, fname);
+-                      return 1;
++                      fmt="%s\n";
++              else
++                      fmt="%s";
++              if (fprintf(fp, fmt, conf->val) < 0) {
++                      logger(-1, errno, "Error writing to %s",
++                                      tmpfile);
++                      fclose(fp);
++                      goto out2;
+               }
+       }
+-      return 0;
++      fclose(fp);
++      if (rename(tmpfile, file)) {
++              logger(-1, errno, "Unable to move %s -> %s",
++                      tmpfile, file );
++              goto out2;
++      }
++      ret = 0;
++out2:
++      unlink(tmpfile);
++out:
++      free(tmpfile);
++      free(file);
++      return ret;
+ }
+ 
+ static int vps_merge_conf(list_head_t *dst, list_head_t *src)

Reply via email to