Bug 11125 - smbd crashes in vfs_fruit
Summary: smbd crashes in vfs_fruit
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: VFS Modules (show other bugs)
Version: unspecified
Hardware: x86 Linux
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-02-27 12:03 UTC by Richard Baker
Modified: 2015-04-09 18:59 UTC (History)
3 users (show)

See Also:


Attachments
Log file and gdb backtrace snippets/ (7.98 KB, text/plain)
2015-02-27 12:03 UTC, Richard Baker
no flags Details
"._people alias" file (3.72 KB, application/octet-stream)
2015-03-02 12:27 UTC, Richard Baker
no flags Details
Fix handling of broken AppleDouble files (4.54 KB, patch)
2015-03-02 18:08 UTC, Ralph Böhme
no flags Details
Fix handling of broken AppleDouble files (4.55 KB, patch)
2015-03-25 14:15 UTC, Ralph Böhme
no flags Details
Fix handling of broken AppleDouble files (4.69 KB, patch)
2015-03-25 14:33 UTC, Ralph Böhme
vl: review+
Details
cherry-picked commit for 4.2 that went into master (5.01 KB, patch)
2015-03-26 12:34 UTC, Ralph Böhme
vl: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Richard Baker 2015-02-27 12:03:09 UTC
Created attachment 10801 [details]
Log file and gdb backtrace snippets/

We have a network here of about 20 Macs accessing a Samba server, a trial of 4.2.0rc5 resulted in a couple
of the Mac clients causing the smbd deamon to crash. I'm afraid I can't find any commonality between the clients
(one was Mac OS X 10.10 the other 10.9). Nor have I been able to ascertain which application on the client
was accessing the share at the time.

The server is a Debian GNU/Linux 6.0 system running kernel 2.6.32-5-686-bigmem.

The samba install was a compile on the server of the 4.2.0rc5 source using the following configure command:

./configure  --sbindir=/usr/local/samba42/sbin --prefix=/usr/local/samba42 --sysconfdir=/etc/samba42 --libdir=/usr/local/lib/samba42 --with-privatedir=/etc/samba42 --with-piddir=/var/run/samba --localstatedir=/var  --with-pammodulesdir=/lib/security --with-pam --with-syslog --with-utmp  --with-pam_smbpass  --with-winbind --with-shared-modules=idmap_rid,idmap_ad,idmap_adex,idmap_hash,idmap_ldap,idmap_tdb2 --with-automount --with-ldap --with-ads --with-dnsupdate --with-modulesdir=/usr/local/lib/samba42 --datarootdir=/usr/local/share --datadir=/usr/local/share/samba42 --with-lockdir=/var/run/samba --with-statedir=/var/lib/samba42 --with-cachedir=/var/cache/samba42 --disable-avahi --with-acl-support --with-quotas 

I've attached the relevant snippets from the log and a copy of the gdb backtrace from the core file.

According to a Wireshark dump the last packet from the client before the connection was reset was:

"Create Request File: AMEX\AM4162 Umissable Hotel Sale\PM;Find Request SMB2_FIND_ID_BOTH_DIRECTORY_INFO Pattern: Usefull;Close Request"

The share for that request has the following config:

[projects]
        comment = shared projects
        path = /mnt/raid/hoard/work
        public = no
        writeable = yes
        write list = @smbusers
        browseable = yes
        printable = no
        guest ok = no
        force user = hoard
        force group = smbusers
        create mode = 0665
        directory mode = 0775
        hide dot files = no
        vfs objects = catia fruit streams_xattr
        fruit:resource = file
        fruit:metadata = netatalk
        fruit:locking = netatalk
        fruit:encoding = native

and the global section contains:

fruit:aapl = yes

I'm afraid I'm not in a position to be able to reproduce this without annoying the users in question and apologies
if this isn't enough info to work with,

Thanks

Richard
Comment 1 Ralph Böhme 2015-02-27 15:34:59 UTC
Can you reconfigure/recompile with --enable-debug please? A SBT with debug symbols should be more telling where it goes off.
Comment 2 Richard Baker 2015-03-02 09:27:27 UTC
Morning, one gdb backtrace with '--enable-debug' added to the daemon build config:

fs3:/var/samba42/cores/smbd# gdb /usr/local/samba42/sbin/smbd core
GNU gdb (GDB) 7.3-debian
Copyright (C) 2011 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show copying"
and "show warranty" for details.
This GDB was configured as "i486-linux-gnu".
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>...
Reading symbols from /usr/local/samba42/sbin/smbd...done.
[New LWP 5292]

warning: Can't read pathname for load map: Input/output error.
[Thread debugging using libthread_db enabled]
Core was generated by `/usr/local/samba42/sbin/smbd -D'.
Program terminated with signal 6, Aborted.
#0  0xb77bd424 in __kernel_vsyscall ()
(gdb) bt
#0  0xb77bd424 in __kernel_vsyscall ()
#1  0xb6a85781 in *__GI_raise (sig=6) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64
#2  0xb6a88bb2 in *__GI_abort () at abort.c:92
#3  0xb70a44e0 in dump_core () at ../source3/lib/dumpcore.c:337
#4  0xb7091793 in smb_panic_s3 (why=0xb77966bd "internal error") at ../source3/lib/util.c:811
#5  0xb778f8e5 in smb_panic (why=0xb77966bd "internal error") at ../lib/util/fault.c:166
#6  0xb778f5af in fault_report (sig=7) at ../lib/util/fault.c:83
#7  0xb778f5c0 in sig_fault (sig=7) at ../lib/util/fault.c:94
#8  <signal handler called>
#9  memmove () at ../sysdeps/i386/i686/memmove.S:68
#10 0xb5985105 in ad_convert (ad=0xb8fb5c38, fd=42) at ../source3/modules/vfs_fruit.c:661
#11 0xb5985a7d in ad_header_read_rsrc (ad=0xb8fb5c38, path=0xb8fb5e68 "webdev.tui.co.uk/html/people alias") at ../source3/modules/vfs_fruit.c:871
#12 0xb5985d3c in ad_read (ad=0xb8fb5c38, path=0xb8fb5e68 "webdev.tui.co.uk/html/people alias") at ../source3/modules/vfs_fruit.c:938
#13 0xb5986278 in ad_get (ctx=0xb8f7a0b8, handle=0xb8f4b200, path=0xb8fb5e68 "webdev.tui.co.uk/html/people alias", type=ADOUBLE_RSRC) at ../source3/modules/vfs_fruit.c:1110
#14 0xb5988db4 in readdir_attr_macmeta (handle=0xb8f4b200, smb_fname=0xb8fb5db8, attr_data=0xb8fb5be0) at ../source3/modules/vfs_fruit.c:1839
#15 0xb598d02c in fruit_readdir_attr (handle=0xb8f4b200, fname=0xb8fb5db8, mem_ctx=0xb8f57928, pattr_data=0xbfd78484) at ../source3/modules/vfs_fruit.c:3224
#16 0xb7534fa1 in smb_vfs_call_readdir_attr (handle=0xb8f4b200, fname=0xb8fb5db8, mem_ctx=0xb8f57928, attr_data=0xbfd78484) at ../source3/smbd/vfs.c:2473
#17 0xb74f81d4 in smbd_marshall_dir_entry (ctx=0xb8f57928, conn=0xb8f48d78, flags2=49217, info_level=262, name_list=0x0, check_mangled_names=true, requires_resume_key=false, mode=128, 
    fname=0xb8faa078 "people alias", smb_fname=0xb8fb5db8, space_remaining=998220, align=8 '\b', do_pad=false, base_data=0xb55b2038 "p", ppdata=0xbfd78700, end_data=0xb56b302f "", 
    last_entry_off=0xbfd78590) at ../source3/smbd/trans2.c:1643
#18 0xb74fb5db in smbd_dirptr_lanman2_entry (ctx=0xb8f57928, conn=0xb8f48d78, dirptr=0xb8f77148, flags2=49217, path_mask=0xb8f784c0 "*", dirtype=22, info_level=262, requires_resume_key=0, 
    dont_descend=false, ask_sharemode=true, align=8 '\b', do_pad=false, ppdata=0xbfd78700, base_data=0xb55b2038 "p", end_data=0xb56b302f "", space_remaining=998220, got_exact_match=0xbfd786c0, 
    _last_entry_off=0xbfd786f4, name_list=0x0) at ../source3/smbd/trans2.c:2362
#19 0xb7584137 in smbd_smb2_find_send (mem_ctx=0xb8f896d0, ev=0xb8f221f0, smb2req=0xb8f896d0, fsp=0xb8f88f00, in_file_info_class=37 '%', in_flags=1 '\001', in_file_index=0, 
    in_output_buffer_length=1048568, in_file_name=0xb8f784c0 "*") at ../source3/smbd/smb2_find.c:440
#20 0xb7583068 in smbd_smb2_request_process_find (req=0xb8f896d0) at ../source3/smbd/smb2_find.c:123
#21 0xb7568603 in smbd_smb2_request_dispatch (req=0xb8f896d0) at ../source3/smbd/smb2_server.c:2304
#22 0xb756926b in smbd_smb2_request_dispatch_immediate (ctx=0xb8f221f0, im=0x0, private_data=0xb8f896d0) at ../source3/smbd/smb2_server.c:2560
#23 0xb73de525 in tevent_common_loop_immediate (ev=0xb8f221f0) at ../lib/tevent/tevent_immediate.c:135
#24 0xb70aeea9 in run_events_poll (ev=0xb8f221f0, pollrtn=0, pfds=0x0, num_pfds=0) at ../source3/lib/events.c:191
#25 0xb70af487 in s3_event_loop_once (ev=0xb8f221f0, location=0xb76df6c4 "../source3/smbd/process.c:3992") at ../source3/lib/events.c:303
#26 0xb73dd8cf in _tevent_loop_once (ev=0xb8f221f0, location=0xb76df6c4 "../source3/smbd/process.c:3992") at ../lib/tevent/tevent.c:530
#27 0xb73ddb2f in tevent_common_loop_wait (ev=0xb8f221f0, location=0xb76df6c4 "../source3/smbd/process.c:3992") at ../lib/tevent/tevent.c:634
#28 0xb73ddbe1 in _tevent_loop_wait (ev=0xb8f221f0, location=0xb76df6c4 "../source3/smbd/process.c:3992") at ../lib/tevent/tevent.c:653
#29 0xb754eac0 in smbd_process (ev_ctx=0xb8f221f0, msg_ctx=0xb8f22270, sock_fd=36, interactive=false) at ../source3/smbd/process.c:3992
#30 0xb77e4fd4 in smbd_accept_connection (ev=0xb8f221f0, fde=0xb8f3cfc8, flags=1, private_data=0xb8f3a0c0) at ../source3/smbd/server.c:627
#31 0xb70af2e0 in run_events_poll (ev=0xb8f221f0, pollrtn=1, pfds=0xb8f33988, num_pfds=4) at ../source3/lib/events.c:257
#32 0xb70af5ab in s3_event_loop_once (ev=0xb8f221f0, location=0xb77e9be0 "../source3/smbd/server.c:985") at ../source3/lib/events.c:326
#33 0xb73dd8cf in _tevent_loop_once (ev=0xb8f221f0, location=0xb77e9be0 "../source3/smbd/server.c:985") at ../lib/tevent/tevent.c:530
#34 0xb73ddb2f in tevent_common_loop_wait (ev=0xb8f221f0, location=0xb77e9be0 "../source3/smbd/server.c:985") at ../lib/tevent/tevent.c:634
#35 0xb73ddbe1 in _tevent_loop_wait (ev=0xb8f221f0, location=0xb77e9be0 "../source3/smbd/server.c:985") at ../lib/tevent/tevent.c:653
#36 0xb77e5e02 in smbd_parent_loop (ev_ctx=0xb8f221f0, parent=0xb8f22a00) at ../source3/smbd/server.c:985
#37 0xb77e7765 in main (argc=2, argv=0xbfd7a0f4) at ../source3/smbd/server.c:1626
Comment 3 Ralph Böhme 2015-03-02 12:01:56 UTC
Is there a file "webdev.tui.co.uk/html/._people alias" ? If so, please attach a copy to this PR.
Comment 4 Richard Baker 2015-03-02 12:27:11 UTC
Created attachment 10803 [details]
"._people alias" file

._people alias file.
Comment 5 Ralph Böhme 2015-03-02 14:55:21 UTC
Thanks! I can reproduce the crash with that AppleDouble file. Looks like it's a generic crash that will happen with *any* AppleDouble file created by OS X when accessing a Samba share without vfs_fruit (or any other named strems supporting VFS modules backend), iow: often!

The crash happens when the AppleDouble converting functions calls memmove with parameters which look totally ok. For the life of me, I wasn't yet able to figure out what's wrong in the code path. Stay tuned...
Comment 6 Ralph Böhme 2015-03-02 18:08:52 UTC
Created attachment 10806 [details]
Fix handling of broken AppleDouble files

Can you please test this patch?
Comment 7 Richard Baker 2015-03-03 11:13:18 UTC
Ok,

Patched added. I no longer get a samba crash when I mount that share.

I'll get a few more people here to test it over the next few days.

Thanks

Richard
Comment 8 Ralph Böhme 2015-03-24 10:11:16 UTC
(In reply to Richard Baker from comment #7)
How is the testing going?
Comment 9 Richard Baker 2015-03-24 16:54:04 UTC
I'm afraid I had to stop testing. As far as I'm aware the problem originally reported 
is fixed, but I was getting other problems. 

Unfortunately we are a small company and I haven't had the time to experiment. My
feeling is that the vfs_fruit code isn't ready for production yet BUT Mac OS X
and particularly Mac Finder are not great when it comes to samba mounts so it's
difficult to diagnose problems.

Thanks

Richard
Comment 10 Volker Lendecke 2015-03-25 13:10:41 UTC
Comment on attachment 10806 [details]
Fix handling of broken AppleDouble files

I believe this is incomplete. In

if (off + len > filesize) {

the "off+len" could wrap so that the end result is smaller than filesize. Or am I not seeing the check?
Comment 11 Ralph Böhme 2015-03-25 14:06:27 UTC
(In reply to Volker Lendecke from comment #10)
Probably. filesize is a size_t, I'll add explicit (size_t) casts to off and len. That way the off+len calculation won't wrap.
Comment 12 Ralph Böhme 2015-03-25 14:15:17 UTC
Created attachment 10908 [details]
Fix handling of broken AppleDouble files
Comment 13 Volker Lendecke 2015-03-25 14:16:12 UTC
(In reply to Ralph Böhme from comment #11)

What about 32-bit platforms? Can't size_t+size_t still wrap?
Comment 14 Ralph Böhme 2015-03-25 14:17:54 UTC
D'oh!
Comment 15 Volker Lendecke 2015-03-25 14:18:49 UTC
Comment on attachment 10908 [details]
Fix handling of broken AppleDouble files

Someone else please ack this. I still believe that on 32-bit platforms this can wrap. From my understanding the only way to be safe is to explicitly check each addition.
Comment 16 Ralph Böhme 2015-03-25 14:31:35 UTC
(In reply to Volker Lendecke from comment #15)
"D'oh" meant to express, "I'm totally stupid". Your totally right, I'll attach an updated patch in a second.
Comment 17 Ralph Böhme 2015-03-25 14:33:49 UTC
Created attachment 10909 [details]
Fix handling of broken AppleDouble files
Comment 18 Volker Lendecke 2015-03-25 15:51:31 UTC
Comment on attachment 10909 [details]
Fix handling of broken AppleDouble files

Thanks, that's what I meant. Please also push to master with my reviewed-by: tag. You might fix some comment typos before ("other then" and "FiderInfo" look wrong), which won't invalidate my R-B :-)
Comment 19 Ralph Böhme 2015-03-26 12:34:25 UTC
Created attachment 10913 [details]
cherry-picked commit for 4.2 that went into master
Comment 20 Ralph Böhme 2015-03-26 12:41:48 UTC
Reassigning to Karolin for inclusion in 4.2.1.
Comment 21 Karolin Seeger 2015-03-27 20:21:17 UTC
Pushed to autobuild-v4-2-test.
Comment 22 Karolin Seeger 2015-04-09 18:59:27 UTC
Pushed to v4-2-test.
Closing out bug report.

Thanks!