Bug 13940 - vfs_ceph debug logs uninitialized memory
Summary: vfs_ceph debug logs uninitialized memory
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: VFS Modules (show other bugs)
Version: unspecified
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2019-05-10 09:41 UTC by David Disseldorp
Modified: 2019-05-21 10:56 UTC (History)
2 users (show)

See Also:


Attachments
fix for master (1.20 KB, patch)
2019-05-10 11:32 UTC, David Disseldorp
no flags Details
Cherry-pick of fix for 4.8, 4.9, and 4.10.next (1.44 KB, patch)
2019-05-13 10:34 UTC, David Disseldorp
asn: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description David Disseldorp 2019-05-10 09:41:01 UTC
Found by valgrind:

==185== Conditional jump or move depends on uninitialised value(s)
==185==    at 0x4C318B9: __strlen_sse2 (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==185==    by 0x15B018C3: vfprintf (in /lib64/libc-2.26.so)
==185==    by 0x15B292E8: vasprintf (in /lib64/libc-2.26.so)
==185==    by 0x60D3F62: __dbgtext_va (in /home/ddiss/isms/samba/bin/default/lib/util/libsamba-deb
ug-samba4.so)
==185==    by 0x60D4064: dbgtext (in /home/ddiss/isms/samba/bin/default/lib/util/libsamba-debug-sa
mba4.so)
==185==    by 0x19741585: cephwrap_flistxattr (in /home/ddiss/isms/samba/bin/default/source3/modul
es/libvfs_module_ceph.so)
==185==    by 0x522568A: smb_vfs_call_flistxattr (in /home/ddiss/isms/samba/bin/default/source3/li
bsmbd-base-samba4.so)



Fix is a simple one liner (I'll attach when done):

diff --git a/source3/modules/vfs_ceph.c b/source3/modules/vfs_ceph.c
index f62fef05614..fbb37dd0b68 100644
--- a/source3/modules/vfs_ceph.c
+++ b/source3/modules/vfs_ceph.c
@@ -1306,7 +1306,7 @@ static ssize_t cephwrap_listxattr(struct vfs_handle_struct *handle,
 static ssize_t cephwrap_flistxattr(struct vfs_handle_struct *handle, struct files_struct *fsp, char *list, size_t size)
 {
        int ret;
-       DBG_DEBUG("[CEPH] flistxattr(%p, %p, %s, %llu)\n", handle, fsp, list, llu(size));
+       DBG_DEBUG("[CEPH] flistxattr(%p, %p, %p, %llu)\n", handle, fsp, list, llu(size));
 #if LIBCEPHFS_VERSION_CODE >= LIBCEPHFS_VERSION(0, 94, 0)
        ret = ceph_flistxattr(handle->data, fsp->fh->fd, list, size);
 #else



The @list buffer is uninitialized until the ceph_flistxattr() call. Looks like the %s was unintended, as cephwrap_listxattr() does the right thing (I've also checked all other vfs_ceph DBG_X(...%s) invocations).
Comment 1 David Disseldorp 2019-05-10 09:57:22 UTC
FWIW, I've flagged this as Samba-Core for now, as I'm not sure whether it should be handled as a security issue (I hope not).
Comment 2 David Disseldorp 2019-05-10 11:32:15 UTC
Created attachment 15141 [details]
fix for master
Comment 3 David Disseldorp 2019-05-10 11:45:39 UTC
Andreas provided review feedback via irc, so I've pushed to autobuild.
Comment 4 David Disseldorp 2019-05-13 10:34:40 UTC
Created attachment 15145 [details]
Cherry-pick of fix for 4.8, 4.9, and 4.10.next
Comment 5 Andreas Schneider 2019-05-13 13:42:27 UTC
Comment on attachment 15145 [details]
Cherry-pick of fix for 4.8, 4.9, and 4.10.next

LGTM
Comment 6 David Disseldorp 2019-05-13 20:34:27 UTC
(In reply to Andreas Schneider from comment #5)
> Comment on attachment 15145 [details]
> Cherry-pick of fix for 4.8, 4.9, and 4.10.next
> 
> LGTM

@Karo: please apply accordingly.
Comment 7 Karolin Seeger 2019-05-16 10:35:22 UTC
(In reply to David Disseldorp from comment #6)
Pushed to autobuild-v4-{10,9}-test.
Comment 8 Karolin Seeger 2019-05-21 10:56:42 UTC
(In reply to Karolin Seeger from comment #7)
Pushed to both branches.
Closing out bug report.

Thanks!