Linux kernel LOCK_MAND deprecation

[Jeremy Allison]

On Mon, Sep 13, 2021 at 11:11:45AM -0700, Christof Schmitt wrote:
>On Mon, Sep 13, 2021 at 10:40:50AM -0700, Jeremy Allison via samba-technical wrote:
>> On Mon, Sep 13, 2021 at 10:02:01AM -0400, Jeff Layton via samba-technical wrote:
>> > I recently proposed a patch to remove most of the support for
>> > flock(..., LOCK_MAND...) from the Linux kernel. The code in question has
>> > been broken for well over a decade, such that trying to use LOCK_MAND
>> > flock locks is really just a no-op:
>> >
>> >    https://lore.kernel.org/linux-fsdevel/20210910201915.95170-1-jlayton@kernel.org/
>> >
>> > Samba references that symbol in kernel_flock(). I started to take a look
>> > at removing the code from samba, but the work kind of snowballed with
>> > all of the wrappers and indirection.
>> >
>> > Would anyone who is actively working on samba want to take a stab at
>> > removing kernel_flock()? It should be safe to just rip it out since it
>> > hasn't worked in ages.
>> >
>> > If it's not removed, then you may see kernel warnings on Linux when
>> > samba tries to set a LOCK_MAND lock, a'la:
>> >
>> >   pr_warn_once("Attempt to set a LOCK_MAND lock via flock(2). This support has been removed and the request ignored.\n");
>>
>> So the only code that sets it is in source3/smbd/open.c:
>>
>> 4048         if (!fsp->fsp_flags.is_pathref &&
>> 4049             fsp_get_io_fd(fsp) != -1 &&
>> 4050             lp_kernel_share_modes(SNUM(conn)))
>> 4051         {
>> 4052                 int ret_flock;
>> 4053                 /*
>> 4054                  * Beware: streams implementing VFS modules may
>> 4055                  * implement streams in a way that fsp will have the
>> 4056                  * basefile open in the fsp fd, so lacking a distinct
>> 4057                  * fd for the stream kernel_flock will apply on the
>> 4058                  * basefile which is wrong. The actual check is
>> 4059                  * deferred to the VFS module implementing the
>> 4060                  * kernel_flock call.
>> 4061                  */
>> 4062                 ret_flock = SMB_VFS_KERNEL_FLOCK(fsp, share_access, access_mask);
>> 4063                 if(ret_flock == -1 ){
>> 4064 4065                         del_share_mode(lck, fsp);
>> 4066                         TALLOC_FREE(lck);
>> 4067                         fd_close(fsp);
>> 4068 4069                         return NT_STATUS_SHARING_VIOLATION;
>> 4070                 }
>> 4071 4072                 fsp->fsp_flags.kernel_share_modes_taken = true;
>> 4073         }
>>
>> and removes it in source3/smbd/close.c:
>>
>> 454         if (fsp->fsp_flags.kernel_share_modes_taken) {
>>  455                 int ret_flock;
>>  456  457                 /*
>>  458                  * A file system sharemode could block the unlink;
>>  459                  * remove filesystem sharemodes first.
>>  460                  */
>>  461                 ret_flock = SMB_VFS_KERNEL_FLOCK(fsp, 0, 0);
>>  462                 if (ret_flock == -1) {
>>  463                         DBG_INFO("removing kernel flock for %s failed: %s\n",
>>  464                                   fsp_str_dbg(fsp), strerror(errno));
>>  465                 }
>>  466  467                 fsp->fsp_flags.kernel_share_modes_taken = false;
>>  468         }
>>
>> (and a couple of other places that do the same thing on close).
>>
>> The rest is just boilerplace VFS glue that allows the VFS call:
>>
>>         int (*kernel_flock_fn)(struct vfs_handle_struct *handle, struct files_struct *fsp,
>>                                uint32_t share_access, uint32_t access_mask);
>>
>> to be caught by all VFS modules. It's not too hard to rip out
>> for 4.16.x (too late for a VFS change in 4.15.0).
>>
>> The only question I have, is this being used in IBM gpfs at all ?
>
>GPFS implements the Samba VFS call and implements sharemodes through a
>private API call to the file system (see vfs_gpfs_kernel_flock). From
>what i can see, the locking calls in kernel_flock are not needed, so
>that function can be removed. I would still advocate for keeping the VFS
>interface in Samba, so that we can still use the codepath in vfs_gpfs.

Oh that's easier then, we just make the vfs_default code return
ENOTSUP and remove kernel_flock(), the interface into the kernel system call
from source3/lib/system.c.

Do you want to do the patch, or shall I ?