https://gitlab.com/samba-team/samba/-/commit/f6550e804ea5604b1d7ec67e629fad8baee87f8b adds a `renameat2()` replacement to lib/replace. On OpenBSD `__NR_renameat2` will be undefined as it is not Linux leaving a function that looks like this.

```
int rep_renameat2(int __oldfd, const char *__old, int __newfd,
    const char *__new, unsigned int __flags)
{
  if (__flags != 0) {

     errno = EINVAL;
     return -1;
  }

  return renameat(__oldfd, __old, __newfd, __new);
}
```

The how flags are set so the Linux renameat2 syscall is meant to run. The  return `renameat()` is never reached.

Proposed patch is attached to this ticket.

Created attachment 18567 [details]
Patch rep_renameat2() in lib/replace/replace.c

(In reply to Bjorn Ketelaars from comment #1)

The replacement is correct it should return an error
if flags is not 0 !

The caller needs to handle that, also for older linux kernels.

(In reply to Bjorn Ketelaars from comment #0)

It seems that renameat() on OpenBSD is not able to replace
an existing directory (at least if it has 000 permissions).

https://man.openbsd.org/rename.2 says EACCES can happen for
these cases:
[EACCES]
    A component of either path prefix denies search permission.
[EACCES]
    The requested change requires writing in a directory that denies write permission.
[EACCES]
    The from argument is a directory and denies write permission.
EACCES]
    The from or to argument specifies a relative path but search permission is denied for the directory which the fromfd or tofd file descriptor, respectively, references.

What are the permissions of the parent directory?

Would it possible for you to provide level 10 logs,
see https://wiki.samba.org/index.php/Client_specific_logging for some hints.

As well as the output of something like strace, I guess openbsd would have to use ktrace/kdump. What we need a verbose listing of systemcalls and their arguments and return values. (Over the time where smbclient does the 'mkdir xx'.

Replacing a 000 directory seems to work for me on Linux:

#include <stdio.h>
#include <sys/stat.h>
#include <fcntl.h>

int main(int argc, char **argv)
{
        int ret;

        if (argc < 3) {
                printf("Usage: %s SRC DST\n", argv[0]);
                return 1;
        }

        ret = renameat(AT_FDCWD, argv[1], AT_FDCWD, argv[2]);
        if (ret != 0) {
                return 1;
        }

        return 0;
}

$ rmdir /srv/samba/test/dir/*                  
$ mkdir /srv/samba/test/dir/a
$ mkdir /srv/samba/test/dir/b             
$ chmod 0000 /srv/samba/test/dir/b
$ strace -v ./rename /srv/samba/test/dir/a /srv/samba/test/dir/b 2>&1 | grep renameat
renameat(AT_FDCWD, "/srv/samba/test/dir/a", AT_FDCWD, "/srv/samba/test/dir/b") = 0

Created attachment 18590 [details]
output ktrace/kdump

Created attachment 18591 [details]
log.smbd

(In reply to Stefan Metzmacher from comment #4)

> What are the permissions of the parent directory?
777

> Would it possible for you to provide level 10 logs,
Output of ktrace/kdump as well as log.smbd have been attached to this ticket.

(In reply to Stefan Metzmacher from comment #4)

> It seems that renameat() on OpenBSD is not able to replace an existing directory (at least if it has 000 permissions).

You are correct. The directory that is going to be replaced by mkdirat() needs to be writeable. As OpenBSD does not have renameat2() an alternative strategy is used, which first creates a dir using mkdirat() and then replaces it using renameat(). The call to mkdirat() sets mode 0 [0]. Setting mode S_IWUSR [1] solves the issue at hand for me. I think this is a cleaner solution than the attached patch.

Not sure what the downside is of setting mode S_IWUSR (versus 0), and if this is an acceptable solution. Change can be easily scoped to OpenBSD by using #ifdef/#else/#endif.

What do you think?


[0] https://gitlab.com/samba-team/samba/-/blob/master/source3/smbd/open.c?ref_type=heads#L4781-4784
[1] https://gitlab.com/samba-team/samba/-/blob/master/source3/smbd/open.c?ref_type=heads#L4784

(In reply to Bjorn Ketelaars from comment #9)

That should be possible or we disable the new code completely.

Can you point me to the OpenBSD sources where S_IWUSR
makes the difference?

I don't see it in
https://github.com/openbsd/src/blob/dac07517c712395c09494e498add8682cbbc60c9/sys/kern/vfs_syscalls.c#L2982

(In reply to Stefan Metzmacher from comment #10)

> Can you point me to the OpenBSD sources where S_IWUSR makes the difference?

If I'm not mistaken it happens in ufs_rename() [0].

[0] https://github.com/openbsd/src/blob/master/sys/ufs/ufs/ufs_vnops.c#L788C12-L788C22

(In reply to Bjorn Ketelaars from comment #11)

Yes, and also the source directory needs the write bit.

So I think we should just disable the new code in order to
avoid the regression

This bug was referenced in samba master:

a3f129f66346dcec41a01caf8060fe1a9da484ac

Created attachment 18596 [details]
Patch for v4-22-test

Reassigning to Jule for inclusion in 4.22.

Pushed to autobuild-v4-22-test.

This bug was referenced in samba v4-22-test:

ca2add8c96822cccc91d5008ab975c54e7244533

This bug was referenced in samba v4-22-stable (Release samba-4.22.0):

ca2add8c96822cccc91d5008ab975c54e7244533