linux-kernel - [PATCH 3.16 325/366] afs: Fix directory permissions check

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <lsq.1539530741.966029350@decadent.org.uk>
Date:   Sun, 14 Oct 2018 16:25:41 +0100
From:   Ben Hutchings <ben@...adent.org.uk>
To:     linux-kernel@...r.kernel.org, stable@...r.kernel.org
CC:     akpm@...ux-foundation.org,
        "Jonathan Billings" <jsbillings@...illings.org>,
        "David Howells" <dhowells@...hat.com>
Subject: [PATCH 3.16 325/366] afs: Fix directory permissions check

3.16.60-rc1 review patch.  If anyone has any objections, please let me know.

------------------

From: David Howells <dhowells@...hat.com>

commit 378831e4daec75fbba6d3612bcf3b4dd00ddbf08 upstream.

Doing faccessat("/afs/some/directory", 0) triggers a BUG in the permissions
check code.

Fix this by just removing the BUG section.  If no permissions are asked
for, just return okay if the file exists.

Also:

 (1) Split up the directory check so that it has separate if-statements
     rather than if-else-if (e.g. checking for MAY_EXEC shouldn't skip the
     check for MAY_READ and MAY_WRITE).

 (2) Check for MAY_CHDIR as MAY_EXEC.

Without the main fix, the following BUG may occur:

 kernel BUG at fs/afs/security.c:386!
 invalid opcode: 0000 [#1] SMP PTI
 ...
 RIP: 0010:afs_permission+0x19d/0x1a0 [kafs]
 ...
 Call Trace:
  ? inode_permission+0xbe/0x180
  ? do_faccessat+0xdc/0x270
  ? do_syscall_64+0x60/0x1f0
  ? entry_SYSCALL_64_after_hwframe+0x49/0xbe

Fixes: 00d3b7a4533e ("[AFS]: Add security support.")
Reported-by: Jonathan Billings <jsbillings@...illings.org>
Signed-off-by: David Howells <dhowells@...hat.com>
Signed-off-by: Ben Hutchings <ben@...adent.org.uk>
---
 fs/afs/security.c | 10 +++-------
 1 file changed, 3 insertions(+), 7 deletions(-)

--- a/fs/afs/security.c
+++ b/fs/afs/security.c
@@ -323,18 +323,14 @@ int afs_permission(struct inode *inode,
 	       mask, access, S_ISDIR(inode->i_mode) ? "dir" : "file");
 
 	if (S_ISDIR(inode->i_mode)) {
-		if (mask & MAY_EXEC) {
+		if (mask & (MAY_EXEC | MAY_READ | MAY_CHDIR)) {
 			if (!(access & AFS_ACE_LOOKUP))
 				goto permission_denied;
-		} else if (mask & MAY_READ) {
-			if (!(access & AFS_ACE_LOOKUP))
-				goto permission_denied;
-		} else if (mask & MAY_WRITE) {
+		}
+		if (mask & MAY_WRITE) {
 			if (!(access & (AFS_ACE_DELETE | /* rmdir, unlink, rename from */
 					AFS_ACE_INSERT))) /* create, mkdir, symlink, rename to */
 				goto permission_denied;
-		} else {
-			BUG();
 		}
 	} else {
 		if (!(access & AFS_ACE_LOOKUP))