On Sun, Oct 19, 2008 at 12:59:06PM -0700, Junio C Hamano wrote:
> Deskin Miller <deskinm@xxxxxxxxx> writes:
>
> > My patch, on the other hand, is to deal with 'ls' output in case a file has
> > certain filesystem extended attributes. These could be e.g. POSIX ACLs, or a
> > SELinux security context, or perhaps others. If such an extended attribute is
> > present, 'ls -l' will print permissions with a '+' appended, e.g.
> > -rw-r--r--+
> > Instead of
> > -rw-r--r--
> > ...
> > For what it's worth, I've experienced this failure on my Ubuntu 8.04 laptop
> > with SELinux permissive mode, so it's possible ls behaves slightly differently
> > on other systems; I've not been able to determine this one way or another.
>
> Is there way to explicitly tell "ls -l" not to do this, I have to wonder?
I looked through all the options to ls several times, and there wasn't anything
which stood out to me. The output format of ls is rather rigid.
> POSIX.1 says that the file mode written under the -l option is
> "%c%s%s%s%c" (where the first %c is for type, three %s are for owner,
> group and other perm, and the last %c is "optional alternate access method
> flag"). If there is no alternate or additional access control method
> associated with the file, the "optional alternate access method flag"
> would be a single SP, otherwise it would be a printable character.
My fault for not reading up more carefully. In that case, yes, I agree that we
should be able to deal with any such flag character, which my patch does not
do.
> If we drop the default ACL from the trash directory like Matt's patch
> does, does a file created in there (i.e. the ones we check with /bin/ls)
> still have "alternate or additional access control method associated with"
> it?
Yes. SELinux, in particular, associates a 'security context' with *every*
file, which is used in SELinux access control checks. This is stored as a
filesystem extended attribute in the 'security.selinux' namespace, and one will
exist from the moment the file is created.
One could dream up other access control methods, such as a kernel module which
would deny access to any files with the string 'frob' in the filename, and
suppose that ls could be extended to recognise the presence of this module and
alter its display accordingly. There is clearly no way to ensure that no
'alternate or additional access control method' is associated with a file,
since the POSIX.1 language is sufficiently general to allow for almost
anything.
> Somehow it feels wrong that you need your patch, but if you do, stripping
> only the trailing '+' as your patch does not look sufficient, either.
> Shouldn't we be stripping the last letter if the length of actual is
> longer than strlen("-rwxrwxrwx"), as any printable can come there?
I hope I've made the case for the necessity of a patch (if nothing else, t1301
fails 10/16 tests on my system without some sort of patch), and I'll happily
agree that stripping any printable is a better choice. Thanks for the
feedback; FWIW, your suggested patch here is
Tested-by: Deskin Miller <deskinm@xxxxxxxxx>
> t/t1301-shared-repo.sh | 10 ++++++----
> 1 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git c/t/t1301-shared-repo.sh i/t/t1301-shared-repo.sh
> index 2275caa..653362b 100755
> --- c/t/t1301-shared-repo.sh
> +++ i/t/t1301-shared-repo.sh
> @@ -20,6 +20,10 @@ test_expect_success 'shared = 0400 (faulty permission u-w)' '
> test $ret != "0"
> '
>
> +modebits () {
> + ls -l "$1" | sed -e 's|^\(..........\).*|\1|'
> +}
> +
> for u in 002 022
> do
> test_expect_success "shared=1 does not clear bits preset by umask $u" '
> @@ -85,8 +89,7 @@ do
>
> rm -f .git/info/refs &&
> git update-server-info &&
> - actual="$(ls -l .git/info/refs)" &&
> - actual=${actual%% *} &&
> + actual="$(modebits .git/info/refs)" &&
> test "x$actual" = "x-$y" || {
> ls -lt .git/info
> false
> @@ -98,8 +101,7 @@ do
>
> rm -f .git/info/refs &&
> git update-server-info &&
> - actual="$(ls -l .git/info/refs)" &&
> - actual=${actual%% *} &&
> + actual="$(modebits .git/info/refs)" &&
> test "x$actual" = "x-$x" || {
> ls -lt .git/info
> false
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
