cifs: Fake rwx permissions if we can not read the CIFS ACL

Message ID 20171004045953.28134-1-lsahlber@redhat.com
State New
Headers show
Series
  • cifs: Fake rwx permissions if we can not read the CIFS ACL
Related show

Commit Message

Ronnie Sahlberg Oct. 4, 2017, 4:59 a.m.
If the cifsacl mount option is used and we try to list a directory that
contains entries where we do not have READ_CONTROL access we will see errors
such as "ls: cannot access '...': Permission denied"
and the directory listing will show files with funny attributes like
"-?????????? ? ?    ?    ?            ? foo002.txt"

This patch fixes this by checking the error from reading the security
descriptor and if it failed with EACCES we fake the attributes as
all 0.

Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
---
 fs/cifs/cifsacl.c | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

Comments

Steve French Oct. 4, 2017, 10:06 p.m. | #1
merged into cifs-2.6.git for-next but notice minor nit about comments
(always run checpatch before submitting)

WARNING: Block comments use * on subsequent lines
#85: FILE: fs/cifs/cifsacl.c:1160:
+            /* If we do not have permission to read the ACL
+               just clear all rwx permissions */

On Tue, Oct 3, 2017 at 11:59 PM, Ronnie Sahlberg <lsahlber@redhat.com> wrote:
> If the cifsacl mount option is used and we try to list a directory that
> contains entries where we do not have READ_CONTROL access we will see errors
> such as "ls: cannot access '...': Permission denied"
> and the directory listing will show files with funny attributes like
> "-?????????? ? ?    ?    ?            ? foo002.txt"
>
> This patch fixes this by checking the error from reading the security
> descriptor and if it failed with EACCES we fake the attributes as
> all 0.
>
> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
> ---
>  fs/cifs/cifsacl.c | 10 +++++++++-
>  1 file changed, 9 insertions(+), 1 deletion(-)
>
> diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
> index b98436f5c7c7..11d0d135a9b7 100644
> --- a/fs/cifs/cifsacl.c
> +++ b/fs/cifs/cifsacl.c
> @@ -1155,7 +1155,15 @@ cifs_acl_to_fattr(struct cifs_sb_info *cifs_sb, struct cifs_fattr *fattr,
>         /* if we can retrieve the ACL, now parse Access Control Entries, ACEs */
>         if (IS_ERR(pntsd)) {
>                 rc = PTR_ERR(pntsd);
> -               cifs_dbg(VFS, "%s: error %d getting sec desc\n", __func__, rc);
> +               if (rc == -EACCES) {
> +                       /* If we do not have permission to read the ACL
> +                          just clear all rwx permissions */
> +                       cifs_dbg(NOISY, "%s: EACCES reading ACL\n", __func__);
> +                       fattr->cf_mode &= ~(S_IRWXUGO);
> +                       rc = 0;
> +               } else
> +                       cifs_dbg(VFS, "%s: error %d getting sec desc\n",
> +                                __func__, rc);
>         } else {
>                 rc = parse_sec_desc(cifs_sb, pntsd, acllen, fattr);
>                 kfree(pntsd);
> --
> 2.13.3
>
Steve French Oct. 4, 2017, 10:07 p.m. | #2
and added cc:stable

On Wed, Oct 4, 2017 at 5:06 PM, Steve French <smfrench@gmail.com> wrote:
> merged into cifs-2.6.git for-next but notice minor nit about comments
> (always run checpatch before submitting)
>
> WARNING: Block comments use * on subsequent lines
> #85: FILE: fs/cifs/cifsacl.c:1160:
> +            /* If we do not have permission to read the ACL
> +               just clear all rwx permissions */
>
> On Tue, Oct 3, 2017 at 11:59 PM, Ronnie Sahlberg <lsahlber@redhat.com> wrote:
>> If the cifsacl mount option is used and we try to list a directory that
>> contains entries where we do not have READ_CONTROL access we will see errors
>> such as "ls: cannot access '...': Permission denied"
>> and the directory listing will show files with funny attributes like
>> "-?????????? ? ?    ?    ?            ? foo002.txt"
>>
>> This patch fixes this by checking the error from reading the security
>> descriptor and if it failed with EACCES we fake the attributes as
>> all 0.
>>
>> Signed-off-by: Ronnie Sahlberg <lsahlber@redhat.com>
>> ---
>>  fs/cifs/cifsacl.c | 10 +++++++++-
>>  1 file changed, 9 insertions(+), 1 deletion(-)
>>
>> diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
>> index b98436f5c7c7..11d0d135a9b7 100644
>> --- a/fs/cifs/cifsacl.c
>> +++ b/fs/cifs/cifsacl.c
>> @@ -1155,7 +1155,15 @@ cifs_acl_to_fattr(struct cifs_sb_info *cifs_sb, struct cifs_fattr *fattr,
>>         /* if we can retrieve the ACL, now parse Access Control Entries, ACEs */
>>         if (IS_ERR(pntsd)) {
>>                 rc = PTR_ERR(pntsd);
>> -               cifs_dbg(VFS, "%s: error %d getting sec desc\n", __func__, rc);
>> +               if (rc == -EACCES) {
>> +                       /* If we do not have permission to read the ACL
>> +                          just clear all rwx permissions */
>> +                       cifs_dbg(NOISY, "%s: EACCES reading ACL\n", __func__);
>> +                       fattr->cf_mode &= ~(S_IRWXUGO);
>> +                       rc = 0;
>> +               } else
>> +                       cifs_dbg(VFS, "%s: error %d getting sec desc\n",
>> +                                __func__, rc);
>>         } else {
>>                 rc = parse_sec_desc(cifs_sb, pntsd, acllen, fattr);
>>                 kfree(pntsd);
>> --
>> 2.13.3
>>
>
>
>
> --
> Thanks,
>
> Steve

Patch

diff --git a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c
index b98436f5c7c7..11d0d135a9b7 100644
--- a/fs/cifs/cifsacl.c
+++ b/fs/cifs/cifsacl.c
@@ -1155,7 +1155,15 @@  cifs_acl_to_fattr(struct cifs_sb_info *cifs_sb, struct cifs_fattr *fattr,
 	/* if we can retrieve the ACL, now parse Access Control Entries, ACEs */
 	if (IS_ERR(pntsd)) {
 		rc = PTR_ERR(pntsd);
-		cifs_dbg(VFS, "%s: error %d getting sec desc\n", __func__, rc);
+		if (rc == -EACCES) {
+			/* If we do not have permission to read the ACL
+			   just clear all rwx permissions */
+			cifs_dbg(NOISY, "%s: EACCES reading ACL\n", __func__);
+			fattr->cf_mode &= ~(S_IRWXUGO);
+			rc = 0;
+		} else
+			cifs_dbg(VFS, "%s: error %d getting sec desc\n",
+				 __func__, rc);
 	} else {
 		rc = parse_sec_desc(cifs_sb, pntsd, acllen, fattr);
 		kfree(pntsd);