GNU bug report logs - #27269
Sed --in-place is messing NTFS file permissions

Previous Next

To reply to this bug, email your comments to 27269 AT debbugs.gnu.org.

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

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

From: Maiko Cezar Rodrigues Costa <maiko.cezar <at> gmail.com>
To: bug-sed <at> gnu.org
Subject: Sed --in-place is messing NTFS file permissions
Date: Wed, 07 Jun 2017 02:45:39 +0000

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

Hi, I've a problem with sed when using the --in-place option in a mounted
NTFS partition;
$ sed --version
sed (GNU sed) 4.4
Copyright (C) 2017 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html
>.
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.

Written by Jay Fenlason, Tom Lord, Ken Pizzini,
and Paolo Bonzini.
GNU sed home page: <http://www.gnu.org/software/sed/>.
General help using GNU software: <http://www.gnu.org/gethelp/>.
E-mail bug reports to: <bug-sed <at> gnu.org>.


$ echo " test string teste" > x

$ sed --in-place=.bak 's <at> tes.*string <at> TESTE <at> g' x

$ ls -l
-rw-r--r-- 1 maiko users   13 jun  6 23:39  x
-rw-r--r-- 1 maiko users   19 jun  6 23:39  x.bak

$ cat x
 TESTE teste

$ cat x.bak
 test string teste

 So far, so good.

 my /etc/fstab

UUID=1CACB8ABACB88136 /media/FILES ntfs defaults,permissions,noatime 0 0


$ cd /media/FILES

$ echo " test string teste" > x

$ sed --in-place=.bak 's <at> tes.*string <at> TESTE <at> g' x

$ ls -l
---------- 1 maiko users   13 jun  6 23:42  x
-rw-r--r-- 1 maiko users   19 jun  6 23:42  x.bak

$ sudo cat x
 TESTE teste

$ cat x.bak
 test string teste


This guy has a very similar problem too
https://unix.stackexchange.com/questions/244603/why-is-sed-changing-permissions-of-a-file-on-a-cifs-mounted-share

Thanks
-- 

Maiko Cezar R Costa

PGP Public Key
<http://keyserver1.pgp.com/vkd/DownloadKey.event?keyid=0xB69302C5F63F5F84>

[Message part 2 (text/html, inline)]

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

From: Assaf Gordon <assafgordon <at> gmail.com>
To: Maiko Cezar Rodrigues Costa <maiko.cezar <at> gmail.com>, 27269 <at> debbugs.gnu.org
Subject: Re: bug#27269: Sed --in-place is messing NTFS file permissions
Date: Wed, 15 Nov 2017 02:23:31 -0700

Hello,

On 2017-06-06 08:45 PM, Maiko Cezar Rodrigues Costa wrote:
> Hi, I've a problem with sed when using the --in-place option in a mounted
> NTFS partition;

Thank you for the report.

I can reproduce this and I believe it is a regression in sed-4.4 that 
happens when an ntfs partition is mounted with "-o permissions".

To reproduce:

Create dummy NTFS partition and mount it:

  cd /tmp/
  dd if=/dev/zero of=ntfs.partition.bin bs=1M count=20
  mkfs.ntfs --force ./ntfs.partition.bin
  mkdir /tmp/mnt
  sudo mount -t ntfs -o permissions ./ntfs.partition.bin /tmp/mnt/
  cd /tmp/mnt

Create first file:

  $ echo hello > world
  $ ls -l
  total 1
  -rw-rw-r-- 1 gordon gordon 6 Nov 15 02:16 world

Use sed-4.2, permissions retained:

  $ sed-4.2.2 -i.bak-sed4.2 's/e/x/' world
  $ ls -l
  total 1
  -rw-rw-r-- 1 gordon gordon 6 Nov 15 02:16 world
  -rw-rw-r-- 1 gordon gordon 6 Nov 15 02:16 world.bak-sed4.2

Same with sed-4.4, permissions lost:

  $ sed-4.4 -i.bak-sed4.4 's/e/x/' world
  $ ls -l
  total 2
  ---------- 1 gordon gordon 6 Nov 15 02:16 world
  -rw-rw-r-- 1 gordon gordon 6 Nov 15 02:16 world.bak-sed4.2
  -rw-rw-r-- 1 gordon gordon 6 Nov 15 02:16 world.bak-sed4.4



I will investigate further to try and pinpoint the offending code.

regards,
 - assaf

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

From: Assaf Gordon <assafgordon <at> gmail.com>
To: Maiko Cezar Rodrigues Costa <maiko.cezar <at> gmail.com>, 27269 <at> debbugs.gnu.org
Subject: Re: bug#27269: Sed --in-place is messing NTFS file permissions
Date: Wed, 15 Nov 2017 03:43:30 -0700

Hello,

On 2017-11-15 02:23 AM, Assaf Gordon wrote:
> On 2017-06-06 08:45 PM, Maiko Cezar Rodrigues Costa wrote:
>> Hi, I've a problem with sed when using the --in-place option in a mounted
>> NTFS partition;
> 
> I can reproduce this and I believe it is a regression in sed-4.4 that 
> happens when an ntfs partition is mounted with "-o permissions".

