RE: CVS lockdir bug with repository symlinks

[Ambalu, Robert]

Just wanted to check if there was any update on this?

-----Original Message-----
From: Ambalu, Robert 
Sent: Monday, June 23, 2003 3:58 PM
To: 'Derek Robert Price'
Cc: 'bug-cvs@gnu.org'
Subject: RE: CVS lockdir bug with repository symlinks


Believe it or not, I'm not too familiar with diffs :)
Here's 'diff -u' output, hope this is what you're looking for.

--- lock.c.orig Mon Jun 23 15:56:39 2003
+++ lock.c      Mon Jun 23 15:56:54 2003
@@ -158,6 +158,10 @@
     mode_t save_umask;
     int saved_umask = 0;
 
+       /*full path in actual repository, less relative portions and 
+         symbolic links*/
+       char real_path[PATH_MAX]; 
+
     if (lock_dir == NULL)
     {
        /* This is the easy case.  Because the lock files go directly
@@ -176,9 +180,53 @@
        assert (current_parsed_root->directory != NULL);
        assert (strncmp (repository, current_parsed_root->directory,
                         strlen (current_parsed_root->directory)) == 0);
-       short_repos = repository + strlen (current_parsed_root->directory) +
1;
 
-       if (strcmp (repository, current_parsed_root->directory) == 0)
+       /*in order to avoid problems with symlinks in the repository, which
would 
+         lead to putting locks in the wrong directories, expand the given
path 
+         to give the REAL path to the directory we are accessing*/
+       if(realpath(repository, real_path)==NULL)
+       {
+               error(1, errno, "Error evaluating real path for %s",
repository);
+       }
+
+       /*if realpath != original given path, then some symlink or possibly 
+         relative dirs were expanded, run extra checks on expanded path,
such 
+         as making sure it exists and making sure it points to somewhere
within 
+         the repository*/
+       if(strcmp(real_path, repository))
+       {
+               
+               /*check if evaluated path actually exists, this may be
overkill 
+                 but doesn't hurt to check*/
+               if(access(real_path, F_OK)) 
+               {
+                       error(1, 0, "path %s evaluated to non-existant
location %s", 
+                                 repository, real_path);
+               }
+               
+               /*check to make sure evaluated path still remains in the
repository, 
+                 simply check the path against the cvs root dir*/
+               if(strncmp (real_path, current_parsed_root->directory, 
+
strlen(current_parsed_root->directory)))
+               {
+                       if(islink(repository))
+                       {
+                               error(1, 0, "Error: path %s (symlink to %s)
leads out of repository", 
+                                         repository, real_path);
+                       }
+                       else
+                               error(1, 0, "Error: path %s leads out of
repository", 
+                                         repository);
+               }
+       }
+       else
+       {
+               strcpy(real_path, repository);
+       }
+       
+       short_repos = real_path + strlen(current_parsed_root->directory) +
1;
+
+       if (strcmp (real_path, current_parsed_root->directory) == 0)
            short_repos = ".";
        else
            assert (short_repos[-1] == '/');
@@ -276,7 +324,8 @@
            saved_umask = 0;
        }
     }
-    return retval;
+
+       return retval;
 }
 
 /*
@@ -636,13 +685,18 @@
     struct dirent *dp;
     struct stat sb;
     int ret = 0;
+       char *real_repository_path;
 
 #ifdef CVS_FUDGELOCKS
 again:
 #endif
 
-    if ((dirp = CVS_OPENDIR (repository)) == NULL)
-       error (1, 0, "cannot open directory %s", repository);
+       /*use lock_name to give use the real path to look in, which is
+         dependent on whether a separate lockdir is in use or not*/
+       real_repository_path = lock_name (repository, "");
+
+    if ((dirp = CVS_OPENDIR (real_repository_path)) == NULL)
+       error (1, 0, "cannot open directory %s", real_repository_path);
 
     errno = 0;
     while ((dp = CVS_READDIR (dirp)) != NULL)
@@ -654,8 +708,8 @@
            (void) time (&now);
 #endif
 
-           line = xmalloc (strlen (repository) + strlen (dp->d_name) + 5);
-           (void) sprintf (line, "%s/%s", repository, dp->d_name);
+           line = xmalloc (strlen (real_repository_path) + strlen
(dp->d_name) + 5);
+           (void) sprintf (line, "%s/%s", real_repository_path,
dp->d_name);
            if ( CVS_STAT (line, &sb) != -1)
            {
 #ifdef CVS_FUDGELOCKS
@@ -690,7 +744,7 @@
        errno = 0;
     }
     if (errno != 0)
-       error (0, errno, "error reading directory %s", repository);
+       error (0, errno, "error reading directory %s",
real_repository_path);
 
     CVS_CLOSEDIR (dirp);
     return (ret);



-----Original Message-----
From: Derek Robert Price [mailto:derek@ximbiot.com]
Sent: Monday, June 23, 2003 3:45 PM
To: Ambalu, Robert
Cc: 'bug-cvs@gnu.org'
Subject: Re: CVS lockdir bug with repository symlinks


Ambalu, Robert wrote:

>There is a bug in cvs when using a separate lock directory and the
>repository has symlinked files within it.  If there are symlinks in the
>repository, the locks will get placed at the location of the symlink rather
>than the location of the actual file (which leads to inconsistent locking).
>  
>

Could you send this patch again as a diff in context or universal 
format?  I prefer universal (`diff -u').

Derek

-- 
                *8^)

Email: derek@ximbiot.com

Get CVS support at <http://ximbiot.com>!
--
If a man speaks in the forest, and there is no
woman around to hear him, is he still wrong?