Bug 35731

Created attachment 7881 [details]
smb.conf

Файл конфигурации samba-сервера. Практически без изменений от конфига по-умолчанию из Сизифа.

На клиентах в /etc/fstab прописаны следующие опции для монтирования:
//$ipaddress/public /mnt/server_public cifs uid=500,gid=500,credentials=/etc/samba/sambacreds 0 0

где $ipaddress - адрес сервера, uid=500 и gid=500 - uid и gid пользователя, из-под которого производилось тестирование, а файл /etc/samba/sambacreds содержит логин и пароль пользователя для samba.

Created attachment 7882 [details]
share_lock.c

Приложение, с помощью которого удаётся воспроизвести проблему с неожиданным errno от fcntl.

На первой системе я создаю файл /mnt/server_public/file_samba и запускаю это приложение с файлом, и оно висит, держа файл:
$ touch /mnt/server_public/file_samba
$ ./share_lock /mnt/server_public/file_samba
File locked successfully.
Press any key to exit...

Если на этой же системе я запускаю ещё раз данное приложение, пока один инстанс висит, то получается ошибка EACCES:
$ ./share_lock /mnt/server_public/file_samba 
fcntl error: 13, Permission denied

Если же пока на первой системе висит приложение, держа файл, запустить также его на другой системе, получается EIO:
$ ./share_lock /mnt/server_public/file_samba 
fcntl error: 5, Input/output error

Могу предоставить дополнительную информацию при необходимости, если укажете какая ещё информация нужна.

Ещё для того, чтобы с указанным конфигом был доступен раздел public на samba, на сервере создана директория с соответствующими правами:
# mkdir /home/samba
# chmod 1777 /home/samba

> Могу предоставить дополнительную информацию при необходимости, если укажете
какая ещё информация нужна.

Давайте ещё, для полноты картины, uname -а на всех машинах.

На первой системе (сервер samba и один из клиентов samba):
Linux kwork-8-2-x86-64-default.localdomain 4.19.6-un-def-alt1 #1 SMP PREEMPT Sun Dec 2 20:26:14 UTC 2018 x86_64 GNU/Linux

На второй системе (один из клиентов samba):
Linux kwork-8-2-x86-64-default.localdomain 4.18.17-un-def-alt1 #1 SMP PREEMPT Mon Nov 5 17:46:32 UTC 2018 x86_64 GNU/Linux

На обоих клиентах одинаковые настройки монтирования, одинаковый локальный пользователь с одинаковыми uid и gid и одинаковый указанный пользователь samba.

Спасибо за тестовый код! Воспроизвелось на сизифе с самбой samba-DC-4.9.3-alt1.x86_64 в режиме AD на версии ядра 4.14.86-std-def-alt1. После отката ядра до версии, которая идёт с P8 4.9.71-std-def-alt0.M80P.1 проблема проявляться перестала. Дальнейшие тесты показали, что имеет значение версия ядра только на стороне, которая пытается делать второй лок. Т.е. табличка примерно такая:

| first lock                 | second lock                | errno on second |
|----------------------------+----------------------------+-----------------|
| 4.14.86-std-def-alt1       | 4.14.86-std-def-alt1       |               5 |
| 4.14.86-std-def-alt1       | 4.9.71-std-def-alt0.M80P.1 |              13 |
| 4.9.71-std-def-alt0.M80P.1 | 4.14.86-std-def-alt1       |               5 |
| 4.9.71-std-def-alt0.M80P.1 | 4.9.71-std-def-alt0.M80P.1 |              13 |

Точнее сказать, в какой именно версии это поломали пока не могу, надо смотреть более детально. Как воркэраунд, если горит и ситуация позволяет, можно попробовать пока сдаунгрейдить ядро.

Нет, конечно никакой downgrade ядра невозможен.

Похоже, что поломало локи вот это
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/fs/cifs?id=eef914a9eb5eb83e60eb498315a491cd1edc13a1

Т.е. если добавить в опции монтирования vers=1.0, то на новых ядрах должно
работать как раньше.

файл держится другой машиной:

# используется дефолтная версия
[root@dc0 ~]# grep cifs /etc/fstab
//dc0.domain.alt/share /mnt/samba_share cifs
uid=500,gid=500,credentials=/etc/samba/sambacreds 0 0
[root@dc0 ~]# mount /mnt/samba_share/
[root@dc0 ~]# ./share_lock /mnt/samba_share/bebebe 
fcntl error: 5, Input/output error
[root@dc0 ~]# umount /mnt/samba_share/

# добавляем vers=1.0
[root@dc0 ~]# vim /etc/fstab
[root@dc0 ~]# grep cifs /etc/fstab
//dc0.domain.alt/share /mnt/samba_share cifs
uid=500,gid=500,credentials=/etc/samba/sambacreds,vers=1.0 0 0
[root@dc0 ~]# mount /mnt/samba_share/
[root@dc0 ~]# ./share_lock /mnt/samba_share/bebebe 
fcntl error: 13, Permission denied

ну сломал то не этот коммит, сломано где-то в новом диалекте.

