Skip to content

Commit af92ee9

Browse files
jdmfrsmfrench
authored andcommitted
ksmbd: fix SID memory leak in set_posix_acl_entries_dacl() on overflow
Commit 299f962 ("ksmbd: use check_add_overflow() to prevent u16 DACL size overflow") added check_add_overflow() guards that break out of the ACE-building loops in set_posix_acl_entries_dacl() when the accumulated DACL size would wrap past 65535. However, each iteration allocates a struct smb_sid via kmalloc_obj() at the top of the loop and relies on the kfree(sid) call at the end of the loop body (the 'pass_same_sid' label in the first loop, and the explicit kfree at the tail of the second loop) to release it. The newly introduced 'break' statements bypass those kfree() calls, leaking the sid buffer every time an overflow is detected. A malicious or malformed file with enough POSIX ACL entries to trip the overflow check will leak one or more struct smb_sid allocations on every request that touches the file's DACL, providing a trivial kernel memory exhaustion vector. Free sid before breaking out of the loops to plug the leak. Fixes: 299f962 ("ksmbd: use check_add_overflow() to prevent u16 DACL size overflow") Cc: stable@vger.kernel.org Signed-off-by: Ferry Meng <mengferry@linux.alibaba.com> Acked-by: Namjae Jeon <linkinjeon@kernel.org> Signed-off-by: Steve French <stfrench@microsoft.com>
1 parent 5d69190 commit af92ee9

1 file changed

Lines changed: 9 additions & 3 deletions

File tree

fs/smb/server/smbacl.c

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -643,8 +643,10 @@ static void set_posix_acl_entries_dacl(struct mnt_idmap *idmap,
643643
ntace = (struct smb_ace *)((char *)pndace + *size);
644644
ace_sz = fill_ace_for_sid(ntace, sid, ACCESS_ALLOWED, flags,
645645
pace->e_perm, 0777);
646-
if (check_add_overflow(*size, ace_sz, size))
646+
if (check_add_overflow(*size, ace_sz, size)) {
647+
kfree(sid);
647648
break;
649+
}
648650
(*num_aces)++;
649651
if (pace->e_tag == ACL_USER)
650652
ntace->access_req |=
@@ -655,8 +657,10 @@ static void set_posix_acl_entries_dacl(struct mnt_idmap *idmap,
655657
ntace = (struct smb_ace *)((char *)pndace + *size);
656658
ace_sz = fill_ace_for_sid(ntace, sid, ACCESS_ALLOWED,
657659
0x03, pace->e_perm, 0777);
658-
if (check_add_overflow(*size, ace_sz, size))
660+
if (check_add_overflow(*size, ace_sz, size)) {
661+
kfree(sid);
659662
break;
663+
}
660664
(*num_aces)++;
661665
if (pace->e_tag == ACL_USER)
662666
ntace->access_req |=
@@ -698,8 +702,10 @@ static void set_posix_acl_entries_dacl(struct mnt_idmap *idmap,
698702
ntace = (struct smb_ace *)((char *)pndace + *size);
699703
ace_sz = fill_ace_for_sid(ntace, sid, ACCESS_ALLOWED, 0x0b,
700704
pace->e_perm, 0777);
701-
if (check_add_overflow(*size, ace_sz, size))
705+
if (check_add_overflow(*size, ace_sz, size)) {
706+
kfree(sid);
702707
break;
708+
}
703709
(*num_aces)++;
704710
if (pace->e_tag == ACL_USER)
705711
ntace->access_req |=

0 commit comments

Comments
 (0)