GNU bug report logs - #17179
[PATCH] cp: use an invalid dev_t for an unknown device

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 17179 in the body.
You can then email your comments to 17179 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Dave Reisner <dreisner <at> archlinux.org>
To: bug-coreutils <at> gnu.org
Cc: thomas <at> archlinux.org, Dave Reisner <dreisner <at> archlinux.org>
Subject: [PATCH] cp: use an invalid dev_t for an unknown device
Date: Thu,  3 Apr 2014 11:03:14 -0400

On the initial call to copy_internal, we must use an invalid device
number. As of linux 3.14, the mount table has shifted slightly, causing
the initramfs filesystem to have a devno of 0. This is valid, but
confuses cp when attempting to copy only a single filesystem (cp -x).
Since dev_t is defined to be an integer type, we can simply use a
negative value to identify the unknown device.

* src/copy.c (copy_internal): Make initial callers pass -1, compare to
  -1 instead of 0, and update documentation.
---
Hi,

This fixes a bug in Arch Linux's early userspace. We call 'cp -ax' to copy some
files from the early userspace into real root. Since 3.14, this is broken and
cp ignores the filesystem boundaries because it expects that 0 is an invalid
device number. If you're curious about why this changed, please see my analysis
on the util-linux mailing list:

http://www.spinics.net/lists/util-linux-ng/msg09074.html

Cheers,
Dave

 src/copy.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/src/copy.c b/src/copy.c
index 781cc1e..472e5f7 100644
--- a/src/copy.c
+++ b/src/copy.c
@@ -1726,7 +1726,7 @@ should_dereference (const struct cp_options *x, bool command_line_arg)
    any type.  NEW_DST should be true if the file DST_NAME cannot
    exist because its parent directory was just created; NEW_DST should
    be false if DST_NAME might already exist.  DEVICE is the device
-   number of the parent directory, or 0 if the parent of this file is
+   number of the parent directory, or -1 if the parent of this file is
    not known.  ANCESTORS points to a linked, null terminated list of
    devices and inodes of parent directories of SRC_NAME.  COMMAND_LINE_ARG
    is true iff SRC_NAME was specified on the command line.
@@ -2434,7 +2434,7 @@ copy_internal (char const *src_name, char const *dst_name,
         }
 
       /* Decide whether to copy the contents of the directory.  */
