Attachment 16630 Details for Bug 14721 – git-am fix for master.

[patch] git-am fix for master.

0001-s3-smbd-Fix-uninitialized-memory-read-in-process_sym.patch (text/plain), 4.52 KB, created by Jeremy Allison on 2021-05-27 05:46:58 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Jeremy Allison

Created: 2021-05-27 05:46:58 UTC

Size: 4.52 KB

patch

obsolete

>From e8f03ae41fb0f1ef3dab682cbd124681d3b3387b Mon Sep 17 00:00:00 2001
>From: Jeremy Allison <jra@samba.org>
>Date: Wed, 26 May 2021 22:41:53 -0700
>Subject: [PATCH] s3: smbd: Fix uninitialized memory read in
> process_symlink_open() when used with vfs_shadow_copy2().
>
>Valgrind trace follows.
>
>==3627798== Invalid read of size 1
>==3627798==    at 0x483FF46: strlen (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
>==3627798==    by 0x55DE412: strdup (strdup.c:41)
>==3627798==    by 0x4F4657E: smb_xstrdup (util.c:660)
>==3627798==    by 0x4C62C2E: vfs_ChDir (vfs.c:988)
>==3627798==    by 0x4C4A51C: process_symlink_open (open.c:656)
>==3627798==    by 0x4C4ADE7: non_widelink_open (open.c:862)
>==3627798==    by 0x4C4AFB7: fd_openat (open.c:918)
>==3627798==    by 0x4BBE895: openat_pathref_fsp (files.c:506)
>==3627798==    by 0x4C48A00: filename_convert_internal (filename.c:2027)
>==3627798==    by 0x4C48B77: filename_convert (filename.c:2067)
>==3627798==    by 0x4C32408: call_trans2qfilepathinfo (trans2.c:6173)
>==3627798==    by 0x4C3C5DA: handle_trans2 (trans2.c:10143)
>==3627798==  Address 0xda8bc90 is 96 bytes inside a block of size 217 free'd
>==3627798==    at 0x483DA3F: free (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
>==3627798==    by 0x4FCA3C9: _tc_free_internal (talloc.c:1222)
>==3627798==    by 0x4FCA481: _talloc_free_internal (talloc.c:1248)
>==3627798==    by 0x4FCB825: _talloc_free (talloc.c:1792)
>==3627798==    by 0xDB248DD: store_cwd_data (vfs_shadow_copy2.c:1473)
>==3627798==    by 0xDB24BEF: shadow_copy2_chdir (vfs_shadow_copy2.c:1542)
>==3627798==    by 0x4C662A4: smb_vfs_call_chdir (vfs.c:2257)
>==3627798==    by 0x4C62B48: vfs_ChDir (vfs.c:940)
>==3627798==    by 0x4C4A51C: process_symlink_open (open.c:656)
>==3627798==    by 0x4C4ADE7: non_widelink_open (open.c:862)
>==3627798==    by 0x4C4AFB7: fd_openat (open.c:918)
>==3627798==    by 0x4BBE895: openat_pathref_fsp (files.c:506)
>==3627798==  Block was alloc'd at
>==3627798==    at 0x483C7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
>==3627798==    by 0x4FC9365: __talloc_with_prefix (talloc.c:783)
>==3627798==    by 0x4FC94FF: __talloc (talloc.c:825)
>==3627798==    by 0x4FCCFDC: __talloc_strlendup (talloc.c:2454)
>==3627798==    by 0x4FCD096: talloc_strdup (talloc.c:2470)
>==3627798==    by 0xDB24977: store_cwd_data (vfs_shadow_copy2.c:1476)
>==3627798==    by 0xDB24BEF: shadow_copy2_chdir (vfs_shadow_copy2.c:1542)
>==3627798==    by 0x4C662A4: smb_vfs_call_chdir (vfs.c:2257)
>==3627798==    by 0x4C62B48: vfs_ChDir (vfs.c:940)
>==3627798==    by 0x4C4A92D: non_widelink_open (open.c:755)
>==3627798==    by 0x4C4AFB7: fd_openat (open.c:918)
>==3627798==    by 0x4BBE895: openat_pathref_fsp (files.c:506)
>==3627798==
>
>Even though SMB_VFS_CONNECTPATH() returns a const char,
>vfs_shadow_copy2() can free and reallocate this whilst
>in use inside process_symlink_open().
>
>Take a copy to make sure we don't reference free'd memory.
>
>BUG: https://bugzilla.samba.org/show_bug.cgi?id=14721
>
>Signed-off-by: Jeremy Allison <jra@samba.org>
>---
> source3/smbd/open.c | 15 +++++++++++----
> 1 file changed, 11 insertions(+), 4 deletions(-)
>
>diff --git a/source3/smbd/open.c b/source3/smbd/open.c
>index 4a5d57cfb7b..0f96afb6265 100644
>--- a/source3/smbd/open.c
>+++ b/source3/smbd/open.c
>@@ -532,7 +532,7 @@ static NTSTATUS process_symlink_open(const struct files_struct *dirfsp,
> {
> 	struct connection_struct *conn = dirfsp->conn;
> 	const char *conn_rootdir = NULL;
>-	struct smb_filename conn_rootdir_fname;
>+	struct smb_filename conn_rootdir_fname = { 0 };
> 	char *link_target = NULL;
> 	int link_len = -1;
> 	struct smb_filename *oldwd_fname = NULL;
>@@ -547,9 +547,15 @@ static NTSTATUS process_symlink_open(const struct files_struct *dirfsp,
> 	if (conn_rootdir == NULL) {
> 		return NT_STATUS_NO_MEMORY;
> 	}
>-	conn_rootdir_fname = (struct smb_filename) {
>-		.base_name = discard_const_p(char, conn_rootdir),
>-	};
>+	/*
>+	 * With shadow_copy2 conn_rootdir can be talloc_freed
>+	 * whilst we use it in this functions. We must take a copy.
>+	 */
>+	conn_rootdir_fname.base_name = talloc_strdup(talloc_tos(),
>+						     conn_rootdir);
>+	if (conn_rootdir_fname.base_name == NULL) {
>+		return NT_STATUS_NO_MEMORY;
>+	}
> 
> 	/*
> 	 * Ensure we don't get stuck in a symlink loop.
>@@ -669,6 +675,7 @@ static NTSTATUS process_symlink_open(const struct files_struct *dirfsp,
> 
> 	TALLOC_FREE(resolved_fname);
> 	TALLOC_FREE(link_target);
>+	TALLOC_FREE(conn_rootdir_fname.base_name);
> 	if (oldwd_fname != NULL) {
> 		int ret = vfs_ChDir(conn, oldwd_fname);
> 		if (ret == -1) {
>-- 
>2.27.0
>

Flags:

slow: review+

Actions: View

Attachments on bug 14721: 16630 | 16632 | 16633 | 16661