повесьте в апстрим, может быть поможет начать процесс поиска.

Кто сломал -- это вопрос точки зрения. Дефолтное поведение с точки зрения пользователей сломал именно он.:)

Судя по этому https://bugzilla.redhat.com/show_bug.cgi?id=1484130 все давно в курсе и предлагалось несколько вариантов решения проблемы. В ядро вошёл вот этот https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/fs/cifs?id=9645759ce6b39013231f4fa312834935c93fe5bc который должен уже быть в 4.20-rc1. Но конкретно проблему их треда оно вроде не решило пока.

Надо собирать и смотреть. Может в нашем варианте поможет.

Отправил на тестовую сборку новый релиз:
#217590 BUILDING #1 [locked] [test-only] sisyphus kernel-image-un-def.git=kernel-image-un-def-4.19.7-alt2

Сборка ядра прошла успешно, таска упала из-за отсутствия модулей, но для теста они и не нужны. В крайней случаем, можно заменить cifs.ko из нового пакета на текущем установленном ядре 4.19.7-alt1:
http://git.altlinux.org/tasks/217590/build/100/x86_64/rpms/

(В ответ на комментарий №13)
> Сборка ядра прошла успешно, таска упала из-за отсутствия модулей, но для теста
> они и не нужны. В крайней случаем, можно заменить cifs.ko из нового пакета на
> текущем установленном ядре 4.19.7-alt1:
> http://git.altlinux.org/tasks/217590/build/100/x86_64/rpms/

Обновил в двух тестовых системах ядра до версии 4.19.7-alt2 из этого задания, перезагрузился. uname -a выдаёт версию 4.19.7-un-def-alt2, а тест-кейс всё ещё выдаёт errno 5.

Ядерными трейсами и самбой с `debug level = 99` ничего криминального найти не удалось. На данный момент рабочая гипотеза следующая:

для протокола первой версии используются другие мапинги ошибок, т.е.
- https://github.com/torvalds/linux/blob/master/fs/cifs/nterr.c#L139
  задефайним мапинг строки

- https://github.com/torvalds/linux/blob/master/fs/cifs/netmisc.c#L322
  замапим это дело как ERRlock

- https://github.com/torvalds/linux/blob/master/fs/cifs/netmisc.c#L66
  будем возвращать -EACCES в случае ERRlock

а вот для SMB2+ уже вот так:
- https://github.com/torvalds/linux/blob/master/fs/cifs/smb2maperror.c#L383
  для SMB2+ будем на такой же responce отдавать -EIO

Что с этим делать непонятно пока.

(В ответ на комментарий №15)
> ...
> Что с этим делать непонятно пока.

Вопрос не только, что с этим делать, но и является ли текущее поведение правильным. Вполне возможно, что при переезде на новый маппинг ошибок что-то напутали и поставили не то. Ещё можно проверить, поможет ли там замена EIO на EACCES получить EACCES и в userspace.

(В ответ на комментарий №16)
> (В ответ на комментарий №15)
> > ...
> > Что с этим делать непонятно пока.
> 
> Вопрос не только, что с этим делать, но и является ли текущее поведение
> правильным. Вполне возможно, что при переезде на новый маппинг ошибок что-то
> напутали и поставили не то.
Тут я честно говоря не уверен. Вот лично с моей точки зрения EACCES при неудаче установки лока как-то тоже не особо логично. Надо помучать более знающих товарищей на предмет смыслов вкладываемых в эти errno:)

> Ещё можно проверить, поможет ли там замена EIO на
> EACCES получить EACCES и в userspace.
Ну да. Мы примерно так же и подумали, просто случился "пожар" и я не успел дописать это в прошлый коммент. Гоша (@gkot) собрал ядро с этими грязными хаками.

#217688 BUILDING #1 [locked] [test-only] sisyphus kernel-image-un-def.git=kernel-image-un-def-4.19.7-alt3

Результат тестов положительный:
- первая машина:

[root@dc1 ~]# umount /mnt/samba_share/
[root@dc1 ~]# uname -r
4.19.7-un-def-alt3
[root@dc1 ~]# mount /mnt/samba_share/
[root@dc1 ~]# ./share_lock /mnt/samba_share/bebebe 
File locked successfully.
Press any key to exit...


- вторая машина:

[root@dc0 ~]# umount /mnt/samba_share/
umount: /mnt/samba_share/: not mounted.
[root@dc0 ~]# mount /mnt/samba_share/
[root@dc0 ~]# ./share_lock /mnt/samba_share/bebebe 
fcntl error: 13, Permission denied
[root@dc0 ~]# umount /mnt/samba_share/
[root@dc0 ~]# uname -r
4.19.7-un-def-alt3
[root@dc0 ~]# mount /mnt/samba_share/
[root@dc0 ~]# ./share_lock /mnt/samba_share/bebebe 
fcntl error: 13, Permission denied


Т.е. старое поведение вернулось. Приложения которые полагались на старое поведение должны перестать сходить с ума.:)

Но вопрос корректности такого подхода никуда не делся.:)