-      if (x->one_file_system && device != 0 && device != src_sb.st_dev)
+      if (x->one_file_system && device != -1 && device != src_sb.st_dev)
         {
           /* Here, we are crossing a file system boundary and cp's -x option
              is in effect: so don't copy the contents of this directory. */
@@ -2827,7 +2827,7 @@ copy (char const *src_name, char const *dst_name,
   top_level_dst_name = dst_name;
 
   bool first_dir_created_per_command_line_arg = false;
-  return copy_internal (src_name, dst_name, nonexistent_dst, 0, NULL,
+  return copy_internal (src_name, dst_name, nonexistent_dst, -1, NULL,
                         options, true,
                         &first_dir_created_per_command_line_arg,
                         copy_into_self, rename_succeeded);
-- 
1.9.1

Message #8 received at 17179 <at> debbugs.gnu.org (full text, mbox):

From: Eric Blake <eblake <at> redhat.com>
To: Dave Reisner <dreisner <at> archlinux.org>, 17179 <at> debbugs.gnu.org
Cc: thomas <at> archlinux.org
Subject: Re: bug#17179: [PATCH] cp: use an invalid dev_t for an unknown device
Date: Thu, 03 Apr 2014 09:54:24 -0600

[Message part 1 (text/plain, inline)]

On 04/03/2014 09:03 AM, Dave Reisner wrote:
> On the initial call to copy_internal, we must use an invalid device
> number. As of linux 3.14, the mount table has shifted slightly, causing
> the initramfs filesystem to have a devno of 0. This is valid, but
> confuses cp when attempting to copy only a single filesystem (cp -x).
> Since dev_t is defined to be an integer type, we can simply use a
> negative value to identify the unknown device.

dev_t is defined as an integral type, but not necessarily a signed
integral type, and not necessarily a type as wide as int.  Using -1 as a
sentinel might be okay, but if you do, you MUST use a cast for maximum
portability.

> @@ -2434,7 +2434,7 @@ copy_internal (char const *src_name, char const *dst_name,
>          }
>  
>        /* Decide whether to copy the contents of the directory.  */
> -      if (x->one_file_system && device != 0 && device != src_sb.st_dev)
> +      if (x->one_file_system && device != -1 && device != src_sb.st_dev)

For example, this comparison may fail to do what you want if dev_t is
uint16_t (it will ALWAYS be false, since ((uint16_t)-1) != -1).

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

[signature.asc (application/pgp-signature, attachment)]

Message #11 received at 17179 <at> debbugs.gnu.org (full text, mbox):

From: Pádraig Brady <P <at> draigBrady.com>
To: Eric Blake <eblake <at> redhat.com>
Cc: Dave Reisner <dreisner <at> archlinux.org>, thomas <at> archlinux.org,
 17179 <at> debbugs.gnu.org
Subject: Re: bug#17179: [PATCH] cp: use an invalid dev_t for an unknown device
Date: Thu, 03 Apr 2014 17:38:12 +0100

On 04/03/2014 04:54 PM, Eric Blake wrote:
> On 04/03/2014 09:03 AM, Dave Reisner wrote:
>> On the initial call to copy_internal, we must use an invalid device
>> number. As of linux 3.14, the mount table has shifted slightly, causing
>> the initramfs filesystem to have a devno of 0. This is valid, but
>> confuses cp when attempting to copy only a single filesystem (cp -x).
>> Since dev_t is defined to be an integer type, we can simply use a
>> negative value to identify the unknown device.

This kernel change seems like it might break lots of stuff.
For example I did a simplistic search for "st_dev == 0" and noticed also:
http://cgit.freedesktop.org/systemd/systemd/plain/src/readahead/readahead-common.c
<shrug>
Has the question been broached with the kernel folks about compat?

> dev_t is defined as an integral type, but not necessarily a signed
> integral type, and not necessarily a type as wide as int.  Using -1 as a
> sentinel might be okay, but if you do, you MUST use a cast for maximum
> portability.
> 
>> @@ -2434,7 +2434,7 @@ copy_internal (char const *src_name, char const *dst_name,
>>          }
>>  
>>        /* Decide whether to copy the contents of the directory.  */
>> -      if (x->one_file_system && device != 0 && device != src_sb.st_dev)
>> +      if (x->one_file_system && device != -1 && device != src_sb.st_dev)
> 
> For example, this comparison may fail to do what you want if dev_t is
> uint16_t (it will ALWAYS be false, since ((uint16_t)-1) != -1).

Right, so:

>> -      if (x->one_file_system && device != 0 && device != src_sb.st_dev)
>> +      if (x->one_file_system && device != (dev_t) -1 && device != src_sb.st_dev)

Given the age of these assumptions, I'd be as much worried about
a clash with (dev_t)-1 as 0 TBH?

Pádraig

Message #16 received at 17179-done <at> debbugs.gnu.org (full text, mbox):

From: Paul Eggert <eggert <at> cs.ucla.edu>
To: Pádraig Brady <P <at> draigBrady.com>, 
 Eric Blake <eblake <at> redhat.com>
Cc: Dave Reisner <dreisner <at> archlinux.org>, thomas <at> archlinux.org,
 17179-done <at> debbugs.gnu.org
Subject: Re: bug#17179: [PATCH] cp: use an invalid dev_t for an unknown device
Date: Thu, 03 Apr 2014 09:50:38 -0700

[Message part 1 (text/plain, inline)]

On 04/03/2014 09:38 AM, Pádraig Brady wrote:
> I'd be as much worried about
> a clash with (dev_t)-1 as 0 TBH?
Absolutely.  I installed the attached patch, which should work 
regardless of what dev_t values are found in the file system. This fixes 
just this particular bug, though; I wouldn't be surprised if there are 
others (and haven't looked for them).

[0001-cp-don-t-reserve-a-device-number.patch (text/x-patch, attachment)]

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.