Description of problem: gvfs return permission denied calling `SMBC_getatr` when file not exists. Investigating, apparently `SMBC_getatr` no longer uses `errno` but instead returns `NTSTATUS`: https://gitlab.com/samba-team/samba/-/commit/d4f8fdd69a1278d8473d541dc6b32014a369bcd0 https://github.com/samba-team/samba/commit/d4f8fdd69a1278d8473d541dc6b32014a369bcd0 Reverting the previous commit fixes the bug. I rebuilt the Fedora 37 Samba SRPM and added the (reverse) patch to SPEC and everything works perfect: https://koji.fedoraproject.org/koji/buildinfo?buildID=2062333 $ git format-patch -1 --stdout d4f8fdd69a1278d8473d541dc6b32014a369bcd0 > 0001-libsmb-Convert-SMBC_getatr-to-NTSTATUS.patch 0001-libsmb-Convert-SMBC_getatr-to-NTSTATUS.patch ... Patch100: 0001-libsmb-Convert-SMBC_getatr-to-NTSTATUS.patch ... %prep xzcat %{SOURCE0} | gpgv2 --quiet --keyring %{SOURCE2} %{SOURCE1} - %autosetup -n samba-%{version}%{pre_release} -N %autopatch -M 99 %patch100 -p1 -R ... Version-Release number of selected component (if applicable): $ rpm -qa gvfs-* gvfs-client-1.50.2-2.fc37.x86_64 gvfs-afc-1.50.2-2.fc37.x86_64 gvfs-fuse-1.50.2-2.fc37.x86_64 gvfs-goa-1.50.2-2.fc37.x86_64 gvfs-afp-1.50.2-2.fc37.x86_64 gvfs-archive-1.50.2-2.fc37.x86_64 gvfs-gphoto2-1.50.2-2.fc37.x86_64 gvfs-mtp-1.50.2-2.fc37.x86_64 gvfs-smb-1.50.2-2.fc37.x86_64 $ rpm -qa gvfs-smb samba-* libsmbclient* gvfs-smb-1.50.2-2.fc37.x86_64 samba-common-4.17.0-1.fc37.noarch samba-client-libs-4.17.0-1.fc37.x86_64 samba-common-libs-4.17.0-1.fc37.x86_64 libsmbclient-4.17.0-1.fc37.x86_64 samba-client-4.17.0-1.fc37.x86_64 $ rpm -q glib2 glib2-2.73.3-3.fc37.x86_64 Steps to Reproduce: $ gio mount -u "smb://bar@SERVER/foo/" $ gio mount "smb://bar@SERVER/foo/" $ date | tee /run/user/1000/gvfs/smb-share\:server\=SERVER\,share\=foo\,user\=bar/folder/another-test.txt tee: '/run/user/1000/gvfs/smb-share:server=192.168.44.1,share=backups,user=bar/folder/another-test.txt': Permission denied Thu Sep 15 03:55:20 PM -05 2022 $ echo $? 1 Additional info: Log messages in `GVFS_SMB_DEBUG=10 GVFS_DEBUG=1 /usr/libexec/gvfsd --replace`: smb: backend_dbus_handler org.gtk.vfs.Mount:QueryInfo (pid=4762) smb: Queued new job 0x7f0b10006cb0 (GVfsJobQueryInfo) smbc_stat(smb://SERVER/foo/folder) smb: backend_dbus_handler org.gtk.vfs.Mount:QueryInfo (pid=4762) smb: Queued new job 0x7f0b10006b70 (GVfsJobQueryInfo) SMBC_getatr: sending qpathinfo smb: send_reply(0x7f0b10006cb0), failed=0 () smbc_stat(smb://SERVER/foo/folder) SMBC_getatr: sending qpathinfo smb: send_reply(0x7f0b10006b70), failed=0 () smb: backend_dbus_handler org.gtk.vfs.Mount:QueryInfo (pid=4762) smb: Queued new job 0x7f0b10002400 (GVfsJobQueryInfo) smbc_stat(smb://SERVER/foo/folder/another-test.txt) SMBC_getatr: sending qpathinfo map_errno_from_nt_status: 32 bit codes: code=c0000022 cli_status_to_errno: 0xc0000022 -> 13 smb: send_reply(0x7f0b10002400), failed=1 (Permission denied) Red Hat Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2127301 Related gvfs issues: https://gitlab.gnome.org/GNOME/gvfs/-/issues/649 https://gitlab.gnome.org/GNOME/gvfs/-/issues/651
I think I see the problem. Reverting the patch isn't the correct thing to do. In SMBC_getatr() the code does: status = cli_resolve_path(frame, "", creds, srv->cli, fixedpath, &targetcli, &targetpath); if (!NT_STATUS_IS_OK(status)) { d_printf("Couldn't resolve %s\n", path); TALLOC_FREE(frame); return NT_STATUS_OBJECT_NAME_NOT_FOUND; } if (!srv->no_pathinfo2) { status = cli_qpathinfo2(targetcli, targetpath, &create_time_ts, &access_time_ts, &write_time_ts, &change_time_ts, &size, &attr, &ino); if (NT_STATUS_IS_OK(status)) { goto setup_stat; } } if (!srv->no_pathinfo3) { status = cli_qpathinfo3(targetcli, targetpath, &create_time_ts, &access_time_ts, &write_time_ts, &change_time_ts, &size, &attr, &ino); if (NT_STATUS_IS_OK(status)) { goto setup_stat; } } ... all_failed: srv->no_pathinfo2 = False; srv->no_pathinfo3 = False; TALLOC_FREE(frame); return NT_STATUS_ACCESS_DENIED; Internally to cli_resolve_path() if we get NT_STATUS_OBJECT_NAME_NOT_FOUND it maps to NT_STATUS_OK (we might be trying to check if a path is valid to create). Currently we just goto success if either of these calls succeeds, and fall through to the next call on any error. But we should only fall through to the next attempt if we get NT_STATUS_INVALID_LEVEL or NT_STATUS_NOT_SUPPORTED. For any other case we should just return the error value returned by the call. Currently as both cli_qpathinfo2() and cli_qpathinfo3() are returning NT_STATUS_OBJECT_NOT_FOUND we're falling through to the 'all_failed:' exit, which arbitarily sets NT_STATUS_ACCESS_DENIED as the error code.
Created attachment 17546 [details] "Raw" patch for master I think this is the correct fix. I'll add a test later and produce an MR for this. If you can test it in the meantime I'd appreciate it ! Thanks, Jeremy.
(In reply to Jeremy Allison from comment #2) Hi Jeremy. I have rebuilt the Fedora Samba SRPM version 4.17.0 by applying your patch and it fixes the issue. Thank you very much.
Hi Jeremy, can you please create a MR for this? I would like to get the fix to Fedora 37. Thanks you, Pavel
I'm still out with COVID. I'm planning to create an MR for this on Monday. It needs a test adding to one of our libsmbclient tests (shouldn't be too hard to do, I'm just not in a good shape to do it yet). When does Fedora37 freeze ?
Running ci on MR with test at: https://gitlab.com/samba-team/devel/samba/-/pipelines/669177604
Pavel, here is the MR for your review. https://gitlab.com/samba-team/samba/-/merge_requests/2756
This bug was referenced in samba master: 9eda432836bfff3d3d4a365a08a5ecb54f0f2e34 fd0c01da1c744ae6fd9d8675616d8b6d3531e469
Created attachment 17583 [details] patch for 4.17
Comment on attachment 17583 [details] patch for 4.17 lgtm
Jule, please apply the patch to the relevant branch. Thanks!
Pushed to autobuild-v4-17-test.
This bug was referenced in samba v4-17-test: 09ec2b13e7cccb0beeac6c87a9acd6ea5537d8ed 142a771d85463216075913695d84530c6cb4ff9e
Closing out bug report. Thanks!
This bug was referenced in samba v4-17-stable (Release samba-4.17.1): 09ec2b13e7cccb0beeac6c87a9acd6ea5537d8ed 142a771d85463216075913695d84530c6cb4ff9e
Created attachment 17585 [details] git-am fix for 4.16.next. Back-port for 4.16.next.
Created attachment 17603 [details] git-am fix for 4.16.next Backported from master. Fixed version - actually removes knownfail.
Comment on attachment 17603 [details] git-am fix for 4.16.next Hi Jeremy, the patch looks good, just two tiny comments: 1) According to https://wiki.samba.org/index.php/Samba_Release_Planning#Patch_Process since it is not a clean cherry pick, this should be changed (cherry picked from commit fd0c01da1c744ae6fd9d8675616d8b6d3531e469) to (backported from commit fd0c01da1c744ae6fd9d8675616d8b6d3531e469) and add a terse oneliner that explains the conflict on the next line using the following format: [your@mail: reason]. 2) there is a mixture of 'false' and 'False' in SMBC_getatr(), should we keep it consistent?
(In reply to Pavel Filipenský from comment #18) Fixed the "Cherry-picked" -> "Back-ported". No, we shouldn't change "false" -> "False". As we change/add new code we should always move towards standards based types. Consistency isn't so important. I'll update a new version. Thanks !
Created attachment 17604 [details] git-am fix for 4.16.next. Fixed version.
Comment on attachment 17604 [details] git-am fix for 4.16.next. LGTM
Hi Jule, if you could merge this for 4.16.next I'd appreciate it - thanks !
Pushed to autobuild-v4-16-test.
This bug was referenced in samba v4-16-test: efa48817d3c6fd3c64051bdf29648dff1702cf5d 618395a7eafbb6224047610659c2d343318a1d33
I'm still running into problems with an issue I believe is related to this bug. I believe many android file managers are connecting to Samba via GVFS. They are no longer able to delete multiple files (or dirs) and it appears the problem started with 4.17.0. Deleting single files works, but returns an access denied message. No change in 4.17.2. I'm not sure how to capture client side communications but here's the server side log: Nov 16 02:43:00 Hippo smbd[1668]: [2022/11/16 02:43:00.249990, 0] ../../source3/smbd/server.c:1741(main) Nov 16 02:43:00 Hippo smbd[1668]: smbd version 4.17.2 started. Nov 16 02:43:00 Hippo smbd[1668]: Copyright Andrew Tridgell and the Samba Team 1992-2022 Nov 16 02:43:00 Hippo nmbd[1673]: [2022/11/16 02:43:00.283325, 0] ../../source3/nmbd/nmbd.c:901(main) Nov 16 02:43:00 Hippo nmbd[1673]: nmbd version 4.17.2 started. Nov 16 02:43:00 Hippo nmbd[1673]: Copyright Andrew Tridgell and the Samba Team 1992-2022 ... Nov 17 00:15:38 Hippo smbd[22786]: [2022/11/17 00:15:38.152578, 2] ../../source3/smbd/open.c:1678(open_file) Nov 17 00:15:38 Hippo smbd[22786]: admin opened file SERVER/FOO/file.mkv read=No write=No (numopen=2) Nov 17 00:15:38 Hippo smbd[22786]: [2022/11/17 00:15:38.167282, 3] ../../source3/smbd/smb2_trans2.c:6834(smbd_do_setfilepathinfo) Nov 17 00:15:38 Hippo smbd[22786]: smbd_do_setfilepathinfo: file SERVER/FOO/file.mkv (fnum 1321203594) info_level=1013 totdata=1 Nov 17 00:15:38 Hippo smbd[22786]: [2022/11/17 00:15:38.175453, 2] ../../source3/smbd/close.c:830(close_normal_file) Nov 17 00:15:38 Hippo smbd[22786]: admin closed file file SERVER/FOO/file.mkv (numopen=0) NT_STATUS_ACCESS_DENIED Nov 17 00:15:38 Hippo smbd[22786]: smbd_smb2_request_error_ex: smbd_smb2_request_error_ex: idx[1] status[NT_STATUS_ACCESS_DENIED] || at ../../source3/smbd/smb2_close.c:111 status[NT_STATUS_ACCESS_DENIED]Nov 17 00:15:38 Hippo smbd[22786]: [2022/11/17 00:15:38.175530, 3] ../../source3/smbd/smb2_server.c:3955(smbd_smb2_request_error_ex)
Can you get me a wireshark trace of this failing please ? Also, open a new bug and upload there as this one is resolved.
This bug was referenced in samba v4-16-stable (Release samba-4.16.8): efa48817d3c6fd3c64051bdf29648dff1702cf5d 618395a7eafbb6224047610659c2d343318a1d33