Bug 10429 - samba returns STATUS_OBJECT_NAME_NOT_FOUND when attempting to remove dangling symlink
Summary: samba returns STATUS_OBJECT_NAME_NOT_FOUND when attempting to remove dangling...
Status: RESOLVED FIXED
Alias: None
Product: Samba 4.1 and newer
Classification: Unclassified
Component: File services (show other bugs)
Version: 4.1.3
Hardware: All All
: P5 normal (vote)
Target Milestone: ---
Assignee: Karolin Seeger
QA Contact: Samba QA Contact
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-02-07 14:20 UTC by Jeff Layton
Modified: 2014-02-16 16:15 UTC (History)
0 users

See Also:


Attachments
capture showing successful lookup followed by OBJECT_NAME_NOT_FOUND error on unlink attempt (1.51 KB, application/cap)
2014-02-07 14:20 UTC, Jeff Layton
no flags Details
log from smbd -d10 -i while reproducing problem (373.39 KB, text/plain)
2014-02-07 14:36 UTC, Jeff Layton
no flags Details
smb.conf (1.22 KB, text/plain)
2014-02-07 17:32 UTC, Jeff Layton
no flags Details
git-am fix for master. (2.47 KB, patch)
2014-02-07 18:21 UTC, Jeremy Allison
jlayton: review+
Details
git-am fix for 4.1.next and 4.0.next including cherry-pick info from master. (2.69 KB, patch)
2014-02-08 00:12 UTC, Jeremy Allison
jlayton: review+
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Jeff Layton 2014-02-07 14:20:41 UTC
Created attachment 9655 [details]
capture showing successful lookup followed by OBJECT_NAME_NOT_FOUND error on unlink attempt

The connectathon test suite creates a bogus symlink:

$ ls -l /mnt/cifs/rawhide.test/
total 0
lrwxrwxrwx. 1 jlayton jlayton 19 Feb  7 09:11 file.0 -> /this/is/a/symlink0

...and then tries to remove it. When I do that, I get back STATUS_OBJECT_NAME_NOT_FOUND. The attached capture shows a successful lookup of the symlink and then an attempt to remove it which fails with that error.

My suspicion is that the POSIX unlink code is doing a stat() instead of an lstat() and getting back -ENOENT, which it then translates into this error. Let me know if you need other info (or whether this might already be fixed in a more recent release).

