All of lore.kernel.org
 help / color / mirror / Atom feed
From: Bjoern Doebel <doebel@amazon.de>
To: <linux-cifs@vger.kernel.org>
Cc: <linkinjeon@kernel.org>, <pc@manguebit.com>,
	<stable@vger.kernel.org>, Bjoern Doebel <doebel@amazon.de>
Subject: [PATCH v3 1/2] smb: client: fix heap overflow in DACL owner/group rewrite
Date: Tue, 8 Sep 2026 16:10:00 +0000	[thread overview]
Message-ID: <20260908161001.2603610-2-doebel@amazon.de> (raw)
In-Reply-To: <20260908161001.2603610-1-doebel@amazon.de>

When id_mode_to_cifs_acl rewrites an existing DACL, it allocates a
buffer sized according to the on-disk DACL length reported by
dacl_ptr->size. However, replace_sids_and_copy_aces may rewrite each
ACE with a new owner/group SID obtained from the cifs.idmap upcall.
Those SIDs can have up to SID_MAX_SUB_AUTHORITIES (15) sub-authorities,
making each ACE up to 76 bytes (sizeof(struct smb_ace)).

If the original DACL contains short SIDs (e.g., 1 sub-authority) while
the replacement SIDs are long, the rewritten ACEs overflow the
allocation.

Fix this by always budgeting for worst-case SID expansion: allocate
sizeof(struct smb_acl) plus num_aces * sizeof(struct smb_ace), which
covers the smb_acl header and room for every ACE at maximum SID size.
This replaces the previous split logic that used dacl_ptr->size for
cifsacl mounts but num_aces * sizeof(struct smb_ace) for mode_from_sid
mounts: both paths can trigger the same rewrite and need the same
headroom.

KASAN reports this as:
  BUG: KASAN: slab-out-of-bounds in build_sec_desc+0x1e8a/0x2680 [cifs]
  Write of size 4 at addr ffff8881a5e25374 by task chown/5298
  ...
  The buggy address is located 0 bytes to the right of
   allocated 884-byte region [ffff8881a5e25000, ffff8881a5e25374)

Cc: stable@vger.kernel.org
Fixes: bc3e9dd9d104 ("cifs: Change SIDs in ACEs while transferring file ownership.")
Assisted-by: Kiro:claude-opus-4.6
Signed-off-by: Bjoern Doebel <doebel@amazon.de>
Reviewed-by: Namjae Jeon <linkinjeon@kernel.org>

---
v3:
- Correct the Fixes: tag; the previous commit id did not exist
- Add Namjae Jeon's Reviewed-by
v2:
- Reword commit message to be more descriptive of what is happening
---
 fs/smb/client/cifsacl.c | 12 +++++++-----
 1 file changed, 7 insertions(+), 5 deletions(-)

diff --git a/fs/smb/client/cifsacl.c b/fs/smb/client/cifsacl.c
index 12005f46307de..2d785a3039585 100644
--- a/fs/smb/client/cifsacl.c
+++ b/fs/smb/client/cifsacl.c
@@ -1815,11 +1815,13 @@ id_mode_to_cifs_acl(struct inode *inode, const char *path, __u64 *pnmode,
 				cifs_put_tlink(tlink);
 				return rc;
 			}
-			if (mode_from_sid)
-				nsecdesclen +=
-					le16_to_cpu(dacl_ptr->num_aces) * sizeof(struct smb_ace);
-			else /* cifsacl */
-				nsecdesclen += le16_to_cpu(dacl_ptr->size);
+			/*
+			 * Worst case: every ACE is rewritten with a new SID of
+			 * SID_MAX_SUB_AUTHORITIES sub-auths -> sizeof(smb_ace) each,
+			 * plus the smb_acl header replace_sids_and_copy_aces() emits.
+			 */
+			nsecdesclen += sizeof(struct smb_acl) +
+				le16_to_cpu(dacl_ptr->num_aces) * sizeof(struct smb_ace);
 		}
 	}
 
-- 
2.50.1


  reply	other threads:[~2026-09-08 16:10 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-09 15:54 [PATCH] smb: client: fix DACL-rewrite heap overflow in id_mode_to_cifs_acl() Bjoern Doebel
2026-07-09 22:15 ` Steve French
2026-07-10  6:37   ` Bjoern Doebel
2026-09-04 12:58 ` [PATCH] smb: client: fix heap overflow in DACL owner/group rewrite Bjoern Doebel
2026-09-05  1:15   ` Namjae Jeon
2026-09-07 17:59     ` Bjoern Doebel
2026-09-08 16:09   ` [PATCH v3 0/2] smb: client: fix DACL rewrite overflows Bjoern Doebel
2026-09-08 16:10     ` Bjoern Doebel [this message]
2026-09-08 16:10     ` [PATCH v3 2/2] smb: client: fail DACL rewrite when the new DACL exceeds 64K Bjoern Doebel
2026-09-09  2:55       ` Namjae Jeon
2026-09-09 17:15     ` [PATCH v3 0/2] smb: client: fix DACL rewrite overflows Paulo Alcantara

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260908161001.2603610-2-doebel@amazon.de \
    --to=doebel@amazon.de \
    --cc=linkinjeon@kernel.org \
    --cc=linux-cifs@vger.kernel.org \
    --cc=pc@manguebit.com \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.