(В ответ на комментарий №17)
>
> ... skipped ...
>
> Тут я честно говоря не уверен. Вот лично с моей точки зрения EACCES при неудаче
> установки лока как-то тоже не особо логично. Надо помучать более знающих
> товарищей на предмет смыслов вкладываемых в эти errno:)
> 
> > Ещё можно проверить, поможет ли там замена EIO на
> > EACCES получить EACCES и в userspace.
> Ну да. Мы примерно так же и подумали, просто случился "пожар" и я не успел
> дописать это в прошлый коммент. Гоша (@gkot) собрал ядро с этими грязными
> хаками.
> 
> ... skipped ...
>
> Но вопрос корректности такого подхода никуда не делся.:)

Как минимум EACCES описан в документации для данного случая. Из LC_ALL=C man fcntl:

       F_SETLK (struct flock *)
              Acquire  a  lock  (when  l_type  is  F_RDLCK  or F_WRLCK) or release a lock (when l_type is F_UNLCK) on the bytes specified by the l_whence, l_start, and l_len fields of lock.  If a conflicting lock is held by another
              process, this call returns -1 and sets errno to EACCES or EAGAIN.  (The error returned in this case differs across implementations, so POSIX requires a portable application to check for both errors.)

(В ответ на комментарий №18)
> 
> Как минимум EACCES описан в документации для данного случая. Из LC_ALL=C man
> fcntl:
> 

Да! Справедливо. В документашке к libc (https://www.gnu.org/software/libc/manual/html_node/File-Locks.html) тоже, но там утверждается, что нужно возвращать EAGAIN

...
Some systems use EAGAIN in this case, and other systems use EACCES; your program should treat them alike, after F_SETLK. (GNU/Linux and GNU/Hurd systems always use EAGAIN.)
...

Ну в любом случае получается, что таки не прав модуль cifs в реализации SMB2+.

В принципе, в таком случае можно просто скопировать мапинги от SMB1 в SMB2+ (которые совпадают) и так и запатчить. Что скажете?

Было бы клёво поднять этот вопрос в апстриме.

(В ответ на комментарий №21)
> Было бы клёво поднять этот вопрос в апстриме.

Ну как-то так https://bugzilla.kernel.org/show_bug.cgi?id=201971

Created attachment 7890 [details]
Fixed OFD locks do not conflict with eachother

To Kernel Team & boyarsh@ примите такой патч в ядро наше?
To darktemplar@ и Kernel Team багу закрываем?

(В ответ на комментарий №23)
> Created an attachment (id=7890) [details]
> Fixed OFD locks do not conflict with eachother

Git с этим и предыдущем пачем http://git.altlinux.org/people/gkot/packages/?p=kernel-image-un-def.git;a=summary

(В ответ на комментарий №24)
> To Kernel Team & boyarsh@ примите такой патч в ядро наше?
Или можете просто зааплувить #217688 BUILDING #1 [locked] [test-only] sisyphus
kernel-image-un-def.git=kernel-image-un-def-4.19.7-alt3

(В ответ на комментарий №25)
> Или можете просто зааплувить #217688 BUILDING #1 [locked] [test-only]
> sisyphus kernel-image-un-def.git=kernel-image-un-def-4.19.7-alt3
В сизифе уже kernel-image-un-def-4.19.8-alt1.

да и исправлять надо не только в сизифе, но и в p8 в ядрах std-def и un-def.

но вообще было бы интересно понять, кто расчитывает на EIO в этом месте.

(В ответ на комментарий №27)
> да и исправлять надо не только в сизифе, но и в p8 в ядрах std-def и un-def.
> 
> но вообще было бы интересно понять, кто расчитывает на EIO в этом месте.
Блокировки никогда не работают, кто на них может рассчитывать?
Я вот знаю только wine, мы после каждого изменения в области блокировок то ядро чиним, то wine.

ge-un-def.git=kernel-image-un-def-4.19.7-alt3

(В ответ на комментарий №24)
> To Kernel Team & boyarsh@ примите такой патч в ядро наше?

Пожалуй, да..

Мне не нравится "Hack fixed OFD locks".
Нормального патча нет?

(В ответ на комментарий №25)
> (В ответ на комментарий №23)
> > Created an attachment (id=7890) [details] [details]
> > Fixed OFD locks do not conflict with eachother
> 
> Git с этим и предыдущем пачем
> http://git.altlinux.org/people/gkot/packages/?p=kernel-image-un-def.git;a=summary

Оформите второй патч отдельно от изменений в spec и вообще.

Тем более что в https://bugzilla.kernel.org/show_bug.cgi?id=201971 уже написан пояснительный текст, было бы жаль его терять.

Created attachment 7907 [details]
[PATCH] cifs: Fixed OFD locks do not conflict with eachother

#217867 BUILDING #1 [locked] [test-only] sisyphus kernel-image-un-def.git=kernel-image-un-def-4.19.8-alt2

fixed?

Сложная в воспроизведении задача. Нужно проверять, как тест, каждый раз я думаю.