On Wed, Jul 02, 2014 at 09:41:07PM +0200, Cédric Barboiron wrote:
> Package: apt
> Version: 1.0.5
> Severity: important
Thanks for your bugreport.
> (warning: attached patch is not a solution, it is just intended to show
> the problem)
>
> Setting Dir::Cache::archives and Dir::Cache to the empty string (as
> instructed by man 5 apt.conf) do NOT disable cache but set it to '/'.
>
> Consequence: apt-get clean then effectively cleans '/' and removes all
> files here.
>
> Not true anymore but even worse, on squeeze it also removes the '/lib64'
> symlink, breaking the loader and preventing any new dynamically linked
> binary to be launched.
This sounds like we want to improve the description of the apt.conf
manpage. I assume your use-case was to disable the binary cache? Maybe
you can suggest a way to make the description clearer to avoid the
issue for others?
As for protecting against this, I attached a patch that makes clean a
bit more careful and fix the example where Dir::Cache::archives="" is
empty. But there is only so much we can do, e.g. if someone sets
Dir::Cache=/vmlinuz its hard to have a programmatic way to detect that
this is a bad idea. But I'm happy to protect against obvious ones
(like Clean("/")).
Cheers,
Michael
> - - -
> all following tests done on debian testing, up to date on 2014-07-02
>
> current result:
>
> debdev# cat apt.conf
> Dir::Cache "";
> Dir::Cache::archives "";
> debdev# touch /VERY_SECRET
> debdev# ls /
> bin boot dev etc home initrd.img initrd.img.old lib lib64 lost+found
> media mnt opt proc root run sbin srv sys tmp usr var VERY_SECRET
> vmlinuz vmlinuz.old
> debdev# apt-get clean
> debdev# ls /
> bin boot dev etc home lib lib64 lock lost+found media mnt opt
> proc root run sbin srv sys tmp usr var
>
>
> reading source code (contrib/configuration.cc) instead of the man page of
> apt.conf:
>
> debdev# cat apt.conf
> Dir::Cache "/dev/null";
> Dir::Cache::archives "/dev/null";
> debdev# touch /VERY_SECRET
> debdev# ls /
> bin boot dev etc home lib lib64 lock lost+found media mnt opt
> proc root run sbin srv sys tmp usr var VERY_SECRET
> debdev# apt-get clean
> debdev# ls /
> bin boot dev etc home lib lib64 lock lost+found media mnt opt
> proc root run sbin srv sys tmp usr var VERY_SECRET
>
> expected result, BUT BUT BUT its not a good idea at all :
>
> debdev# cat /etc/apt/apt.conf
> Dir::Cache "/dev/null";
> Dir::Cache::archives "/dev/null";
> debdev# ls -l /dev/null
> crw-rw-rw- 1 root root 1, 3 Jul 2 20:11 /dev/null
> debdev# apt-get install libcaca
> Reading package lists... Error!
> E: Write error - write (28: No space left on device)
> E: Can't mmap an empty file
> E: Failed to truncate file - ftruncate (9: Bad file descriptor)
> E: The package lists or status file could not be parsed or opened.
> debdev# ls -l /dev/null
> -rw-r--r-- 1 root root 0 Jul 2 20:17 /dev/null
> debdev# df -h
> Filesystem Size Used Avail Use% Mounted on
> /dev/mapper/debdev-root 95G 5.0G 85G 6% /
> udev 10M 10M 0 100% /dev
> tmpfs 202M 200K 201M 1% /run
> tmpfs 5.0M 0 5.0M 0% /run/lock
> tmpfs 403M 0 403M 0% /run/shm
> /dev/sda1 228M 80M 137M 37% /boot
> none 4.0K 0 4.0K 0% /sys/fs/cgroup
>
> and finally with attached patch (built without "make test" because it has
> other side-effects):
>
> debdev# cat /etc/apt/apt.conf
> Dir::Cache "";
> Dir::Cache::archives "";
> debdev# touch /MYTRALALA
> debdev# ls /
> bin dev home lib64 lost+found mnt opt root sbin sys usr
> boot etc lib lock media MYTRALALA proc run srv tmp var
> debdev# apt-get clean
> E: Ignored empty string directory configuration (would have been expanded to
> '/' otherwise)
> debdev# ls /
> bin dev home lib64 lost+found mnt opt root sbin sys usr
> boot etc lib lock media MYTRALALA proc run srv tmp var
>
>
> diff --git a/apt-pkg/contrib/configuration.cc
> b/apt-pkg/contrib/configuration.cc
> index 00f6ad0..3dd63aa 100644
> --- a/apt-pkg/contrib/configuration.cc
> +++ b/apt-pkg/contrib/configuration.cc
> @@ -240,6 +240,11 @@ string Configuration::FindFile(const char *Name,const
> char *Default) const
> string Configuration::FindDir(const char *Name,const char *Default) const
> {
> string Res = FindFile(Name,Default);
> + if (Res == "")
> + {
> + _error->Error(_("Ignored empty string directory configuration (would
> have been expanded to '/' otherwise)"));
> + return Res;
> + }
> if (Res.end()[-1] != '/')
> {
> size_t const found = Res.rfind("/dev/null");
> diff --git a/doc/apt.conf.5.xml b/doc/apt.conf.5.xml
> index fcbf20d..e30898c 100644
> --- a/doc/apt.conf.5.xml
> +++ b/doc/apt.conf.5.xml
> @@ -607,8 +607,8 @@ DPkg::Pre-Install-Pkgs {"/usr/sbin/dpkg-preconfigure
> --apt";};
> <para><literal>Dir::Cache</literal> contains locations pertaining to
> local cache
> information, such as the two package caches
> <literal>srcpkgcache</literal> and
> <literal>pkgcache</literal> as well as the location to place downloaded
> archives,
> - <literal>Dir::Cache::archives</literal>. Generation of caches can be
> turned off
> - by setting their names to the empty string. This will slow down startup
> but
> + <literal>Dir::Cache::archives</literal>. Generation of caches CANNOT BE
> TURNED OFF.
> + This would slow down startup but could
> save disk space. It is probably preferable to turn off the pkgcache rather
> than the srcpkgcache. Like <literal>Dir::State</literal> the default
> directory is contained in <literal>Dir::Cache</literal></para>
diff --git a/apt-pkg/acquire.cc b/apt-pkg/acquire.cc
index a187a00..057bc24 100644
--- a/apt-pkg/acquire.cc
+++ b/apt-pkg/acquire.cc
@@ -486,6 +486,9 @@ bool pkgAcquire::Clean(string Dir)
if (DirectoryExists(Dir) == false)
return true;
+ if(Dir == "/")
+ return _error->Error(_("Clean of %s is not supported"), Dir.c_str());
+
DIR *D = opendir(Dir.c_str());
if (D == 0)
return _error->Errno("opendir",_("Unable to read %s"),Dir.c_str());
diff --git a/apt-pkg/clean.cc b/apt-pkg/clean.cc
index 0ee3b76..37128e9 100644
--- a/apt-pkg/clean.cc
+++ b/apt-pkg/clean.cc
@@ -34,7 +34,10 @@
bool pkgArchiveCleaner::Go(std::string Dir,pkgCache &Cache)
{
bool CleanInstalled = _config->FindB("APT::Clean-Installed",true);
-
+
+ if(Dir == "/")
+ return _error->Error(_("Clean of %s is not supported"), Dir.c_str());
+
DIR *D = opendir(Dir.c_str());
if (D == 0)
return _error->Errno("opendir",_("Unable to read %s"),Dir.c_str());
