Bug 35731 - Неожиданное поведение при попытке залочить файл на samba
Summary: Неожиданное поведение при попытке залочить файл на samba
Status: ASSIGNED
Alias: None
Product: Sisyphus
Classification: Development
Component: samba (show other bugs)
Version: unstable
Hardware: all Linux
: P3 normal
Assignee: Evgeny Sinelnikov
QA Contact: qa-sisyphus
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2018-12-06 12:41 MSK by Aleksei Nikiforov
Modified: 2021-08-22 22:45 MSK (History)
9 users (show)

See Also:


Attachments
smb.conf (9.80 KB, application/octet-stream)
2018-12-06 12:45 MSK, Aleksei Nikiforov
no flags Details
share_lock.c (791 bytes, text/x-csrc)
2018-12-06 12:51 MSK, Aleksei Nikiforov
no flags Details
Fixed OFD locks do not conflict with eachother (836 bytes, patch)
2018-12-12 17:55 MSK, Georgy A Bystrenin
no flags Details | Diff
[PATCH] cifs: Fixed OFD locks do not conflict with eachother (2.12 KB, patch)
2018-12-14 21:47 MSK, Georgy A Bystrenin
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Aleksei Nikiforov 2018-12-06 12:41:36 MSK
Имеется доступ к файлу, лежащему на samba и несколько систем, с которых пытаются одновременно получить доступ к этому файлу и залочить его.

Если все клиентские системы - Linux, то первая система успешно открывает файл, лочит его и держит файл с локом.

Если с этой же системы попытаться запустить ещё один инстанс приложения, который тоже попытается открыть файл, то вызов fcntl возвращает ошибку с errno 13 (EACCES). Данный код ошибки описан в мануалах и вполне соответствует ожидаемому поведению.

Если же, пока на одной системе держится лок, с другой системы попытаться открыть и залочить файл, то вызов fcntl возвращает ошибку с errno 5 (EIO). Описание данного кода в связке с вызовом fcntl(..., F_SETLK, ...) мне найти не удалось, и соответственно оно является неожиданным.

Приложение LibreOffice аналогичным образом действует при открытии файла (но это лишь небольшая часть шагов, предпринимаемых в LibreOffice), и если файл находится на samba и уже открыт с другой системы Linux, то при получении ошибки EIO LibreOffice сообщает что у файла проблема IO и что LibreOffice не может его открыть.
Comment 1 Aleksei Nikiforov 2018-12-06 12:45:05 MSK
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.
Comment 2 Aleksei Nikiforov 2018-12-06 12:51:05 MSK
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

Могу предоставить дополнительную информацию при необходимости, если укажете какая ещё информация нужна.
Comment 3 Aleksei Nikiforov 2018-12-06 13:11:15 MSK
Ещё для того, чтобы с указанным конфигом был доступен раздел public на samba, на сервере создана директория с соответствующими правами:
# mkdir /home/samba
# chmod 1777 /home/samba
Comment 4 Ivan A. Melnikov 2018-12-06 13:32:54 MSK
> Могу предоставить дополнительную информацию при необходимости, если укажете
какая ещё информация нужна.

Давайте ещё, для полноты картины, uname -а на всех машинах.
Comment 5 Aleksei Nikiforov 2018-12-06 13:53:00 MSK
На первой системе (сервер 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.
Comment 6 Sergey Bubnov 2018-12-07 21:12:24 MSK
Спасибо за тестовый код! Воспроизвелось на сизифе с самбой 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 |

Точнее сказать, в какой именно версии это поломали пока не могу, надо смотреть более детально. Как воркэраунд, если горит и ситуация позволяет, можно попробовать пока сдаунгрейдить ядро.
Comment 7 Anton Farygin 2018-12-07 21:15:18 MSK
Нет, конечно никакой downgrade ядра невозможен.
Comment 8 Sergey Bubnov 2018-12-07 21:47:34 MSK
Похоже, что поломало локи вот это
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
Comment 9 Anton Farygin 2018-12-07 21:55:13 MSK
ну сломал то не этот коммит, сломано где-то в новом диалекте.
Comment 10 Anton Farygin 2018-12-07 21:56:52 MSK
повесьте в апстрим, может быть поможет начать процесс поиска.
Comment 11 Sergey Bubnov 2018-12-07 22:22:54 MSK
Кто сломал -- это вопрос точки зрения. Дефолтное поведение с точки зрения пользователей сломал именно он.:)

Судя по этому 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. Но конкретно проблему их треда оно вроде не решило пока.