The above statement is incorrect.
It is not a regression in sed-4.4 compared to sed-4.2 but a different 
issue relating to ACL support during compilation ("./configure 
--disable-acl").

If ACL support is included, NTFS permissions DO NOT work.
If ACL support is NOT included, NTFS permissions DO work.

What confused me before is that on Debian/Ubuntu systems
the default sed (/bin/sed) is compiled without ACL
and using it "just worked", while compiling from source code
did include ACL and it never worked.


Current Work around:
1. Use the system's default sed (e.g /bin/sed) which is likely compiled 
without ACL, or
2. Build sed with "./configure --disable-acl".
This of course will lose the ability to copy other ACLs on non-NTFS file 
systems.


----

Technical reason:
1.
If "./configure" finds the header file <sys/acl.h>, ACL support is included.

2.
Another way to test ACL support is by checking if the sed binary
requires the acl shared library:

    $ ldd /bin/sed | grep acl
    $ ldd ~/projects/sed/sed/sed | grep acl
        libacl.so.1 => /lib/x86_64-linux-gnu/libacl.so.1

3.
If ACL support is compiled in sed, using "--in-place"
leads to the following syscalls:

  fchown(4, 1000, 1000)                   = 0
  fgetxattr(3, "system.posix_acl_access", "", 132) = 0
  fstat(3, {st_mode=S_IFREG|0664, st_size=6, ...}) = 0
  fsetxattr(4, "system.posix_acl_access", "\2\0\0\0\37....", 28, 0) = 0
  close(3)                                = 0
  close(4)                                = 0
  rename("world", "world.bak")            = 0
  rename("./sedcrOmKi", "world")          = 0

4.
If ACL support is not compiled in sed, using "--in-place"
leads to the following syscalls:

  fchown(4, 1000, 1000)                   = 0
  fchmod(4, 0100664)                      = 0
  close(3)                                = 0
  close(4)                                = 0
  rename("world", "world.bak")            = 0
  rename("./sednDUqpF", "world")          = 0


One can perhaps infer that fsetxattr/fgetxattr do not work on NTFS
mounts, but also don't fail with ENOTSUP.

----

Code flow:
sed's execute.c calls gnulib's "copy_acl" [1].
copy_acl calls qcopy_acl [2].
qcopy_acl calls set_permissions [3]
set_permissions uses the USE_ACL #define to either call "set_acls"
or call chmod/fchmod directly [4].

[1] https://opengrok.housegordon.com/source/xref/sed/sed/execute.c#677
[2] https://opengrok.housegordon.com/source/xref/gnulib/lib/copy-acl.c#43
[3] https://opengrok.housegordon.com/source/xref/gnulib/lib/qcopy-acl.c#39
[4] 
https://opengrok.housegordon.com/source/xref/gnulib/lib/set-permissions.c#776

----

Is this a bug?
and if so, is it in sed or gnulib or libacl ?
Not sure about this.

Jim and all,
What do you think?



regards,
 - assaf

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

From: Jim Meyering <jim <at> meyering.net>
To: Assaf Gordon <assafgordon <at> gmail.com>
Cc: 27269 <at> debbugs.gnu.org, Maiko Cezar Rodrigues Costa <maiko.cezar <at> gmail.com>
Subject: Re: bug#27269: Sed --in-place is messing NTFS file permissions
Date: Thu, 16 Nov 2017 22:22:40 -0800

On Wed, Nov 15, 2017 at 2:43 AM, Assaf Gordon <assafgordon <at> gmail.com> wrote:
...
> 3.
> If ACL support is compiled in sed, using "--in-place"
> leads to the following syscalls:
>
>   fchown(4, 1000, 1000)                   = 0
>   fgetxattr(3, "system.posix_acl_access", "", 132) = 0
>   fstat(3, {st_mode=S_IFREG|0664, st_size=6, ...}) = 0
>   fsetxattr(4, "system.posix_acl_access", "\2\0\0\0\37....", 28, 0) = 0
>   close(3)                                = 0
>   close(4)                                = 0
>   rename("world", "world.bak")            = 0
>   rename("./sedcrOmKi", "world")          = 0
>
> 4.
> If ACL support is not compiled in sed, using "--in-place"
> leads to the following syscalls:
>
>   fchown(4, 1000, 1000)                   = 0
>   fchmod(4, 0100664)                      = 0
>   close(3)                                = 0
>   close(4)                                = 0
>   rename("world", "world.bak")            = 0
>   rename("./sednDUqpF", "world")          = 0
>
>
> One can perhaps infer that fsetxattr/fgetxattr do not work on NTFS
> mounts, but also don't fail with ENOTSUP.
...
> Is this a bug?
> and if so, is it in sed or gnulib or libacl ?
> Not sure about this.

Hi Assaf,
Thank you for the thorough analysis.
Following your instructions in https://debbugs.gnu.org/27269#8, I
reproduced the failure on Fedora 26 with Linux 4.13.12.
That looks like either we're somehow telling f[gs]etxattr to do the
wrong thing, or it's a bug that they fail to do what we want, yet they
return 0/success.

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

From: Assaf Gordon <assafgordon <at> gmail.com>
To: Jim Meyering <jim <at> meyering.net>
Cc: 27269 <at> debbugs.gnu.org, Maiko Cezar Rodrigues Costa <maiko.cezar <at> gmail.com>
Subject: Re: bug#27269: Sed --in-place is messing NTFS file permissions
Date: Tue, 9 Oct 2018 05:24:18 -0600

severity 27269 wishlist
stop

Hello,

> On Wed, Nov 15, 2017 at 2:43 AM, Assaf Gordon <assafgordon <at> gmail.com> wrote:
>> One can perhaps infer that fsetxattr/fgetxattr do not work on NTFS
>> mounts, but also don't fail with ENOTSUP.
In a follow-up thread in bug-gnulib@ [1], Paul Eggert wrote [2]:

  "This sounds like a bug inside the kernel then, in the ntfs code
  presumably. Perhaps we could work around the bug in Gnulib, but that
  sounds a bit expensive."

[1] https://lists.gnu.org/archive/html/bug-gnulib/2017-11/msg00031.html
[2] https://lists.gnu.org/archive/html/bug-gnulib/2017-11/msg00035.html


I'm therefore marking this as a "wishlist" item (but keeping it open, 
for future improvements).

regards,
 - assaf

Previous Next

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