Bug 13938 - recvfile fails on streams
Summary: recvfile fails on streams
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services (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-09 11:00 UTC by Ralph Böhme
Modified: 2019-05-21 10:56 UTC (History)
2 users (show)

See Also:


Attachments
Disable recvfile on streams (770 bytes, patch)
2019-05-09 11:00 UTC, Ralph Böhme
no flags Details
git am fix for master for SMB1 (906 bytes, patch)
2019-05-09 19:55 UTC, Jeremy Allison
slow: review+
Details
Patch for 4.9 and 4.10 cherry-picked from master (2.27 KB, patch)
2019-05-10 10:36 UTC, Ralph Böhme
jra: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Ralph Böhme 2019-05-09 11:00:16 UTC
Created attachment 15135 [details]
Disable recvfile on streams

The recvfile implmentation relies on a proper system fd being available. Depending on the VFS module providing streams support, this assumption doesn't hold so client writes on a stream may file.

This has been seen in the wild with vfs_fruit + vfs_streams_xattr, "min receivefile size = 131072" in smb.conf and a Mac copying a file with a large Resource fork (AFP_AfpResource:$DATA stream).

I don't think it makes sense to enable recvfile on streams in the first place. The attached patch would disable it. Thoughts?
Comment 1 Jeremy Allison 2019-05-09 15:49:43 UTC
Oh, good catch ! I'll push.
Comment 2 Jeremy Allison 2019-05-09 18:24:18 UTC
Ah, no signed-off or bugid in the patch. I think that's trying to tell me something. I won't push until I get your OK with it :-).

It's clearly right though, so in your own time please !
Comment 3 Ralph Böhme 2019-05-09 19:30:29 UTC
Thanks! I wasn't entirely sure if I possibly missed some aspect, that's why I just asked for feedback and provided a WIP patch.

Fwiw, the patch fixed the issue at a customer.

Pushed to master. Thanks!
Comment 4 Jeremy Allison 2019-05-09 19:45:51 UTC
Hmmm. It's possible this might be a problem with SMB1 also, but I'm really trying to kill that :-). Let me check and see if there's anything we can do in an SMB1 case also - but that would be an additional patch.
Comment 5 Jeremy Allison 2019-05-09 19:52:37 UTC
Hold off on the push! There's an easy fix for SMB1 also. Sorry for not noticing this earlier.
Comment 6 Jeremy Allison 2019-05-09 19:55:15 UTC
Created attachment 15136 [details]
git am fix for master for SMB1

Ralph, here's the extra fix for SMB1. You can either include this in your patch, or push as an additional patch if you are OK with the review.

Sorry once again for missing this case.

Jeremy.
Comment 7 Ralph Böhme 2019-05-10 10:36:47 UTC
Created attachment 15140 [details]
Patch for 4.9 and 4.10 cherry-picked from master
Comment 8 Jeremy Allison 2019-05-10 17:16:16 UTC
Re-assigning to Karolin for inclusion in 4.10.next, 4.9.next.
Comment 9 Karolin Seeger 2019-05-16 10:32:24 UTC
(In reply to Jeremy Allison from comment #8)
Pushed to autobuild-v4-{10,9}-test.
Comment 10 Karolin Seeger 2019-05-21 10:56:01 UTC
(In reply to Karolin Seeger from comment #9)
Pushed to both branches.
Closing out bug report.

Thanks!