Надо собирать и смотреть. Может в нашем варианте поможет.
Comment 12 Evgeny Sinelnikov 2018-12-10 14:39:24 MSK
Отправил на тестовую сборку новый релиз:
#217590 BUILDING #1 [locked] [test-only] sisyphus kernel-image-un-def.git=kernel-image-un-def-4.19.7-alt2
Comment 13 Evgeny Sinelnikov 2018-12-10 16:10:34 MSK
Сборка ядра прошла успешно, таска упала из-за отсутствия модулей, но для теста они и не нужны. В крайней случаем, можно заменить cifs.ko из нового пакета на текущем установленном ядре 4.19.7-alt1:
http://git.altlinux.org/tasks/217590/build/100/x86_64/rpms/
Comment 14 Aleksei Nikiforov 2018-12-10 16:30:28 MSK
(В ответ на комментарий №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.
Comment 15 Sergey Bubnov 2018-12-11 15:09:22 MSK
Ядерными трейсами и самбой с `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

Что с этим делать непонятно пока.
Comment 16 Aleksei Nikiforov 2018-12-11 16:10:31 MSK
(В ответ на комментарий №15)
> ...
> Что с этим делать непонятно пока.

Вопрос не только, что с этим делать, но и является ли текущее поведение правильным. Вполне возможно, что при переезде на новый маппинг ошибок что-то напутали и поставили не то. Ещё можно проверить, поможет ли там замена EIO на EACCES получить EACCES и в userspace.
Comment 17 Sergey Bubnov 2018-12-11 19:54:42 MSK
(В ответ на комментарий №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


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

Но вопрос корректности такого подхода никуда не делся.:)
Comment 18 Aleksei Nikiforov 2018-12-12 10:16:21 MSK
(В ответ на комментарий №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.)
Comment 19 Sergey Bubnov 2018-12-12 10:31:20 MSK
(В ответ на комментарий №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+.
Comment 20 Sergey Bubnov 2018-12-12 10:34:01 MSK
В принципе, в таком случае можно просто скопировать мапинги от SMB1 в SMB2+ (которые совпадают) и так и запатчить. Что скажете?
Comment 21 Anton Farygin 2018-12-12 10:38:29 MSK
Было бы клёво поднять этот вопрос в апстриме.
Comment 22 Sergey Bubnov 2018-12-12 11:59:10 MSK
(В ответ на комментарий №21)
> Было бы клёво поднять этот вопрос в апстриме.

Ну как-то так https://bugzilla.kernel.org/show_bug.cgi?id=201971
Comment 23 Georgy A Bystrenin 2018-12-12 17:55:12 MSK
Created attachment 7890 [details]
Fixed OFD locks do not conflict with eachother
Comment 24 Georgy A Bystrenin 2018-12-12 17:57:16 MSK
To Kernel Team & boyarsh@ примите такой патч в ядро наше?
To darktemplar@ и Kernel Team багу закрываем?
Comment 25 Georgy A Bystrenin 2018-12-12 18:01:10 MSK
(В ответ на комментарий №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
Comment 26 Michael Shigorin 2018-12-12 18:06:47 MSK
(В ответ на комментарий №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.
Comment 27 Anton Farygin 2018-12-12 19:46:41 MSK
да и исправлять надо не только в сизифе, но и в p8 в ядрах std-def и un-def.

но вообще было бы интересно понять, кто расчитывает на EIO в этом месте.
Comment 28 Vitaly Lipatov 2018-12-12 19:55:45 MSK
(В ответ на комментарий №27)
> да и исправлять надо не только в сизифе, но и в p8 в ядрах std-def и un-def.
> 
> но вообще было бы интересно понять, кто расчитывает на EIO в этом месте.
Блокировки никогда не работают, кто на них может рассчитывать?
Я вот знаю только wine, мы после каждого изменения в области блокировок то ядро чиним, то wine.
Comment 29 Anton V. Boyarshinov 2018-12-13 13:59:11 MSK
ge-un-def.git=kernel-image-un-def-4.19.7-alt3

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

Пожалуй, да..
Comment 30 Dmitry V. Levin 2018-12-13 14:03:28 MSK
Мне не нравится "Hack fixed OFD locks".
Нормального патча нет?
Comment 31 Anton V. Boyarshinov 2018-12-13 14:13:09 MSK
(В ответ на комментарий №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 и вообще.
Comment 32 Dmitry V. Levin 2018-12-13 14:16:15 MSK
Тем более что в https://bugzilla.kernel.org/show_bug.cgi?id=201971 уже написан пояснительный текст, было бы жаль его терять.
Comment 33 Georgy A Bystrenin 2018-12-14 21:47:03 MSK
Created attachment 7907 [details]
[PATCH] cifs: Fixed OFD locks do not conflict with eachother
Comment 34 Georgy A Bystrenin 2018-12-14 21:49:25 MSK
#217867 BUILDING #1 [locked] [test-only] sisyphus kernel-image-un-def.git=kernel-image-un-def-4.19.8-alt2
Comment 35 Anton Farygin 2020-07-20 18:26:37 MSK
fixed?
Comment 36 Evgeny Sinelnikov 2020-07-20 23:31:04 MSK
Сложная в воспроизведении задача. Нужно проверять, как тест, каждый раз я думаю.