Re: [PATCH v11 0/4] MR6855: server: Fix renaming file to a hardlink of itself.

[Jinoh Kang (＠iamahuman)]

Jinoh Kang (@iamahuman) commented about server/fd.c:
> - for (i = 0; i < 0x8000; i++, tmp_value += 7777)
> - {
> -
>    snprintf( tmpname, sizeof(tmpname), tmpname_fmt, tmp_value );
> -
>    if (!is_dir)
> -
>    {
> -
>        /* to avoid TOCTTOU, probe existence of tmpname and link at the same time */
> -
>        if (linkat( dirfd, srcname, dirfd, tmpname, 0 ))
> -
>        {
> -
>            if (errno == EEXIST) continue;
> -
>            if (errno != EPERM && errno != EOPNOTSUPP)
> -
>            {
> -
>                file_set_error();
> -
>                goto ret;
> -
>            }
> -
> -
>            /* FIXME: filesystem doesn't support hardlinks, so this can race with other wineservers */

FYI, this fixme can be resolved by creating a temporary file on tmpname and then renaming to it (just like the is_dir case). I didn't originally mention it because it involves one extra sysca (linkat+unlinkat vs. openat+close+renameat).

I'll leave this up to you, because I'm not sure how common this scenario would be in practice. But I think it could make the logic simpler since now we can do one renameat(2) call for both file and dir, and the EPERM check can go away—at the expense of one extra syscall.