FWIW, I'm running: samba-4.1.3-2.fc20.x86_64
Comment 1 Jeff Layton 2014-02-07 14:36:52 UTC
Created attachment 9657 [details]
log from smbd -d10 -i while reproducing problem
Comment 2 Jeff Layton 2014-02-07 15:20:53 UTC
FWIW, I was able to reproduce this with the current tip of the samba tree, so it doesn't appear to be fixed already.
Comment 3 Jeremy Allison 2014-02-07 17:05:33 UTC
Hmmm. In the UNIX extensions codepath we have..

                if (INFO_LEVEL_IS_UNIX(info_level)) {
                        /*
                         * For CIFS UNIX extensions the target name may not exist.
                         */

                        /* Always do lstat for UNIX calls. */
                        SMB_VFS_LSTAT(conn, smb_fname);


which should do this correctly. I'll investigate.

Jeremy.
Comment 4 Jeremy Allison 2014-02-07 17:27:27 UTC
From your log we have the first instance of NT_STATUS_OBJECT_NAME_NOT_FOUND here:

>close_remove_share_mode: file rawhide.test/file.0. Delete on close was set - deleting file.
>find_delete_on_close_token: name_hash = 0xf2ac662e
>find__delete_on_close_token: dt->name_hash = 0xf2ac662e
>vfs_streaminfo failed: NT_STATUS_OBJECT_NAME_NOT_FOUND
>delete_all_streams failed: NT_STATUS_OBJECT_NAME_NOT_FOUND

So it's inside vfs_streaminfo.

The code here in vfs_default.c looks like:

static NTSTATUS vfswrap_streaminfo(vfs_handle_struct *handle,
                                   struct files_struct *fsp,
                                   const char *fname,
                                   TALLOC_CTX *mem_ctx,
                                   unsigned int *pnum_streams,
                                   struct stream_struct **pstreams)
{
        SMB_STRUCT_STAT sbuf;
        struct stream_struct *tmp_streams = NULL;
        int ret;

        if ((fsp != NULL) && (fsp->is_directory)) {
                /*
                 * No default streams on directories
                 */
                goto done;
        }

        if ((fsp != NULL) && (fsp->fh->fd != -1)) {
                ret = SMB_VFS_FSTAT(fsp, &sbuf);
        }
        else {
                struct smb_filename smb_fname;

                ZERO_STRUCT(smb_fname);
                smb_fname.base_name = discard_const_p(char, fname);

                if (lp_posix_pathnames()) {
                        ret = SMB_VFS_LSTAT(handle->conn, &smb_fname);
                } else {
                        ret = SMB_VFS_STAT(handle->conn, &smb_fname);
                }
                sbuf = smb_fname.st;
        }

Which should handle this case correctly once CIFS_UNIX_POSIX_PATHNAMES_CAP is negotiated. I'm assuming you're doing this (although I'll check in the log :-).

Do you have any modules loaded on this share ? What is your smb.conf ?
Comment 5 Jeff Layton 2014-02-07 17:32:13 UTC
Created attachment 9662 [details]
smb.conf

Here's my smb.conf -- it's nothing too exotic.

I'm pretty sure that cifs sets CIFS_UNIX_POSIX_PATHNAMES_CAP...
Comment 6 Jeff Layton 2014-02-07 17:39:14 UTC
Hrm, no it doesn't seem to set that anywhere, but I don't think the client ever sends a call that would have that set. The only place we seem to consider it is in the response to a SMB_QUERY_CIFS_UNIX_INFO call (basically, the UNIX fsinfo call)...
Comment 7 Jeff Layton 2014-02-07 17:49:48 UTC
Ok it does send it. What the client does is take the caps that the server sends, apply a mask and then send them back. Weird.

In any case, the caps sent back are 0x1db, which has CIFS_UNIX_POSIX_PATHNAMES_CAP set. It does not have CIFS_UNIX_POSIX_PATH_OPS_CAP set however. Could that be the problem?
Comment 8 Jeremy Allison 2014-02-07 18:01:41 UTC
Ah, but you have:

vfs objects = streams_xattr

in your [export] share. Bet that's it...
Comment 9 Jeremy Allison 2014-02-07 18:08:54 UTC
Yep, got it. The problem is the stream_xattr code calls get_ea_names_from_file() which has the code:

                if (fsp && fsp->fh->fd != -1) {
                        sizeret = SMB_VFS_FLISTXATTR(fsp, ea_namelist,
                                                     ea_namelist_size);
                } else {
                        sizeret = SMB_VFS_LISTXATTR(conn, fname, ea_namelist,
                                                    ea_namelist_size);
                }

because we don't have a SMB_VFS_LLISTXATTR call in our VFS. (I do have a patch for this but it's not finished yet).

The stream_xattr code doesn't cope with that. Fix to follow shortly.
Comment 10 Jeremy Allison 2014-02-07 18:21:33 UTC
Created attachment 9663 [details]
git-am fix for master.

Jeff, can you confirm it fixes it and add your 'Reviewed-by:' tag and I'll push to master and we'll get this into 4.0.next, 4.1.next.

Cheers,

Jeremy.
Comment 11 Jeff Layton 2014-02-07 18:46:15 UTC
Comment on attachment 9663 [details]
git-am fix for master.

Yep, that fixed it! Thanks!
Comment 12 Jeff Layton 2014-02-07 18:47:19 UTC
Erm...I'll leave you to add:

Reviewed-by: Jeff Layton <jlayton@samba.org>

...or maybe "Tested-by" would be better since I don't know this code well enough to do a proper review...
Comment 13 Jeremy Allison 2014-02-08 00:12:57 UTC
Created attachment 9667 [details]
git-am fix for 4.1.next and 4.0.next including cherry-pick info from master.

This is exactly the same fix Jeff +1'ed and went into master, just including the git cherry-pick info for 4.1.next and 4.0.next.

Jeremy.
Comment 14 Jeremy Allison 2014-02-08 00:13:30 UTC
Re-assigning to Karolin for inclusion in 4.1.next, 4.0.next.

Jeremy.
Comment 15 Jeff Layton 2014-02-08 00:47:58 UTC
Comment on attachment 9667 [details]
git-am fix for 4.1.next and 4.0.next including cherry-pick info from master.

Looks like the same patch...
Comment 16 Karolin Seeger 2014-02-14 19:34:24 UTC
Pushed to autobuild-v4-1-test and autobuild-v4-0-test.
Comment 17 Karolin Seeger 2014-02-16 16:15:22 UTC
Pushed to v4-1-test and v4-0-test.
Closing out bug report.

Thanks!