bug#27269: Sed --in-place is messing NTFS file permissions

[Assaf Gordon]

Hello gnulib people,

I'm forwarding (below) a recent sed bug report
( https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27269#11 ).

It seems that gnulib's copy_acl silently fails on NTFS mounts
when compiled with libacl .
When compiled without acl (./configure --disable-acl), it falls back
to chmod/fchmod and everything "just works".

What do you think about it ?

Commands to reproduce are in
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=27269#8 .

Thanks!
- assaf




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

Hello,

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

[Paul Eggert]

Post by Assaf Gordon
set_permissions uses the USE_ACL #define to either call "set_acls"
or call chmod/fchmod directly [4].

It vaguely sounds like a Gnulib bug if set_permissions isn't calling set_acls
when it should. However, your summary of what goes wrong is a bit sketchy; I
can't tell which lines of set-permissions.c are being executed in what order.

[Assaf Gordon]

Hi Paul,

Post by Paul Eggert

Post by Assaf Gordon
set_permissions uses the USE_ACL #define to either call "set_acls"
or call chmod/fchmod directly [4].

It vaguely sounds like a Gnulib bug if set_permissions isn't calling
set_acls when it should. However, your summary of what goes wrong is a
bit sketchy; I can't tell which lines of set-permissions.c are being
executed in what order.

Thanks for looking into this.

Here's a better flow using gdb (running in an NTFS-mounted directory, as
before)
====
$ gdb ~/projects/sed/sed/sed
(gdb) b set_acls
(gdb) r -i.bak 1ifoo world
[....]
Breakpoint 1, set_acls (ctx=0x7fffffffdbb0, name=0x623c30 "./sedmmglZZ",
desc=4, from_mode=0, must_chmod=0x7fffffffdb6e, acls_set=0x7fffffffdb6f)
at lib/set-permissions.c:488
488 int ret = 0;
(gdb) bt
#0 set_acls (ctx=0x7fffffffdbb0, name=0x623c30 "./sedmmglZZ", desc=4,
from_mode=0, must_chmod=0x7fffffffdb6e, acls_set=0x7fffffffdb6f)
at lib/set-permissions.c:488
#1 0x0000000000417ecc in set_permissions (ctx=0x7fffffffdbb0,
name=0x623c30 "./sedmmglZZ", desc=4) at lib/set-permissions.c:812
#2 0x0000000000414b6b in qcopy_acl (src_name=0x7fffffffe2de "world",
source_desc=3, dst_name=0x623c30 "./sedmmglZZ", dest_desc=4, mode=33204)
at lib/qcopy-acl.c:48
#3 0x000000000040b8b0 in copy_acl (src_name=0x7fffffffe2de "world",
source_desc=3, dst_name=0x623c30 "./sedmmglZZ", dest_desc=4, mode=33204)
at lib/copy-acl.c:46
#4 0x00000000004076ae in closedown (input=0x7fffffffdcc0) at
sed/execute.c:677
#5 0x0000000000407847 in read_pattern_space (input=0x7fffffffdcc0,
the_program=0x622260, append=0) at sed/execute.c:730
#6 0x0000000000409576 in process_files (the_program=0x622260,
argv=0x7fffffffdf10) at sed/execute.c:1685
#7 0x000000000040a83b in main (argc=4, argv=0x7fffffffdef8) at
sed/sed.c:377
====

Stepping through "set_acls", acl_set_fd returns 0,
but didn't actually set the permissions:
===
(gdb) n
503 if (! ctx->acls_not_supported)
(gdb)
505 if (ret == 0 && from_mode)
(gdb)
514 if (ret == 0 && ctx->acl)
(gdb) n
516 if (HAVE_ACL_SET_FD && desc != -1)
(gdb) n
517 ret = acl_set_fd (desc, ctx->acl);
(gdb) n
520 if (ret != 0)
(gdb) p ret
$3 = 0
(gdb) n
531 *acls_set = true;
(gdb)
532 if (S_ISDIR(ctx->mode))
(gdb)
749 return ret;

(gdb) !ls -l ./sedmmglZZ
---------- 1 gordon gordon 10 Nov 18 16:14 ./sedmmglZZ
===


And from here all functions just return success:


===
(gdb) n
750 }
(gdb)
set_permissions (ctx=0x7fffffffdbb0, name=0x623c30 "./sedmmglZZ",
desc=4) at lib/set-permissions.c:813
813 if (! acls_set)
(gdb)
833 if (must_chmod && ! early_chmod)
(gdb)
846 return ret;
(gdb)
847 }
(gdb)
qcopy_acl (src_name=0x7fffffffe2de "world", source_desc=3,
dst_name=0x623c30 "./sedmmglZZ", dest_desc=4, mode=33204) at
lib/qcopy-acl.c:49
49 free_permission_context (&ctx);
(gdb)
50 return ret;
(gdb)
51 }
(gdb)
copy_acl (src_name=0x7fffffffe2de "world", source_desc=3,
dst_name=0x623c30 "./sedmmglZZ", dest_desc=4, mode=33204) at
lib/copy-acl.c:47
47 switch (ret)
(gdb)
58 break;
(gdb)
60 return ret;
(gdb)
61 }
(gdb)
closedown (input=0x7fffffffdcc0) at sed/execute.c:681
681 ck_fclose (input->fp);
(gdb)
===


regards,
- assaf

[Paul Eggert]

Post by Assaf Gordon
acl_set_fd returns 0,

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.