linux-cifs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] cifsd: introduce new SMB3 kernel server
       [not found] <CGME20210322052203epcas1p21fe2d04c4df5396c466c38f4d57d8bb8@epcas1p2.samsung.com>
@ 2021-03-22  5:13 ` Namjae Jeon
       [not found]   ` <CGME20210322052207epcas1p3f0a5bdfd2c994a849a67b465479d0721@epcas1p3.samsung.com>
                     ` (4 more replies)
  0 siblings, 5 replies; 31+ messages in thread
From: Namjae Jeon @ 2021-03-22  5:13 UTC (permalink / raw)
  To: linux-fsdevel, linux-kernel, linux-cifs
  Cc: linux-cifsd-devel, smfrench, senozhatsky, hyc.lee, viro, hch,
	hch, ronniesahlberg, aurelien.aptel, aaptel, sandeen,
	dan.carpenter, colin.king, rdunlap, Namjae Jeon

This is the patch series for cifsd(ksmbd) kernel server.

What is cifsd(ksmbd) ?
======================

The SMB family of protocols is the most widely deployed
network filesystem protocol, the default on Windows and Macs (and even
on many phones and tablets), with clients and servers on all major
operating systems, but lacked a kernel server for Linux. For many
cases the current userspace server choices were suboptimal
either due to memory footprint, performance or difficulty integrating
well with advanced Linux features.

ksmbd is a new kernel module which implements the server-side of the SMB3 protocol.
The target is to provide optimized performance, GPLv2 SMB server, better
lease handling (distributed caching). The bigger goal is to add new
features more rapidly (e.g. RDMA aka "smbdirect", and recent encryption
and signing improvements to the protocol) which are easier to develop
on a smaller, more tightly optimized kernel server than for example
in Samba.  The Samba project is much broader in scope (tools, security services,
LDAP, Active Directory Domain Controller, and a cross platform file server
for a wider variety of purposes) but the user space file server portion
of Samba has proved hard to optimize for some Linux workloads, including
for smaller devices. This is not meant to replace Samba, but rather be
an extension to allow better optimizing for Linux, and will continue to
integrate well with Samba user space tools and libraries where appropriate.
Working with the Samba team we have already made sure that the configuration
files and xattrs are in a compatible format between the kernel and
user space server.


Architecture
============

               |--- ...
       --------|--- ksmbd/3 - Client 3
       |-------|--- ksmbd/2 - Client 2
       |       |         ____________________________________________________
       |       |        |- Client 1                                          |
<--- Socket ---|--- ksmbd/1   <<= Authentication : NTLM/NTLM2, Kerberos      |
       |       |      | |     <<= SMB engine : SMB2, SMB2.1, SMB3, SMB3.0.2, |
       |       |      | |                SMB3.1.1                            |
       |       |      | |____________________________________________________|
       |       |      |
       |       |      |--- VFS --- Local Filesystem
       |       |
KERNEL |--- ksmbd/0(forker kthread)
---------------||---------------------------------------------------------------
USER           ||
               || communication using NETLINK
               ||  ______________________________________________
               || |                                              |
        ksmbd.mountd <<= DCE/RPC(srvsvc, wkssvc, samr, lsarpc)   |
               ^  |  <<= configure shares setting, user accounts |
               |  |______________________________________________|
               |
               |------ smb.conf(config file)
               |
               |------ ksmbdpwd.db(user account/password file)
                            ^
  ksmbd.adduser ------------|

The subset of performance related operations(open/read/write/close etc.) belong
in kernelspace(ksmbd) and the other subset which belong to operations(DCE/RPC,
user account/share database) which are not really related with performance are
handled in userspace(ksmbd.mountd).

When the ksmbd.mountd is started, It starts up a forker thread at initialization
time and opens a dedicated port 445 for listening to SMB requests. Whenever new
clients make request, Forker thread will accept the client connection and fork
a new thread for dedicated communication channel between the client and
the server.


ksmbd feature status
====================

============================== =================================================
Feature name                   Status
============================== =================================================
Dialects                       Supported. SMB2.1 SMB3.0, SMB3.1.1 dialects
                               (intentionally excludes security vulnerable SMB1 dialect).
Auto Negotiation               Supported.
Compound Request               Supported.
Oplock Cache Mechanism         Supported.
SMB2 leases(v1 lease)          Supported.
Directory leases(v2 lease)     Planned for future.
Multi-credits                  Supported.
NTLM/NTLMv2                    Supported.
HMAC-SHA256 Signing            Supported.
Secure negotiate               Supported.
Signing Update                 Supported.
Pre-authentication integrity   Supported.
SMB3 encryption(CCM, GCM)      Supported. (CCM and GCM128 supported, GCM256 in progress)
SMB direct(RDMA)               Partially Supported. SMB3 Multi-channel is required
                               to connect to Windows client.
SMB3 Multi-channel             In Progress.
SMB3.1.1 POSIX extension       Supported.
ACLs                           Partially Supported. only DACLs available, SACLs
                               (auditing) is planned for the future. For
                               ownership (SIDs) ksmbd generates random subauth
                               values(then store it to disk) and use uid/gid
                               get from inode as RID for local domain SID.
                               The current acl implementation is limited to
                               standalone server, not a domain member.
                               Integration with Samba tools is being worked on to
                               allow future support for running as a domain member.
Kerberos                       Supported.
Durable handle v1,v2           Planned for future.
Persistent handle              Planned for future.
SMB2 notify                    Planned for future.
Sparse file support            Supported.
DCE/RPC support                Partially Supported. a few calls(NetShareEnumAll,
                               NetServerGetInfo, SAMR, LSARPC) that are needed 
                               for file server handled via netlink interface from
                               ksmbd.mountd. Additional integration with Samba
                               tools and libraries via upcall is being investigated
                               to allow support for additional DCE/RPC management
                               calls (and future support for Witness protocol e.g.)
============================== =================================================

All features required as file server are currently implemented in ksmbd.
In particular, the implementation of SMB Direct(RDMA) is only currently
possible with ksmbd (among Linux servers)


Stability
=========

It has been proved to be stable. A significant amount of xfstests pass and
are run regularly from Linux to Linux:

  http://smb3-test-rhel-75.southcentralus.cloudapp.azure.com/#/builders/8/builds/26

In addition regression tests using the broadest SMB3 functional test suite
(Samba's "smbtorture") are run on every checkin. 
It has already been used by many other open source toolkits and commercial companies
that need NAS functionality. Their issues have been fixed and contributions are
applied into ksmbd. Ksmbd has been well tested and verified in the field and market.


Mailing list and repositories
=============================
 - linux-cifsd-devel@lists.sourceforge.net
 - https://github.com/smfrench/smb3-kernel/tree/cifsd-for-next
 - https://github.com/cifsd-team/cifsd (out-of-tree)
 - https://github.com/cifsd-team/ksmbd-tools


How to run ksmbd 
================

   a. Download ksmbd-tools and compile them.
	- https://github.com/cifsd-team/ksmbd-tools

   b. Create user/password for SMB share.

	# mkdir /etc/ksmbd/
	# ksmbd.adduser -a <Enter USERNAME for SMB share access>

   c. Create /etc/ksmbd/smb.conf file, add SMB share in smb.conf file
	- Refer smb.conf.example and Documentation/configuration.txt
	  in ksmbd-tools

   d. Insert ksmbd.ko module

	# insmod ksmbd.ko

   e. Start ksmbd user space daemon
	# ksmbd.mountd

   f. Access share from Windows or Linux using SMB 
       e.g. "mount -t cifs //server/share /mnt ..."

v0:
 - fix a handful of spelling mistakes (Colin Ian King)
 - fix a precedence bug in parse_dacl() (Dan Carpenter)
 - fix a IS_ERR() vs NULL bug (Dan Carpenter)
 - fix a use after free on error path  (Dan Carpenter)
 - update cifsd.rst Documentation
 - remove unneeded FIXME comments
 - fix static checker warnings (Dan Carpenter)
 - fix WARNING: unmet direct dependencies detected for CRYPTO_ARC4 (Randy Dunlap)
 - uniquify extract_sharename() (Stephen Rothwell)
 - fix WARNING: document isn't included in any toctree (Stephen Rothwell)
 - fix WARNING: Title overline too short (Stephen Rothwell)
 - fix incorrect function comments

Namjae Jeon (5):
  cifsd: add server handler and tranport layers
  cifsd: add server-side procedures for SMB3
  cifsd: add file operations
  cifsd: add Kconfig and Makefile
  MAINTAINERS: add cifsd kernel server

 Documentation/filesystems/cifs/cifsd.rst |  180 +
 Documentation/filesystems/cifs/index.rst |   10 +
 Documentation/filesystems/index.rst      |    2 +-
 MAINTAINERS                              |   12 +-
 fs/Kconfig                               |    1 +
 fs/Makefile                              |    1 +
 fs/cifsd/Kconfig                         |   64 +
 fs/cifsd/Makefile                        |   13 +
 fs/cifsd/asn1.c                          |  702 ++
 fs/cifsd/asn1.h                          |   29 +
 fs/cifsd/auth.c                          | 1348 ++++
 fs/cifsd/auth.h                          |   90 +
 fs/cifsd/buffer_pool.c                   |  292 +
 fs/cifsd/buffer_pool.h                   |   28 +
 fs/cifsd/connection.c                    |  416 ++
 fs/cifsd/connection.h                    |  212 +
 fs/cifsd/crypto_ctx.c                    |  287 +
 fs/cifsd/crypto_ctx.h                    |   77 +
 fs/cifsd/glob.h                          |   67 +
 fs/cifsd/ksmbd_server.h                  |  285 +
 fs/cifsd/ksmbd_work.c                    |   93 +
 fs/cifsd/ksmbd_work.h                    |  124 +
 fs/cifsd/mgmt/ksmbd_ida.c                |   69 +
 fs/cifsd/mgmt/ksmbd_ida.h                |   41 +
 fs/cifsd/mgmt/share_config.c             |  238 +
 fs/cifsd/mgmt/share_config.h             |   81 +
 fs/cifsd/mgmt/tree_connect.c             |  128 +
 fs/cifsd/mgmt/tree_connect.h             |   56 +
 fs/cifsd/mgmt/user_config.c              |   69 +
 fs/cifsd/mgmt/user_config.h              |   66 +
 fs/cifsd/mgmt/user_session.c             |  344 +
 fs/cifsd/mgmt/user_session.h             |  105 +
 fs/cifsd/misc.c                          |  296 +
 fs/cifsd/misc.h                          |   38 +
 fs/cifsd/ndr.c                           |  344 +
 fs/cifsd/ndr.h                           |   21 +
 fs/cifsd/netmisc.c                       |   46 +
 fs/cifsd/nterr.c                         |  674 ++
 fs/cifsd/nterr.h                         |  552 ++
 fs/cifsd/ntlmssp.h                       |  169 +
 fs/cifsd/oplock.c                        | 1681 +++++
 fs/cifsd/oplock.h                        |  138 +
 fs/cifsd/server.c                        |  632 ++
 fs/cifsd/server.h                        |   62 +
 fs/cifsd/smb2misc.c                      |  458 ++
 fs/cifsd/smb2ops.c                       |  300 +
 fs/cifsd/smb2pdu.c                       | 8452 ++++++++++++++++++++++
 fs/cifsd/smb2pdu.h                       | 1649 +++++
 fs/cifsd/smb_common.c                    |  667 ++
 fs/cifsd/smb_common.h                    |  546 ++
 fs/cifsd/smbacl.c                        | 1324 ++++
 fs/cifsd/smbacl.h                        |  202 +
 fs/cifsd/smberr.h                        |  235 +
 fs/cifsd/smbfsctl.h                      |   90 +
 fs/cifsd/smbstatus.h                     | 1822 +++++
 fs/cifsd/time_wrappers.h                 |   34 +
 fs/cifsd/transport_ipc.c                 |  897 +++
 fs/cifsd/transport_ipc.h                 |   62 +
 fs/cifsd/transport_rdma.c                | 2051 ++++++
 fs/cifsd/transport_rdma.h                |   61 +
 fs/cifsd/transport_tcp.c                 |  625 ++
 fs/cifsd/transport_tcp.h                 |   13 +
 fs/cifsd/unicode.c                       |  391 +
 fs/cifsd/unicode.h                       |  374 +
 fs/cifsd/uniupr.h                        |  268 +
 fs/cifsd/vfs.c                           | 1989 +++++
 fs/cifsd/vfs.h                           |  314 +
 fs/cifsd/vfs_cache.c                     |  851 +++
 fs/cifsd/vfs_cache.h                     |  213 +
 69 files changed, 34069 insertions(+), 2 deletions(-)
 create mode 100644 Documentation/filesystems/cifs/cifsd.rst
 create mode 100644 Documentation/filesystems/cifs/index.rst
 create mode 100644 fs/cifsd/Kconfig
 create mode 100644 fs/cifsd/Makefile
 create mode 100644 fs/cifsd/asn1.c
 create mode 100644 fs/cifsd/asn1.h
 create mode 100644 fs/cifsd/auth.c
 create mode 100644 fs/cifsd/auth.h
 create mode 100644 fs/cifsd/buffer_pool.c
 create mode 100644 fs/cifsd/buffer_pool.h
 create mode 100644 fs/cifsd/connection.c
 create mode 100644 fs/cifsd/connection.h
 create mode 100644 fs/cifsd/crypto_ctx.c
 create mode 100644 fs/cifsd/crypto_ctx.h
 create mode 100644 fs/cifsd/glob.h
 create mode 100644 fs/cifsd/ksmbd_server.h
 create mode 100644 fs/cifsd/ksmbd_work.c
 create mode 100644 fs/cifsd/ksmbd_work.h
 create mode 100644 fs/cifsd/mgmt/ksmbd_ida.c
 create mode 100644 fs/cifsd/mgmt/ksmbd_ida.h
 create mode 100644 fs/cifsd/mgmt/share_config.c
 create mode 100644 fs/cifsd/mgmt/share_config.h
 create mode 100644 fs/cifsd/mgmt/tree_connect.c
 create mode 100644 fs/cifsd/mgmt/tree_connect.h
 create mode 100644 fs/cifsd/mgmt/user_config.c
 create mode 100644 fs/cifsd/mgmt/user_config.h
 create mode 100644 fs/cifsd/mgmt/user_session.c
 create mode 100644 fs/cifsd/mgmt/user_session.h
 create mode 100644 fs/cifsd/misc.c
 create mode 100644 fs/cifsd/misc.h
 create mode 100644 fs/cifsd/ndr.c
 create mode 100644 fs/cifsd/ndr.h
 create mode 100644 fs/cifsd/netmisc.c
 create mode 100644 fs/cifsd/nterr.c
 create mode 100644 fs/cifsd/nterr.h
 create mode 100644 fs/cifsd/ntlmssp.h
 create mode 100644 fs/cifsd/oplock.c
 create mode 100644 fs/cifsd/oplock.h
 create mode 100644 fs/cifsd/server.c
 create mode 100644 fs/cifsd/server.h
 create mode 100644 fs/cifsd/smb2misc.c
 create mode 100644 fs/cifsd/smb2ops.c
 create mode 100644 fs/cifsd/smb2pdu.c
 create mode 100644 fs/cifsd/smb2pdu.h
 create mode 100644 fs/cifsd/smb_common.c
 create mode 100644 fs/cifsd/smb_common.h
 create mode 100644 fs/cifsd/smbacl.c
 create mode 100644 fs/cifsd/smbacl.h
 create mode 100644 fs/cifsd/smberr.h
 create mode 100644 fs/cifsd/smbfsctl.h
 create mode 100644 fs/cifsd/smbstatus.h
 create mode 100644 fs/cifsd/time_wrappers.h
 create mode 100644 fs/cifsd/transport_ipc.c
 create mode 100644 fs/cifsd/transport_ipc.h
 create mode 100644 fs/cifsd/transport_rdma.c
 create mode 100644 fs/cifsd/transport_rdma.h
 create mode 100644 fs/cifsd/transport_tcp.c
 create mode 100644 fs/cifsd/transport_tcp.h
 create mode 100644 fs/cifsd/unicode.c
 create mode 100644 fs/cifsd/unicode.h
 create mode 100644 fs/cifsd/uniupr.h
 create mode 100644 fs/cifsd/vfs.c
 create mode 100644 fs/cifsd/vfs.h
 create mode 100644 fs/cifsd/vfs_cache.c
 create mode 100644 fs/cifsd/vfs_cache.h

-- 
2.17.1


^ permalink raw reply	[flat|nested] 31+ messages in thread

* [PATCH 3/5] cifsd: add file operations
       [not found]   ` <CGME20210322052207epcas1p3f0a5bdfd2c994a849a67b465479d0721@epcas1p3.samsung.com>
@ 2021-03-22  5:13     ` Namjae Jeon
  2021-03-22  6:55       ` Al Viro
                         ` (3 more replies)
  0 siblings, 4 replies; 31+ messages in thread
From: Namjae Jeon @ 2021-03-22  5:13 UTC (permalink / raw)
  To: linux-fsdevel, linux-kernel, linux-cifs
  Cc: linux-cifsd-devel, smfrench, senozhatsky, hyc.lee, viro, hch,
	hch, ronniesahlberg, aurelien.aptel, aaptel, sandeen,
	dan.carpenter, colin.king, rdunlap, Namjae Jeon,
	Sergey Senozhatsky, Steve French

This adds file operations and buffer pool for cifsd.

Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com>
Acked-by: Ronnie Sahlberg <lsahlber@redhat.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifsd/buffer_pool.c |  292 ++++++
 fs/cifsd/buffer_pool.h |   28 +
 fs/cifsd/vfs.c         | 1989 ++++++++++++++++++++++++++++++++++++++++
 fs/cifsd/vfs.h         |  314 +++++++
 fs/cifsd/vfs_cache.c   |  851 +++++++++++++++++
 fs/cifsd/vfs_cache.h   |  213 +++++
 6 files changed, 3687 insertions(+)
 create mode 100644 fs/cifsd/buffer_pool.c
 create mode 100644 fs/cifsd/buffer_pool.h
 create mode 100644 fs/cifsd/vfs.c
 create mode 100644 fs/cifsd/vfs.h
 create mode 100644 fs/cifsd/vfs_cache.c
 create mode 100644 fs/cifsd/vfs_cache.h

diff --git a/fs/cifsd/buffer_pool.c b/fs/cifsd/buffer_pool.c
new file mode 100644
index 000000000000..864fea547c68
--- /dev/null
+++ b/fs/cifsd/buffer_pool.c
@@ -0,0 +1,292 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *   Copyright (C) 2018 Samsung Electronics Co., Ltd.
+ */
+
+#include <linux/kernel.h>
+#include <linux/wait.h>
+#include <linux/sched.h>
+#include <linux/mm.h>
+#include <linux/slab.h>
+#include <linux/vmalloc.h>
+#include <linux/rwlock.h>
+
+#include "glob.h"
+#include "buffer_pool.h"
+#include "connection.h"
+#include "mgmt/ksmbd_ida.h"
+
+static struct kmem_cache *filp_cache;
+
+struct wm {
+	struct list_head	list;
+	unsigned int		sz;
+	char			buffer[0];
+};
+
+struct wm_list {
+	struct list_head	list;
+	unsigned int		sz;
+
+	spinlock_t		wm_lock;
+	int			avail_wm;
+	struct list_head	idle_wm;
+	wait_queue_head_t	wm_wait;
+};
+
+static LIST_HEAD(wm_lists);
+static DEFINE_RWLOCK(wm_lists_lock);
+
+void *ksmbd_alloc(size_t size)
+{
+	return kvmalloc(size, GFP_KERNEL | __GFP_ZERO);
+}
+
+void ksmbd_free(void *ptr)
+{
+	kvfree(ptr);
+}
+
+static struct wm *wm_alloc(size_t sz, gfp_t flags)
+{
+	struct wm *wm;
+	size_t alloc_sz = sz + sizeof(struct wm);
+
+	wm = kvmalloc(alloc_sz, flags);
+	if (!wm)
+		return NULL;
+	wm->sz = sz;
+	return wm;
+}
+
+static int register_wm_size_class(size_t sz)
+{
+	struct wm_list *l, *nl;
+
+	nl = kvmalloc(sizeof(struct wm_list), GFP_KERNEL);
+	if (!nl)
+		return -ENOMEM;
+
+	nl->sz = sz;
+	spin_lock_init(&nl->wm_lock);
+	INIT_LIST_HEAD(&nl->idle_wm);
+	INIT_LIST_HEAD(&nl->list);
+	init_waitqueue_head(&nl->wm_wait);
+	nl->avail_wm = 0;
+
+	write_lock(&wm_lists_lock);
+	list_for_each_entry(l, &wm_lists, list) {
+		if (l->sz == sz) {
+			write_unlock(&wm_lists_lock);
+			kvfree(nl);
+			return 0;
+		}
+	}
+
+	list_add(&nl->list, &wm_lists);
+	write_unlock(&wm_lists_lock);
+	return 0;
+}
+
+static struct wm_list *match_wm_list(size_t size)
+{
+	struct wm_list *l, *rl = NULL;
+
+	read_lock(&wm_lists_lock);
+	list_for_each_entry(l, &wm_lists, list) {
+		if (l->sz == size) {
+			rl = l;
+			break;
+		}
+	}
+	read_unlock(&wm_lists_lock);
+	return rl;
+}
+
+static struct wm *find_wm(size_t size)
+{
+	struct wm_list *wm_list;
+	struct wm *wm;
+
+	wm_list = match_wm_list(size);
+	if (!wm_list) {
+		if (register_wm_size_class(size))
+			return NULL;
+		wm_list = match_wm_list(size);
+	}
+
+	if (!wm_list)
+		return NULL;
+
+	while (1) {
+		spin_lock(&wm_list->wm_lock);
+		if (!list_empty(&wm_list->idle_wm)) {
+			wm = list_entry(wm_list->idle_wm.next,
+					struct wm,
+					list);
+			list_del(&wm->list);
+			spin_unlock(&wm_list->wm_lock);
+			return wm;
+		}
+
+		if (wm_list->avail_wm > num_online_cpus()) {
+			spin_unlock(&wm_list->wm_lock);
+			wait_event(wm_list->wm_wait,
+				   !list_empty(&wm_list->idle_wm));
+			continue;
+		}
+
+		wm_list->avail_wm++;
+		spin_unlock(&wm_list->wm_lock);
+
+		wm = wm_alloc(size, GFP_KERNEL);
+		if (!wm) {
+			spin_lock(&wm_list->wm_lock);
+			wm_list->avail_wm--;
+			spin_unlock(&wm_list->wm_lock);
+			wait_event(wm_list->wm_wait,
+				   !list_empty(&wm_list->idle_wm));
+			continue;
+		}
+		break;
+	}
+
+	return wm;
+}
+
+static void release_wm(struct wm *wm, struct wm_list *wm_list)
+{
+	if (!wm)
+		return;
+
+	spin_lock(&wm_list->wm_lock);
+	if (wm_list->avail_wm <= num_online_cpus()) {
+		list_add(&wm->list, &wm_list->idle_wm);
+		spin_unlock(&wm_list->wm_lock);
+		wake_up(&wm_list->wm_wait);
+		return;
+	}
+
+	wm_list->avail_wm--;
+	spin_unlock(&wm_list->wm_lock);
+	ksmbd_free(wm);
+}
+
+static void wm_list_free(struct wm_list *l)
+{
+	struct wm *wm;
+
+	while (!list_empty(&l->idle_wm)) {
+		wm = list_entry(l->idle_wm.next, struct wm, list);
+		list_del(&wm->list);
+		kvfree(wm);
+	}
+	kvfree(l);
+}
+
+static void wm_lists_destroy(void)
+{
+	struct wm_list *l;
+
+	while (!list_empty(&wm_lists)) {
+		l = list_entry(wm_lists.next, struct wm_list, list);
+		list_del(&l->list);
+		wm_list_free(l);
+	}
+}
+
+void ksmbd_free_request(void *addr)
+{
+	kvfree(addr);
+}
+
+void *ksmbd_alloc_request(size_t size)
+{
+	return kvmalloc(size, GFP_KERNEL);
+}
+
+void ksmbd_free_response(void *buffer)
+{
+	kvfree(buffer);
+}
+
+void *ksmbd_alloc_response(size_t size)
+{
+	return kvmalloc(size, GFP_KERNEL | __GFP_ZERO);
+}
+
+void *ksmbd_find_buffer(size_t size)
+{
+	struct wm *wm;
+
+	wm = find_wm(size);
+
+	WARN_ON(!wm);
+	if (wm)
+		return wm->buffer;
+	return NULL;
+}
+
+void ksmbd_release_buffer(void *buffer)
+{
+	struct wm_list *wm_list;
+	struct wm *wm;
+
+	if (!buffer)
+		return;
+
+	wm = container_of(buffer, struct wm, buffer);
+	wm_list = match_wm_list(wm->sz);
+	WARN_ON(!wm_list);
+	if (wm_list)
+		release_wm(wm, wm_list);
+}
+
+void *ksmbd_realloc_response(void *ptr, size_t old_sz, size_t new_sz)
+{
+	size_t sz = min(old_sz, new_sz);
+	void *nptr;
+
+	nptr = ksmbd_alloc_response(new_sz);
+	if (!nptr)
+		return ptr;
+	memcpy(nptr, ptr, sz);
+	ksmbd_free_response(ptr);
+	return nptr;
+}
+
+void ksmbd_free_file_struct(void *filp)
+{
+	kmem_cache_free(filp_cache, filp);
+}
+
+void *ksmbd_alloc_file_struct(void)
+{
+	return kmem_cache_zalloc(filp_cache, GFP_KERNEL);
+}
+
+void ksmbd_destroy_buffer_pools(void)
+{
+	wm_lists_destroy();
+	ksmbd_work_pool_destroy();
+	kmem_cache_destroy(filp_cache);
+}
+
+int ksmbd_init_buffer_pools(void)
+{
+	if (ksmbd_work_pool_init())
+		goto out;
+
+	filp_cache = kmem_cache_create("ksmbd_file_cache",
+					sizeof(struct ksmbd_file), 0,
+					SLAB_HWCACHE_ALIGN, NULL);
+	if (!filp_cache)
+		goto out;
+
+	return 0;
+
+out:
+	ksmbd_err("failed to allocate memory\n");
+	ksmbd_destroy_buffer_pools();
+	return -ENOMEM;
+}
diff --git a/fs/cifsd/buffer_pool.h b/fs/cifsd/buffer_pool.h
new file mode 100644
index 000000000000..2b3d03afcf27
--- /dev/null
+++ b/fs/cifsd/buffer_pool.h
@@ -0,0 +1,28 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ *   Copyright (C) 2018 Samsung Electronics Co., Ltd.
+ */
+
+#ifndef __KSMBD_BUFFER_POOL_H__
+#define __KSMBD_BUFFER_POOL_H__
+
+void *ksmbd_find_buffer(size_t size);
+void ksmbd_release_buffer(void *buffer);
+
+void *ksmbd_alloc(size_t size);
+void ksmbd_free(void *ptr);
+
+void ksmbd_free_request(void *addr);
+void *ksmbd_alloc_request(size_t size);
+void ksmbd_free_response(void *buffer);
+void *ksmbd_alloc_response(size_t size);
+
+void *ksmbd_realloc_response(void *ptr, size_t old_sz, size_t new_sz);
+
+void ksmbd_free_file_struct(void *filp);
+void *ksmbd_alloc_file_struct(void);
+
+void ksmbd_destroy_buffer_pools(void);
+int ksmbd_init_buffer_pools(void);
+
+#endif /* __KSMBD_BUFFER_POOL_H__ */
diff --git a/fs/cifsd/vfs.c b/fs/cifsd/vfs.c
new file mode 100644
index 000000000000..00f80ca45690
--- /dev/null
+++ b/fs/cifsd/vfs.c
@@ -0,0 +1,1989 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ *   Copyright (C) 2016 Namjae Jeon <linkinjeon@kernel.org>
+ *   Copyright (C) 2018 Samsung Electronics Co., Ltd.
+ */
+
+#include <linux/kernel.h>
+#include <linux/fs.h>
+#include <linux/uaccess.h>
+#include <linux/backing-dev.h>
+#include <linux/writeback.h>
+#include <linux/version.h>
+#include <linux/xattr.h>
+#include <linux/falloc.h>
+#include <linux/genhd.h>
+#include <linux/blkdev.h>
+#include <linux/fsnotify.h>
+#include <linux/dcache.h>
+#include <linux/slab.h>
+#include <linux/vmalloc.h>
+#include <linux/sched/xacct.h>
+#include <linux/crc32c.h>
+
+#include "glob.h"
+#include "oplock.h"
+#include "connection.h"
+#include "buffer_pool.h"
+#include "vfs.h"
+#include "vfs_cache.h"
+#include "smbacl.h"
+#include "ndr.h"
+#include "auth.h"
+
+#include "time_wrappers.h"
+#include "smb_common.h"
+#include "mgmt/share_config.h"
+#include "mgmt/tree_connect.h"
+#include "mgmt/user_session.h"
+#include "mgmt/user_config.h"
+
+static char *extract_last_component(char *path)
+{
+	char *p = strrchr(path, '/');
+
+	if (p && p[1] != '\0') {
+		*p = '\0';
+		p++;
+	} else {
+		p = NULL;
+		ksmbd_err("Invalid path %s\n", path);
+	}
+	return p;
+}
+
+static void rollback_path_modification(char *filename)
+{
+	if (filename) {
+		filename--;
+		*filename = '/';
+	}
+}
+
+static void ksmbd_vfs_inherit_owner(struct ksmbd_work *work,
+				    struct inode *parent_inode,
+				    struct inode *inode)
+{
+	if (!test_share_config_flag(work->tcon->share_conf,
+				   KSMBD_SHARE_FLAG_INHERIT_OWNER))
+		return;
+
+	i_uid_write(inode, i_uid_read(parent_inode));
+}
+
+static void ksmbd_vfs_inherit_smack(struct ksmbd_work *work,
+				    struct dentry *dir_dentry,
+				    struct dentry *dentry)
+{
+	char *name, *xattr_list = NULL, *smack_buf;
+	int value_len, xattr_list_len;
+
+	if (!test_share_config_flag(work->tcon->share_conf,
+				    KSMBD_SHARE_FLAG_INHERIT_SMACK))
+		return;
+
+	xattr_list_len = ksmbd_vfs_listxattr(dir_dentry, &xattr_list);
+	if (xattr_list_len < 0) {
+		goto out;
+	} else if (!xattr_list_len) {
+		ksmbd_err("no ea data in the file\n");
+		return;
+	}
+
+	for (name = xattr_list; name - xattr_list < xattr_list_len;
+			name += strlen(name) + 1) {
+		int rc;
+
+		ksmbd_debug(VFS, "%s, len %zd\n", name, strlen(name));
+		if (strcmp(name, XATTR_NAME_SMACK))
+			continue;
+
+		value_len = ksmbd_vfs_getxattr(dir_dentry, name, &smack_buf);
+		if (value_len <= 0)
+			continue;
+
+		rc = ksmbd_vfs_setxattr(dentry, XATTR_NAME_SMACK, smack_buf,
+					value_len, 0);
+		ksmbd_free(smack_buf);
+		if (rc < 0)
+			ksmbd_err("ksmbd_vfs_setxattr() failed: %d\n", rc);
+	}
+out:
+	ksmbd_vfs_xattr_free(xattr_list);
+}
+
+int ksmbd_vfs_inode_permission(struct dentry *dentry, int acc_mode, bool delete)
+{
+	int mask;
+
+	mask = 0;
+	acc_mode &= O_ACCMODE;
+
+	if (acc_mode == O_RDONLY)
+		mask = MAY_READ;
+	else if (acc_mode == O_WRONLY)
+		mask = MAY_WRITE;
+	else if (acc_mode == O_RDWR)
+		mask = MAY_READ | MAY_WRITE;
+
+	if (inode_permission(&init_user_ns, d_inode(dentry), mask | MAY_OPEN))
+		return -EACCES;
+
+	if (delete) {
+		struct dentry *parent;
+
+		parent = dget_parent(dentry);
+		if (!parent)
+			return -EINVAL;
+
+		if (inode_permission(&init_user_ns, d_inode(parent), MAY_EXEC | MAY_WRITE)) {
+			dput(parent);
+			return -EACCES;
+		}
+		dput(parent);
+	}
+	return 0;
+}
+
+int ksmbd_vfs_query_maximal_access(struct dentry *dentry, __le32 *daccess)
+{
+	struct dentry *parent;
+
+	*daccess = cpu_to_le32(FILE_READ_ATTRIBUTES | READ_CONTROL);
+
+	if (!inode_permission(&init_user_ns, d_inode(dentry), MAY_OPEN | MAY_WRITE))
+		*daccess |= cpu_to_le32(WRITE_DAC | WRITE_OWNER | SYNCHRONIZE |
+				FILE_WRITE_DATA | FILE_APPEND_DATA |
+				FILE_WRITE_EA | FILE_WRITE_ATTRIBUTES |
+				FILE_DELETE_CHILD);
+
+	if (!inode_permission(&init_user_ns, d_inode(dentry), MAY_OPEN | MAY_READ))
+		*daccess |= FILE_READ_DATA_LE | FILE_READ_EA_LE;
+
+	if (!inode_permission(&init_user_ns, d_inode(dentry), MAY_OPEN | MAY_EXEC))
+		*daccess |= FILE_EXECUTE_LE;
+
+	parent = dget_parent(dentry);
+	if (!parent)
+		return 0;
+
+	if (!inode_permission(&init_user_ns, d_inode(parent), MAY_EXEC | MAY_WRITE))
+		*daccess |= FILE_DELETE_LE;
+	dput(parent);
+	return 0;
+}
+
+
+/**
+ * ksmbd_vfs_create() - vfs helper for smb create file
+ * @work:	work
+ * @name:	file name
+ * @mode:	file create mode
+ *
+ * Return:	0 on success, otherwise error
+ */
+int ksmbd_vfs_create(struct ksmbd_work *work,
+		     const char *name,
+		     umode_t mode)
+{
+	struct path path;
+	struct dentry *dentry;
+	int err;
+
+	dentry = kern_path_create(AT_FDCWD, name, &path, 0);
+	if (IS_ERR(dentry)) {
+		err = PTR_ERR(dentry);
+		if (err != -ENOENT)
+			ksmbd_err("path create failed for %s, err %d\n",
+				name, err);
+		return err;
+	}
+
+	mode |= S_IFREG;
+	err = vfs_create(&init_user_ns, d_inode(path.dentry), dentry, mode, true);
+	if (!err) {
+		ksmbd_vfs_inherit_owner(work, d_inode(path.dentry),
+			d_inode(dentry));
+		ksmbd_vfs_inherit_smack(work, path.dentry, dentry);
+	} else {
+		ksmbd_err("File(%s): creation failed (err:%d)\n", name, err);
+	}
+	done_path_create(&path, dentry);
+	return err;
+}
+
+/**
+ * ksmbd_vfs_mkdir() - vfs helper for smb create directory
+ * @work:	work
+ * @name:	directory name
+ * @mode:	directory create mode
+ *
+ * Return:	0 on success, otherwise error
+ */
+int ksmbd_vfs_mkdir(struct ksmbd_work *work,
+		    const char *name,
+		    umode_t mode)
+{
+	struct path path;
+	struct dentry *dentry;
+	int err;
+
+	dentry = kern_path_create(AT_FDCWD, name, &path, LOOKUP_DIRECTORY);
+	if (IS_ERR(dentry)) {
+		err = PTR_ERR(dentry);
+		if (err != -EEXIST)
+			ksmbd_debug(VFS, "path create failed for %s, err %d\n",
+					name, err);
+		return err;
+	}
+
+	mode |= S_IFDIR;
+	err = vfs_mkdir(&init_user_ns, d_inode(path.dentry), dentry, mode);
+	if (!err) {
+		ksmbd_vfs_inherit_owner(work, d_inode(path.dentry),
+			d_inode(dentry));
+		ksmbd_vfs_inherit_smack(work, path.dentry, dentry);
+	} else
+		ksmbd_err("mkdir(%s): creation failed (err:%d)\n", name, err);
+
+	done_path_create(&path, dentry);
+	return err;
+}
+
+static ssize_t ksmbd_vfs_getcasexattr(struct dentry *dentry,
+				      char *attr_name,
+				      int attr_name_len,
+				      char **attr_value)
+{
+	char *name, *xattr_list = NULL;
+	ssize_t value_len = -ENOENT, xattr_list_len;
+
+	xattr_list_len = ksmbd_vfs_listxattr(dentry, &xattr_list);
+	if (xattr_list_len <= 0)
+		goto out;
+
+	for (name = xattr_list; name - xattr_list < xattr_list_len;
+			name += strlen(name) + 1) {
+		ksmbd_debug(VFS, "%s, len %zd\n", name, strlen(name));
+		if (strncasecmp(attr_name, name, attr_name_len))
+			continue;
+
+		value_len = ksmbd_vfs_getxattr(dentry,
+					       name,
+					       attr_value);
+		if (value_len < 0)
+			ksmbd_err("failed to get xattr in file\n");
+		break;
+	}
+
+out:
+	ksmbd_vfs_xattr_free(xattr_list);
+	return value_len;
+}
+
+static int ksmbd_vfs_stream_read(struct ksmbd_file *fp, char *buf, loff_t *pos,
+	size_t count)
+{
+	ssize_t v_len;
+	char *stream_buf = NULL;
+	int err;
+
+	ksmbd_debug(VFS, "read stream data pos : %llu, count : %zd\n",
+			*pos, count);
+
+	v_len = ksmbd_vfs_getcasexattr(fp->filp->f_path.dentry,
+				       fp->stream.name,
+				       fp->stream.size,
+				       &stream_buf);
+	if (v_len == -ENOENT) {
+		ksmbd_err("not found stream in xattr : %zd\n", v_len);
+		err = -ENOENT;
+		return err;
+	}
+
+	memcpy(buf, &stream_buf[*pos], count);
+	return v_len > count ? count : v_len;
+}
+
+/**
+ * check_lock_range() - vfs helper for smb byte range file locking
+ * @filp:	the file to apply the lock to
+ * @start:	lock start byte offset
+ * @end:	lock end byte offset
+ * @type:	byte range type read/write
+ *
+ * Return:	0 on success, otherwise error
+ */
+static int check_lock_range(struct file *filp,
+			    loff_t start,
+			    loff_t end,
+			    unsigned char type)
+{
+	struct file_lock *flock;
+	struct file_lock_context *ctx = file_inode(filp)->i_flctx;
+	int error = 0;
+
+	if (!ctx || list_empty_careful(&ctx->flc_posix))
+		return 0;
+
+	spin_lock(&ctx->flc_lock);
+	list_for_each_entry(flock, &ctx->flc_posix, fl_list) {
+		/* check conflict locks */
+		if (flock->fl_end >= start && end >= flock->fl_start) {
+			if (flock->fl_type == F_RDLCK) {
+				if (type == WRITE) {
+					ksmbd_err("not allow write by shared lock\n");
+					error = 1;
+					goto out;
+				}
+			} else if (flock->fl_type == F_WRLCK) {
+				/* check owner in lock */
+				if (flock->fl_file != filp) {
+					error = 1;
+					ksmbd_err("not allow rw access by exclusive lock from other opens\n");
+					goto out;
+				}
+			}
+		}
+	}
+out:
+	spin_unlock(&ctx->flc_lock);
+	return error;
+}
+
+/**
+ * ksmbd_vfs_read() - vfs helper for smb file read
+ * @work:	smb work
+ * @fid:	file id of open file
+ * @count:	read byte count
+ * @pos:	file pos
+ *
+ * Return:	number of read bytes on success, otherwise error
+ */
+int ksmbd_vfs_read(struct ksmbd_work *work,
+		 struct ksmbd_file *fp,
+		 size_t count,
+		 loff_t *pos)
+{
+	struct file *filp;
+	ssize_t nbytes = 0;
+	char *rbuf, *name;
+	struct inode *inode;
+	char namebuf[NAME_MAX];
+	int ret;
+
+	rbuf = AUX_PAYLOAD(work);
+	filp = fp->filp;
+	inode = d_inode(filp->f_path.dentry);
+	if (S_ISDIR(inode->i_mode))
+		return -EISDIR;
+
+	if (unlikely(count == 0))
+		return 0;
+
+	if (work->conn->connection_type) {
+		if (!(fp->daccess & (FILE_READ_DATA_LE | FILE_EXECUTE_LE))) {
+			ksmbd_err("no right to read(%s)\n", FP_FILENAME(fp));
+			return -EACCES;
+		}
+	}
+
+	if (ksmbd_stream_fd(fp))
+		return ksmbd_vfs_stream_read(fp, rbuf, pos, count);
+
+	ret = check_lock_range(filp, *pos, *pos + count - 1,
+			READ);
+	if (ret) {
+		ksmbd_err("unable to read due to lock\n");
+		return -EAGAIN;
+	}
+
+	nbytes = kernel_read(filp, rbuf, count, pos);
+	if (nbytes < 0) {
+		name = d_path(&filp->f_path, namebuf, sizeof(namebuf));
+		if (IS_ERR(name))
+			name = "(error)";
+		ksmbd_err("smb read failed for (%s), err = %zd\n",
+				name, nbytes);
+		return nbytes;
+	}
+
+	filp->f_pos = *pos;
+	return nbytes;
+}
+
+static int ksmbd_vfs_stream_write(struct ksmbd_file *fp, char *buf, loff_t *pos,
+	size_t count)
+{
+	char *stream_buf = NULL, *wbuf;
+	size_t size, v_len;
+	int err = 0;
+
+	ksmbd_debug(VFS, "write stream data pos : %llu, count : %zd\n",
+			*pos, count);
+
+	size = *pos + count;
+	if (size > XATTR_SIZE_MAX) {
+		size = XATTR_SIZE_MAX;
+		count = (*pos + count) - XATTR_SIZE_MAX;
+	}
+
+	v_len = ksmbd_vfs_getcasexattr(fp->filp->f_path.dentry,
+				       fp->stream.name,
+				       fp->stream.size,
+				       &stream_buf);
+	if (v_len == -ENOENT) {
+		ksmbd_err("not found stream in xattr : %zd\n", v_len);
+		err = -ENOENT;
+		goto out;
+	}
+
+	if (v_len < size) {
+		wbuf = ksmbd_alloc(size);
+		if (!wbuf) {
+			err = -ENOMEM;
+			goto out;
+		}
+
+		if (v_len > 0)
+			memcpy(wbuf, stream_buf, v_len);
+		stream_buf = wbuf;
+	}
+
+	memcpy(&stream_buf[*pos], buf, count);
+
+	err = ksmbd_vfs_setxattr(fp->filp->f_path.dentry,
+				 fp->stream.name,
+				 (void *)stream_buf,
+				 size,
+				 0);
+	if (err < 0)
+		goto out;
+
+	fp->filp->f_pos = *pos;
+	err = 0;
+out:
+	ksmbd_free(stream_buf);
+	return err;
+}
+
+/**
+ * ksmbd_vfs_write() - vfs helper for smb file write
+ * @work:	work
+ * @fid:	file id of open file
+ * @buf:	buf containing data for writing
+ * @count:	read byte count
+ * @pos:	file pos
+ * @sync:	fsync after write
+ * @written:	number of bytes written
+ *
+ * Return:	0 on success, otherwise error
+ */
+int ksmbd_vfs_write(struct ksmbd_work *work, struct ksmbd_file *fp,
+	char *buf, size_t count, loff_t *pos, bool sync, ssize_t *written)
+{
+	struct ksmbd_session *sess = work->sess;
+	struct file *filp;
+	loff_t	offset = *pos;
+	int err = 0;
+
+	if (sess->conn->connection_type) {
+		if (!(fp->daccess & FILE_WRITE_DATA_LE)) {
+			ksmbd_err("no right to write(%s)\n", FP_FILENAME(fp));
+			err = -EACCES;
+			goto out;
+		}
+	}
+
+	filp = fp->filp;
+
+	if (ksmbd_stream_fd(fp)) {
+		err = ksmbd_vfs_stream_write(fp, buf, pos, count);
+		if (!err)
+			*written = count;
+		goto out;
+	}
+
+	err = check_lock_range(filp, *pos, *pos + count - 1, WRITE);
+	if (err) {
+		ksmbd_err("unable to write due to lock\n");
+		err = -EAGAIN;
+		goto out;
+	}
+
+	/* Do we need to break any of a levelII oplock? */
+	smb_break_all_levII_oplock(work, fp, 1);
+
+	err = kernel_write(filp, buf, count, pos);
+	if (err < 0) {
+		ksmbd_debug(VFS, "smb write failed, err = %d\n", err);
+		goto out;
+	}
+
+	filp->f_pos = *pos;
+	*written = err;
+	err = 0;
+	if (sync) {
+		err = vfs_fsync_range(filp, offset, offset + *written, 0);
+		if (err < 0)
+			ksmbd_err("fsync failed for filename = %s, err = %d\n",
+					FP_FILENAME(fp), err);
+	}
+
+out:
+	return err;
+}
+
+/**
+ * ksmbd_vfs_getattr() - vfs helper for smb getattr
+ * @work:	work
+ * @fid:	file id of open file
+ * @attrs:	inode attributes
+ *
+ * Return:	0 on success, otherwise error
+ */
+int ksmbd_vfs_getattr(struct path *path, struct kstat *stat)
+{
+	int err;
+
+	err = vfs_getattr(path, stat, STATX_BTIME, AT_STATX_SYNC_AS_STAT);
+	if (err)
+		ksmbd_err("getattr failed, err %d\n", err);
+	return err;
+}
+
+/**
+ * ksmbd_vfs_fsync() - vfs helper for smb fsync
+ * @work:	work
+ * @fid:	file id of open file
+ *
+ * Return:	0 on success, otherwise error
+ */
+int ksmbd_vfs_fsync(struct ksmbd_work *work, uint64_t fid, uint64_t p_id)
+{
+	struct ksmbd_file *fp;
+	int err;
+
+	fp = ksmbd_lookup_fd_slow(work, fid, p_id);
+	if (!fp) {
+		ksmbd_err("failed to get filp for fid %llu\n", fid);
+		return -ENOENT;
+	}
+	err = vfs_fsync(fp->filp, 0);
+	if (err < 0)
+		ksmbd_err("smb fsync failed, err = %d\n", err);
+	ksmbd_fd_put(work, fp);
+	return err;
+}
+
+/**
+ * ksmbd_vfs_remove_file() - vfs helper for smb rmdir or unlink
+ * @name:	absolute directory or file name
+ *
+ * Return:	0 on success, otherwise error
+ */
+int ksmbd_vfs_remove_file(struct ksmbd_work *work, char *name)
+{
+	struct path parent;
+	struct dentry *dir, *dentry;
+	char *last;
+	int err = -ENOENT;
+
+	last = extract_last_component(name);
+	if (!last)
+		return -ENOENT;
+
+	if (ksmbd_override_fsids(work))
+		return -ENOMEM;
+
+	err = kern_path(name, LOOKUP_FOLLOW | LOOKUP_DIRECTORY, &parent);
+	if (err) {
+		ksmbd_debug(VFS, "can't get %s, err %d\n", name, err);
+		ksmbd_revert_fsids(work);
+		rollback_path_modification(last);
+		return err;
+	}
+
+	dir = parent.dentry;
+	if (!d_inode(dir))
+		goto out;
+
+	inode_lock_nested(d_inode(dir), I_MUTEX_PARENT);
+	dentry = lookup_one_len(last, dir, strlen(last));
+	if (IS_ERR(dentry)) {
+		err = PTR_ERR(dentry);
+		ksmbd_debug(VFS, "%s: lookup failed, err %d\n", last, err);
+		goto out_err;
+	}
+
+	if (!d_inode(dentry) || !d_inode(dentry)->i_nlink) {
+		dput(dentry);
+		err = -ENOENT;
+		goto out_err;
+	}
+
+	if (S_ISDIR(d_inode(dentry)->i_mode)) {
+		err = vfs_rmdir(&init_user_ns, d_inode(dir), dentry);
+		if (err && err != -ENOTEMPTY)
+			ksmbd_debug(VFS, "%s: rmdir failed, err %d\n", name,
+				err);
+	} else {
+		err = vfs_unlink(&init_user_ns, d_inode(dir), dentry, NULL);
+		if (err)
+			ksmbd_debug(VFS, "%s: unlink failed, err %d\n", name,
+				err);
+	}
+
+	dput(dentry);
+out_err:
+	inode_unlock(d_inode(dir));
+out:
+	rollback_path_modification(last);
+	path_put(&parent);
+	ksmbd_revert_fsids(work);
+	return err;
+}
+
+/**
+ * ksmbd_vfs_link() - vfs helper for creating smb hardlink
+ * @oldname:	source file name
+ * @newname:	hardlink name
+ *
+ * Return:	0 on success, otherwise error
+ */
+int ksmbd_vfs_link(struct ksmbd_work *work,
+		const char *oldname, const char *newname)
+{
+	struct path oldpath, newpath;
+	struct dentry *dentry;
+	int err;
+
+	if (ksmbd_override_fsids(work))
+		return -ENOMEM;
+
+	err = kern_path(oldname, LOOKUP_FOLLOW, &oldpath);
+	if (err) {
+		ksmbd_err("cannot get linux path for %s, err = %d\n",
+				oldname, err);
+		goto out1;
+	}
+
+	dentry = kern_path_create(AT_FDCWD, newname, &newpath,
+			LOOKUP_FOLLOW | LOOKUP_REVAL);
+	if (IS_ERR(dentry)) {
+		err = PTR_ERR(dentry);
+		ksmbd_err("path create err for %s, err %d\n", newname, err);
+		goto out2;
+	}
+
+	err = -EXDEV;
+	if (oldpath.mnt != newpath.mnt) {
+		ksmbd_err("vfs_link failed err %d\n", err);
+		goto out3;
+	}
+
+	err = vfs_link(oldpath.dentry, &init_user_ns, d_inode(newpath.dentry),
+			dentry, NULL);
+	if (err)
+		ksmbd_debug(VFS, "vfs_link failed err %d\n", err);
+
+out3:
+	done_path_create(&newpath, dentry);
+out2:
+	path_put(&oldpath);
+out1:
+	ksmbd_revert_fsids(work);
+	return err;
+}
+
+static int __ksmbd_vfs_rename(struct ksmbd_work *work,
+			      struct dentry *src_dent_parent,
+			      struct dentry *src_dent,
+			      struct dentry *dst_dent_parent,
+			      struct dentry *trap_dent,
+			      char *dst_name)
+{
+	struct dentry *dst_dent;
+	int err;
+
+	spin_lock(&src_dent->d_lock);
+	list_for_each_entry(dst_dent, &src_dent->d_subdirs, d_child) {
+		struct ksmbd_file *child_fp;
+
+		if (d_really_is_negative(dst_dent))
+			continue;
+
+		child_fp = ksmbd_lookup_fd_inode(d_inode(dst_dent));
+		if (child_fp) {
+			spin_unlock(&src_dent->d_lock);
+			ksmbd_debug(VFS, "Forbid rename, sub file/dir is in use\n");
+			return -EACCES;
+		}
+	}
+	spin_unlock(&src_dent->d_lock);
+
+	if (d_really_is_negative(src_dent_parent))
+		return -ENOENT;
+	if (d_really_is_negative(dst_dent_parent))
+		return -ENOENT;
+	if (d_really_is_negative(src_dent))
+		return -ENOENT;
+	if (src_dent == trap_dent)
+		return -EINVAL;
+
+	if (ksmbd_override_fsids(work))
+		return -ENOMEM;
+
+	dst_dent = lookup_one_len(dst_name, dst_dent_parent, strlen(dst_name));
+	err = PTR_ERR(dst_dent);
+	if (IS_ERR(dst_dent)) {
+		ksmbd_err("lookup failed %s [%d]\n", dst_name, err);
+		goto out;
+	}
+
+	err = -ENOTEMPTY;
+	if (dst_dent != trap_dent && !d_really_is_positive(dst_dent)) {
+		struct renamedata rd = {
+			.old_mnt_userns	= &init_user_ns,
+			.old_dir	= d_inode(src_dent_parent),
+			.old_dentry	= src_dent,
+			.new_mnt_userns	= &init_user_ns,
+			.new_dir	= d_inode(dst_dent_parent),
+			.new_dentry	= dst_dent,
+		};
+		err = vfs_rename(&rd);
+	}
+	if (err)
+		ksmbd_err("vfs_rename failed err %d\n", err);
+	if (dst_dent)
+		dput(dst_dent);
+out:
+	ksmbd_revert_fsids(work);
+	return err;
+}
+
+int ksmbd_vfs_fp_rename(struct ksmbd_work *work, struct ksmbd_file *fp,
+		char *newname)
+{
+	struct path dst_path;
+	struct dentry *src_dent_parent, *dst_dent_parent;
+	struct dentry *src_dent, *trap_dent;
+	char *dst_name;
+	int err;
+
+	dst_name = extract_last_component(newname);
+	if (!dst_name)
+		return -EINVAL;
+
+	src_dent_parent = dget_parent(fp->filp->f_path.dentry);
+	if (!src_dent_parent)
+		return -EINVAL;
+
+	src_dent = fp->filp->f_path.dentry;
+	dget(src_dent);
+
+	err = kern_path(newname, LOOKUP_FOLLOW | LOOKUP_DIRECTORY, &dst_path);
+	if (err) {
+		ksmbd_debug(VFS, "Cannot get path for %s [%d]\n", newname, err);
+		goto out;
+	}
+	dst_dent_parent = dst_path.dentry;
+	dget(dst_dent_parent);
+
+	trap_dent = lock_rename(src_dent_parent, dst_dent_parent);
+	err = __ksmbd_vfs_rename(work,
+				 src_dent_parent,
+				 src_dent,
+				 dst_dent_parent,
+				 trap_dent,
+				 dst_name);
+	unlock_rename(src_dent_parent, dst_dent_parent);
+	dput(dst_dent_parent);
+	path_put(&dst_path);
+out:
+	dput(src_dent);
+	dput(src_dent_parent);
+	return err;
+}
+
+/**
+ * ksmbd_vfs_truncate() - vfs helper for smb file truncate
+ * @work:	work
+ * @name:	old filename
+ * @fid:	file id of old file
+ * @size:	truncate to given size
+ *
+ * Return:	0 on success, otherwise error
+ */
+int ksmbd_vfs_truncate(struct ksmbd_work *work, const char *name,
+	struct ksmbd_file *fp, loff_t size)
+{
+	struct path path;
+	int err = 0;
+	struct inode *inode;
+
+	if (name) {
+		err = kern_path(name, 0, &path);
+		if (err) {
+			ksmbd_err("cannot get linux path for %s, err %d\n",
+					name, err);
+			return err;
+		}
+		err = vfs_truncate(&path, size);
+		if (err)
+			ksmbd_err("truncate failed for %s err %d\n",
+					name, err);
+		path_put(&path);
+	} else {
+		struct file *filp;
+
+		filp = fp->filp;
+
+		/* Do we need to break any of a levelII oplock? */
+		smb_break_all_levII_oplock(work, fp, 1);
+
+		inode = file_inode(filp);
+		if (size < inode->i_size) {
+			err = check_lock_range(filp, size,
+					inode->i_size - 1, WRITE);
+		} else {
+			err = check_lock_range(filp, inode->i_size,
+					size - 1, WRITE);
+		}
+
+		if (err) {
+			ksmbd_err("failed due to lock\n");
+			return -EAGAIN;
+		}
+
+		err = vfs_truncate(&filp->f_path, size);
+		if (err)
+			ksmbd_err("truncate failed for filename : %s err %d\n",
+					fp->filename, err);
+	}
+
+	return err;
+}
+
+/**
+ * ksmbd_vfs_listxattr() - vfs helper for smb list extended attributes
+ * @dentry:	dentry of file for listing xattrs
+ * @list:	destination buffer
+ * @size:	destination buffer length
+ *
+ * Return:	xattr list length on success, otherwise error
+ */
+ssize_t ksmbd_vfs_listxattr(struct dentry *dentry, char **list)
+{
+	ssize_t size;
+	char *vlist = NULL;
+
+	size = vfs_listxattr(dentry, NULL, 0);
+	if (size <= 0)
+		return size;
+
+	vlist = ksmbd_alloc(size);
+	if (!vlist)
+		return -ENOMEM;
+
+	*list = vlist;
+	size = vfs_listxattr(dentry, vlist, size);
+	if (size < 0) {
+		ksmbd_debug(VFS, "listxattr failed\n");
+		ksmbd_vfs_xattr_free(vlist);
+		*list = NULL;
+	}
+
+	return size;
+}
+
+static ssize_t ksmbd_vfs_xattr_len(struct dentry *dentry,
+			   char *xattr_name)
+{
+	return vfs_getxattr(&init_user_ns, dentry, xattr_name, NULL, 0);
+}
+
+/**
+ * ksmbd_vfs_getxattr() - vfs helper for smb get extended attributes value
+ * @dentry:	dentry of file for getting xattrs
+ * @xattr_name:	name of xattr name to query
+ * @xattr_buf:	destination buffer xattr value
+ *
+ * Return:	read xattr value length on success, otherwise error
+ */
+ssize_t ksmbd_vfs_getxattr(struct dentry *dentry,
+			   char *xattr_name,
+			   char **xattr_buf)
+{
+	ssize_t xattr_len;
+	char *buf;
+
+	*xattr_buf = NULL;
+	xattr_len = ksmbd_vfs_xattr_len(dentry, xattr_name);
+	if (xattr_len < 0)
+		return xattr_len;
+
+	buf = kmalloc(xattr_len + 1, GFP_KERNEL);
+	if (!buf)
+		return -ENOMEM;
+
+	xattr_len = vfs_getxattr(&init_user_ns, dentry, xattr_name, (void *)buf,
+			xattr_len);
+	if (xattr_len > 0)
+		*xattr_buf = buf;
+	else
+		kfree(buf);
+	return xattr_len;
+}
+
+/**
+ * ksmbd_vfs_setxattr() - vfs helper for smb set extended attributes value
+ * @dentry:	dentry to set XATTR at
+ * @name:	xattr name for setxattr
+ * @value:	xattr value to set
+ * @size:	size of xattr value
+ * @flags:	destination buffer length
+ *
+ * Return:	0 on success, otherwise error
+ */
+int ksmbd_vfs_setxattr(struct dentry *dentry,
+		       const char *attr_name,
+		       const void *attr_value,
+		       size_t attr_size,
+		       int flags)
+{
+	int err;
+
+	err = vfs_setxattr(&init_user_ns, dentry,
+			   attr_name,
+			   attr_value,
+			   attr_size,
+			   flags);
+	if (err)
+		ksmbd_debug(VFS, "setxattr failed, err %d\n", err);
+	return err;
+}
+
+/**
+ * ksmbd_vfs_set_fadvise() - convert smb IO caching options to linux options
+ * @filp:	file pointer for IO
+ * @options:	smb IO options
+ */
+void ksmbd_vfs_set_fadvise(struct file *filp, __le32 option)
+{
+	struct address_space *mapping;
+
+	mapping = filp->f_mapping;
+
+	if (!option || !mapping)
+		return;
+
+	if (option & FILE_WRITE_THROUGH_LE)
+		filp->f_flags |= O_SYNC;
+	else if (option & FILE_SEQUENTIAL_ONLY_LE) {
+		filp->f_ra.ra_pages = inode_to_bdi(mapping->host)->ra_pages * 2;
+		spin_lock(&filp->f_lock);
+		filp->f_mode &= ~FMODE_RANDOM;
+		spin_unlock(&filp->f_lock);
+	} else if (option & FILE_RANDOM_ACCESS_LE) {
+		spin_lock(&filp->f_lock);
+		filp->f_mode |= FMODE_RANDOM;
+		spin_unlock(&filp->f_lock);
+	}
+}
+
+/**
+ * ksmbd_vfs_lock() - vfs helper for smb file locking
+ * @filp:	the file to apply the lock to
+ * @cmd:	type of locking operation (F_SETLK, F_GETLK, etc.)
+ * @flock:	The lock to be applied
+ *
+ * Return:	0 on success, otherwise error
+ */
+int ksmbd_vfs_lock(struct file *filp, int cmd,
+			struct file_lock *flock)
+{
+	ksmbd_debug(VFS, "calling vfs_lock_file\n");
+	return vfs_lock_file(filp, cmd, flock, NULL);
+}
+
+int ksmbd_vfs_readdir(struct file *file, struct ksmbd_readdir_data *rdata)
+{
+	return iterate_dir(file, &rdata->ctx);
+}
+
+int ksmbd_vfs_alloc_size(struct ksmbd_work *work,
+			 struct ksmbd_file *fp,
+			 loff_t len)
+{
+	smb_break_all_levII_oplock(work, fp, 1);
+	return vfs_fallocate(fp->filp, FALLOC_FL_KEEP_SIZE, 0, len);
+}
+
+int ksmbd_vfs_zero_data(struct ksmbd_work *work,
+			 struct ksmbd_file *fp,
+			 loff_t off,
+			 loff_t len)
+{
+	smb_break_all_levII_oplock(work, fp, 1);
+	if (fp->f_ci->m_fattr & ATTR_SPARSE_FILE_LE)
+		return vfs_fallocate(fp->filp,
+			FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE, off, len);
+
+	return vfs_fallocate(fp->filp, FALLOC_FL_ZERO_RANGE, off, len);
+}
+
+int ksmbd_vfs_fqar_lseek(struct ksmbd_file *fp, loff_t start, loff_t length,
+	struct file_allocated_range_buffer *ranges,
+	int in_count, int *out_count)
+{
+	struct file *f = fp->filp;
+	struct inode *inode = FP_INODE(fp);
+	loff_t maxbytes = (u64)inode->i_sb->s_maxbytes, end;
+	loff_t extent_start, extent_end;
+	int ret = 0;
+
+	if (start > maxbytes)
+		return -EFBIG;
+
+	if (!in_count)
+		return 0;
+
+	/*
+	 * Shrink request scope to what the fs can actually handle.
+	 */
+	if (length > maxbytes || (maxbytes - length) < start)
+		length = maxbytes - start;
+
+	if (start + length > inode->i_size)
+		length = inode->i_size - start;
+
+	*out_count = 0;
+	end = start + length;
+	while (start < end && *out_count < in_count) {
+		extent_start = f->f_op->llseek(f, start, SEEK_DATA);
+		if (extent_start < 0) {
+			if (extent_start != -ENXIO)
+				ret = (int)extent_start;
+			break;
+		}
+
+		if (extent_start >= end)
+			break;
+
+		extent_end = f->f_op->llseek(f, extent_start, SEEK_HOLE);
+		if (extent_end < 0) {
+			if (extent_end != -ENXIO)
+				ret = (int)extent_end;
+			break;
+		} else if (extent_start >= extent_end)
+			break;
+
+		ranges[*out_count].file_offset = cpu_to_le64(extent_start);
+		ranges[(*out_count)++].length =
+			cpu_to_le64(min(extent_end, end) - extent_start);
+
+		start = extent_end;
+	}
+
+	return ret;
+}
+
+int ksmbd_vfs_remove_xattr(struct dentry *dentry, char *attr_name)
+{
+	return vfs_removexattr(&init_user_ns, dentry, attr_name);
+}
+
+void ksmbd_vfs_xattr_free(char *xattr)
+{
+	ksmbd_free(xattr);
+}
+
+int ksmbd_vfs_unlink(struct dentry *dir, struct dentry *dentry)
+{
+	int err = 0;
+
+	dget(dentry);
+	inode_lock_nested(d_inode(dir), I_MUTEX_PARENT);
+	if (!d_inode(dentry) || !d_inode(dentry)->i_nlink) {
+		err = -ENOENT;
+		goto out;
+	}
+
+	if (S_ISDIR(d_inode(dentry)->i_mode))
+		err = vfs_rmdir(&init_user_ns, d_inode(dir), dentry);
+	else
+		err = vfs_unlink(&init_user_ns, d_inode(dir), dentry, NULL);
+
+out:
+	inode_unlock(d_inode(dir));
+	dput(dentry);
+	if (err)
+		ksmbd_debug(VFS, "failed to delete, err %d\n", err);
+
+	return err;
+}
+
+/*
+ * ksmbd_vfs_get_logical_sector_size() - get logical sector size from inode
+ * @inode: inode
+ *
+ * Return: logical sector size
+ */
+unsigned short ksmbd_vfs_logical_sector_size(struct inode *inode)
+{
+	struct request_queue *q;
+	unsigned short ret_val = 512;
+
+	if (!inode->i_sb->s_bdev)
+		return ret_val;
+
+	q = inode->i_sb->s_bdev->bd_disk->queue;
+
+	if (q && q->limits.logical_block_size)
+		ret_val = q->limits.logical_block_size;
+
+	return ret_val;
+}
+
+/*
+ * ksmbd_vfs_get_smb2_sector_size() - get fs sector sizes
+ * @inode: inode
+ * @fs_ss: fs sector size struct
+ */
+void ksmbd_vfs_smb2_sector_size(struct inode *inode,
+	struct ksmbd_fs_sector_size *fs_ss)
+{
+	struct request_queue *q;
+
+	fs_ss->logical_sector_size = 512;
+	fs_ss->physical_sector_size = 512;
+	fs_ss->optimal_io_size = 512;
+
+	if (!inode->i_sb->s_bdev)
+		return;
+
+	q = inode->i_sb->s_bdev->bd_disk->queue;
+
+	if (q) {
+		if (q->limits.logical_block_size)
+			fs_ss->logical_sector_size =
+				q->limits.logical_block_size;
+		if (q->limits.physical_block_size)
+			fs_ss->physical_sector_size =
+				q->limits.physical_block_size;
+		if (q->limits.io_opt)
+			fs_ss->optimal_io_size = q->limits.io_opt;
+	}
+}
+
+static int __dir_empty(struct dir_context *ctx,
+				   const char *name,
+				   int namlen,
+				   loff_t offset,
+				   u64 ino,
+				   unsigned int d_type)
+{
+	struct ksmbd_readdir_data *buf;
+
+	buf = container_of(ctx, struct ksmbd_readdir_data, ctx);
+	buf->dirent_count++;
+
+	if (buf->dirent_count > 2)
+		return -ENOTEMPTY;
+	return 0;
+}
+
+/**
+ * ksmbd_vfs_empty_dir() - check for empty directory
+ * @fp:	ksmbd file pointer
+ *
+ * Return:	true if directory empty, otherwise false
+ */
+int ksmbd_vfs_empty_dir(struct ksmbd_file *fp)
+{
+	int err;
+	struct ksmbd_readdir_data readdir_data;
+
+	memset(&readdir_data, 0, sizeof(struct ksmbd_readdir_data));
+
+	set_ctx_actor(&readdir_data.ctx, __dir_empty);
+	readdir_data.dirent_count = 0;
+
+	err = ksmbd_vfs_readdir(fp->filp, &readdir_data);
+	if (readdir_data.dirent_count > 2)
+		err = -ENOTEMPTY;
+	else
+		err = 0;
+	return err;
+}
+
+static int __caseless_lookup(struct dir_context *ctx,
+			     const char *name,
+			     int namlen,
+			     loff_t offset,
+			     u64 ino,
+			     unsigned int d_type)
+{
+	struct ksmbd_readdir_data *buf;
+
+	buf = container_of(ctx, struct ksmbd_readdir_data, ctx);
+
+	if (buf->used != namlen)
+		return 0;
+	if (!strncasecmp((char *)buf->private, name, namlen)) {
+		memcpy((char *)buf->private, name, namlen);
+		buf->dirent_count = 1;
+		return -EEXIST;
+	}
+	return 0;
+}
+
+/**
+ * ksmbd_vfs_lookup_in_dir() - lookup a file in a directory
+ * @dirname:	directory name
+ * @filename:	filename to lookup
+ *
+ * Return:	0 on success, otherwise error
+ */
+static int ksmbd_vfs_lookup_in_dir(char *dirname, char *filename)
+{
+	struct path dir_path;
+	int ret;
+	struct file *dfilp;
+	int flags = O_RDONLY|O_LARGEFILE;
+	int dirnamelen = strlen(dirname);
+	struct ksmbd_readdir_data readdir_data = {
+		.ctx.actor	= __caseless_lookup,
+		.private	= filename,
+		.used		= strlen(filename),
+	};
+
+	ret = ksmbd_vfs_kern_path(dirname, 0, &dir_path, true);
+	if (ret)
+		goto error;
+
+	dfilp = dentry_open(&dir_path, flags, current_cred());
+	if (IS_ERR(dfilp)) {
+		path_put(&dir_path);
+		ksmbd_err("cannot open directory %s\n", dirname);
+		ret = -EINVAL;
+		goto error;
+	}
+
+	ret = ksmbd_vfs_readdir(dfilp, &readdir_data);
+	if (readdir_data.dirent_count > 0)
+		ret = 0;
+
+	fput(dfilp);
+	path_put(&dir_path);
+error:
+	dirname[dirnamelen] = '/';
+	return ret;
+}
+
+/**
+ * ksmbd_vfs_kern_path() - lookup a file and get path info
+ * @name:	name of file for lookup
+ * @flags:	lookup flags
+ * @path:	if lookup succeed, return path info
+ * @caseless:	caseless filename lookup
+ *
+ * Return:	0 on success, otherwise error
+ */
+int ksmbd_vfs_kern_path(char *name, unsigned int flags, struct path *path,
+		bool caseless)
+{
+	char *filename = NULL;
+	int err;
+
+	err = kern_path(name, flags, path);
+	if (!err)
+		return err;
+
+	if (caseless) {
+		filename = extract_last_component(name);
+		if (!filename)
+			goto out;
+
+		/* root reached */
+		if (strlen(name) == 0)
+			goto out;
+
+		err = ksmbd_vfs_lookup_in_dir(name, filename);
+		if (err)
+			goto out;
+		err = kern_path(name, flags, path);
+	}
+
+out:
+	rollback_path_modification(filename);
+	return err;
+}
+
+int ksmbd_vfs_remove_acl_xattrs(struct dentry *dentry)
+{
+	char *name, *xattr_list = NULL;
+	ssize_t xattr_list_len;
+	int err = 0;
+
+	xattr_list_len = ksmbd_vfs_listxattr(dentry, &xattr_list);
+	if (xattr_list_len < 0) {
+		goto out;
+	} else if (!xattr_list_len) {
+		ksmbd_debug(SMB, "empty xattr in the file\n");
+		goto out;
+	}
+
+	for (name = xattr_list; name - xattr_list < xattr_list_len;
+			name += strlen(name) + 1) {
+		ksmbd_debug(SMB, "%s, len %zd\n", name, strlen(name));
+
+		if (!strncmp(name, XATTR_NAME_POSIX_ACL_ACCESS,
+			     sizeof(XATTR_NAME_POSIX_ACL_ACCESS)-1) ||
+		    !strncmp(name, XATTR_NAME_POSIX_ACL_DEFAULT,
+			     sizeof(XATTR_NAME_POSIX_ACL_DEFAULT)-1)) {
+			err = ksmbd_vfs_remove_xattr(dentry, name);
+			if (err)
+				ksmbd_debug(SMB,
+					"remove acl xattr failed : %s\n", name);
+		}
+	}
+out:
+	ksmbd_vfs_xattr_free(xattr_list);
+	return err;
+}
+
+int ksmbd_vfs_remove_sd_xattrs(struct dentry *dentry)
+{
+	char *name, *xattr_list = NULL;
+	ssize_t xattr_list_len;
+	int err = 0;
+
+	xattr_list_len = ksmbd_vfs_listxattr(dentry, &xattr_list);
+	if (xattr_list_len < 0) {
+		goto out;
+	} else if (!xattr_list_len) {
+		ksmbd_debug(SMB, "empty xattr in the file\n");
+		goto out;
+	}
+
+	for (name = xattr_list; name - xattr_list < xattr_list_len;
+			name += strlen(name) + 1) {
+		ksmbd_debug(SMB, "%s, len %zd\n", name, strlen(name));
+
+		if (!strncmp(name, XATTR_NAME_SD, XATTR_NAME_SD_LEN)) {
+			err = ksmbd_vfs_remove_xattr(dentry, name);
+			if (err)
+				ksmbd_debug(SMB, "remove xattr failed : %s\n", name);
+		}
+	}
+out:
+	ksmbd_vfs_xattr_free(xattr_list);
+	return err;
+}
+
+static struct xattr_smb_acl *ksmbd_vfs_make_xattr_posix_acl(struct inode *inode,
+		int acl_type)
+{
+	struct xattr_smb_acl *smb_acl = NULL;
+	struct posix_acl *posix_acls;
+	struct posix_acl_entry *pa_entry;
+	struct xattr_acl_entry *xa_entry;
+	int i;
+
+	posix_acls = ksmbd_vfs_get_acl(inode, acl_type);
+	if (!posix_acls)
+		return NULL;
+
+	smb_acl = kzalloc(sizeof(struct xattr_smb_acl) +
+			  sizeof(struct xattr_acl_entry) * posix_acls->a_count,
+			  GFP_KERNEL);
+	if (!smb_acl)
+		goto out;
+
+	smb_acl->count = posix_acls->a_count;
+	pa_entry = posix_acls->a_entries;
+	xa_entry = smb_acl->entries;
+	for (i = 0; i < posix_acls->a_count; i++, pa_entry++, xa_entry++) {
+		switch (pa_entry->e_tag) {
+		case ACL_USER:
+			xa_entry->type = SMB_ACL_USER;
+			xa_entry->uid = from_kuid(&init_user_ns, pa_entry->e_uid);
+			break;
+		case ACL_USER_OBJ:
+			xa_entry->type = SMB_ACL_USER_OBJ;
+			break;
+		case ACL_GROUP:
+			xa_entry->type = SMB_ACL_GROUP;
+			xa_entry->gid = from_kgid(&init_user_ns, pa_entry->e_gid);
+			break;
+		case ACL_GROUP_OBJ:
+			xa_entry->type = SMB_ACL_GROUP_OBJ;
+			break;
+		case ACL_OTHER:
+			xa_entry->type = SMB_ACL_OTHER;
+			break;
+		case ACL_MASK:
+			xa_entry->type = SMB_ACL_MASK;
+			break;
+		default:
+			ksmbd_err("unknown type : 0x%x\n", pa_entry->e_tag);
+			goto out;
+		}
+
+		if (pa_entry->e_perm & ACL_READ)
+			xa_entry->perm |= SMB_ACL_READ;
+		if (pa_entry->e_perm & ACL_WRITE)
+			xa_entry->perm |= SMB_ACL_WRITE;
+		if (pa_entry->e_perm & ACL_EXECUTE)
+			xa_entry->perm |= SMB_ACL_EXECUTE;
+	}
+out:
+	posix_acl_release(posix_acls);
+	return smb_acl;
+}
+
+int ksmbd_vfs_set_sd_xattr(struct ksmbd_conn *conn, struct dentry *dentry,
+		struct smb_ntsd *pntsd, int len)
+{
+	int rc;
+	struct ndr sd_ndr = {0}, acl_ndr = {0};
+	struct xattr_ntacl acl = {0};
+	struct xattr_smb_acl *smb_acl, *def_smb_acl = NULL;
+	struct inode *inode = dentry->d_inode;
+
+	acl.version = 4;
+	acl.hash_type = XATTR_SD_HASH_TYPE_SHA256;
+	acl.current_time = ksmbd_UnixTimeToNT(current_time(dentry->d_inode));
+
+	memcpy(acl.desc, "posix_acl", 9);
+	acl.desc_len = 10;
+
+	pntsd->osidoffset =
+		cpu_to_le32(le32_to_cpu(pntsd->osidoffset) + NDR_NTSD_OFFSETOF);
+	pntsd->gsidoffset =
+		cpu_to_le32(le32_to_cpu(pntsd->gsidoffset) + NDR_NTSD_OFFSETOF);
+	pntsd->dacloffset =
+		cpu_to_le32(le32_to_cpu(pntsd->dacloffset) + NDR_NTSD_OFFSETOF);
+
+	acl.sd_buf = (char *)pntsd;
+	acl.sd_size = len;
+
+	rc = ksmbd_gen_sd_hash(conn, acl.sd_buf, acl.sd_size, acl.hash);
+	if (rc) {
+		ksmbd_err("failed to generate hash for ndr acl\n");
+		return rc;
+	}
+
+	smb_acl = ksmbd_vfs_make_xattr_posix_acl(dentry->d_inode, ACL_TYPE_ACCESS);
+	if (S_ISDIR(inode->i_mode))
+		def_smb_acl = ksmbd_vfs_make_xattr_posix_acl(dentry->d_inode,
+				ACL_TYPE_DEFAULT);
+
+	rc = ndr_encode_posix_acl(&acl_ndr, inode, smb_acl, def_smb_acl);
+	if (rc) {
+		ksmbd_err("failed to encode ndr to posix acl\n");
+		goto out;
+	}
+
+	rc = ksmbd_gen_sd_hash(conn, acl_ndr.data, acl_ndr.offset,
+			acl.posix_acl_hash);
+	if (rc) {
+		ksmbd_err("failed to generate hash for ndr acl\n");
+		goto out;
+	}
+
+	rc = ndr_encode_v4_ntacl(&sd_ndr, &acl);
+	if (rc) {
+		ksmbd_err("failed to encode ndr to posix acl\n");
+		goto out;
+	}
+
+	rc = ksmbd_vfs_setxattr(dentry, XATTR_NAME_SD, sd_ndr.data,
+			sd_ndr.offset, 0);
+	if (rc < 0)
+		ksmbd_err("Failed to store XATTR ntacl :%d\n", rc);
+
+	kfree(sd_ndr.data);
+out:
+	kfree(acl_ndr.data);
+	kfree(smb_acl);
+	kfree(def_smb_acl);
+	return rc;
+}
+
+int ksmbd_vfs_get_sd_xattr(struct ksmbd_conn *conn, struct dentry *dentry,
+		struct smb_ntsd **pntsd)
+{
+	int rc;
+	struct ndr n;
+
+	rc = ksmbd_vfs_getxattr(dentry, XATTR_NAME_SD, &n.data);
+	if (rc > 0) {
+		struct inode *inode = dentry->d_inode;
+		struct ndr acl_ndr = {0};
+		struct xattr_ntacl acl;
+		struct xattr_smb_acl *smb_acl = NULL, *def_smb_acl = NULL;
+		__u8 cmp_hash[XATTR_SD_HASH_SIZE] = {0};
+
+		n.length = rc;
+		rc = ndr_decode_v4_ntacl(&n, &acl);
+		if (rc)
+			return rc;
+
+		smb_acl = ksmbd_vfs_make_xattr_posix_acl(inode,
+				ACL_TYPE_ACCESS);
+		if (S_ISDIR(inode->i_mode))
+			def_smb_acl = ksmbd_vfs_make_xattr_posix_acl(inode,
+					ACL_TYPE_DEFAULT);
+
+		rc = ndr_encode_posix_acl(&acl_ndr, inode, smb_acl, def_smb_acl);
+		if (rc) {
+			ksmbd_err("failed to encode ndr to posix acl\n");
+			goto out;
+		}
+
+		rc = ksmbd_gen_sd_hash(conn, acl_ndr.data, acl_ndr.offset,
+				cmp_hash);
+		if (rc) {
+			ksmbd_err("failed to generate hash for ndr acl\n");
+			goto out;
+		}
+
+		if (memcmp(cmp_hash, acl.posix_acl_hash, XATTR_SD_HASH_SIZE)) {
+			ksmbd_err("hash value diff\n");
+			rc = -EINVAL;
+			goto out;
+		}
+
+		*pntsd = acl.sd_buf;
+		(*pntsd)->osidoffset =
+			cpu_to_le32(le32_to_cpu((*pntsd)->osidoffset) - NDR_NTSD_OFFSETOF);
+		(*pntsd)->gsidoffset =
+			cpu_to_le32(le32_to_cpu((*pntsd)->gsidoffset) - NDR_NTSD_OFFSETOF);
+		(*pntsd)->dacloffset =
+			cpu_to_le32(le32_to_cpu((*pntsd)->dacloffset) - NDR_NTSD_OFFSETOF);
+
+		rc = acl.sd_size;
+out:
+		kfree(n.data);
+		kfree(acl_ndr.data);
+		kfree(smb_acl);
+		kfree(def_smb_acl);
+	}
+
+	return rc;
+}
+
+int ksmbd_vfs_set_dos_attrib_xattr(struct dentry *dentry,
+		struct xattr_dos_attrib *da)
+{
+	struct ndr n;
+	int err;
+
+	err = ndr_encode_dos_attr(&n, da);
+	if (err)
+		return err;
+
+	err = ksmbd_vfs_setxattr(dentry,
+			XATTR_NAME_DOS_ATTRIBUTE,
+			(void *)n.data,
+			n.offset,
+			0);
+	if (err)
+		ksmbd_debug(SMB, "failed to store dos attribute in xattr\n");
+	kfree(n.data);
+
+	return err;
+}
+
+int ksmbd_vfs_get_dos_attrib_xattr(struct dentry *dentry,
+		struct xattr_dos_attrib *da)
+{
+	struct ndr n;
+	int err;
+
+	err = ksmbd_vfs_getxattr(dentry,
+			XATTR_NAME_DOS_ATTRIBUTE,
+			(char **)&n.data);
+	if (err > 0) {
+		n.length = err;
+		if (ndr_decode_dos_attr(&n, da))
+			err = -EINVAL;
+		ksmbd_free(n.data);
+	} else
+		ksmbd_debug(SMB, "failed to load dos attribute in xattr\n");
+
+	return err;
+}
+
+struct posix_acl *ksmbd_vfs_posix_acl_alloc(int count, gfp_t flags)
+{
+#if IS_ENABLED(CONFIG_FS_POSIX_ACL)
+	return posix_acl_alloc(count, flags);
+#else
+	return NULL;
+#endif
+}
+
+struct posix_acl *ksmbd_vfs_get_acl(struct inode *inode, int type)
+{
+#if IS_ENABLED(CONFIG_FS_POSIX_ACL)
+	return get_acl(inode, type);
+#else
+	return NULL;
+#endif
+}
+
+int ksmbd_vfs_set_posix_acl(struct inode *inode, int type,
+		struct posix_acl *acl)
+{
+#if IS_ENABLED(CONFIG_FS_POSIX_ACL)
+	return set_posix_acl(&init_user_ns, inode, type, acl);
+#else
+	return -EOPNOTSUPP;
+#endif
+}
+
+/**
+ * ksmbd_vfs_init_kstat() - convert unix stat information to smb stat format
+ * @p:          destination buffer
+ * @ksmbd_kstat:      ksmbd kstat wrapper
+ */
+void *ksmbd_vfs_init_kstat(char **p, struct ksmbd_kstat *ksmbd_kstat)
+{
+	struct file_directory_info *info = (struct file_directory_info *)(*p);
+	struct kstat *kstat = ksmbd_kstat->kstat;
+	u64 time;
+
+	info->FileIndex = 0;
+	info->CreationTime = cpu_to_le64(ksmbd_kstat->create_time);
+	time = ksmbd_UnixTimeToNT(kstat->atime);
+	info->LastAccessTime = cpu_to_le64(time);
+	time = ksmbd_UnixTimeToNT(kstat->mtime);
+	info->LastWriteTime = cpu_to_le64(time);
+	time = ksmbd_UnixTimeToNT(kstat->ctime);
+	info->ChangeTime = cpu_to_le64(time);
+
+	if (ksmbd_kstat->file_attributes & ATTR_DIRECTORY_LE) {
+		info->EndOfFile = 0;
+		info->AllocationSize = 0;
+	} else {
+		info->EndOfFile = cpu_to_le64(kstat->size);
+		info->AllocationSize = cpu_to_le64(kstat->blocks << 9);
+	}
+	info->ExtFileAttributes = ksmbd_kstat->file_attributes;
+
+	return info;
+}
+
+int ksmbd_vfs_fill_dentry_attrs(struct ksmbd_work *work,
+				struct dentry *dentry,
+				struct ksmbd_kstat *ksmbd_kstat)
+{
+	u64 time;
+	int rc;
+
+	generic_fillattr(&init_user_ns, d_inode(dentry), ksmbd_kstat->kstat);
+
+	time = ksmbd_UnixTimeToNT(ksmbd_kstat->kstat->ctime);
+	ksmbd_kstat->create_time = time;
+
+	/*
+	 * set default value for the case that store dos attributes is not yes
+	 * or that acl is disable in server's filesystem and the config is yes.
+	 */
+	if (S_ISDIR(ksmbd_kstat->kstat->mode))
+		ksmbd_kstat->file_attributes = ATTR_DIRECTORY_LE;
+	else
+		ksmbd_kstat->file_attributes = ATTR_ARCHIVE_LE;
+
+	if (test_share_config_flag(work->tcon->share_conf,
+	    KSMBD_SHARE_FLAG_STORE_DOS_ATTRS)) {
+		struct xattr_dos_attrib da;
+
+		rc = ksmbd_vfs_get_dos_attrib_xattr(dentry, &da);
+		if (rc > 0) {
+			ksmbd_kstat->file_attributes = cpu_to_le32(da.attr);
+			ksmbd_kstat->create_time = da.create_time;
+		} else
+			ksmbd_debug(VFS, "fail to load dos attribute.\n");
+	}
+
+	return 0;
+}
+
+ssize_t ksmbd_vfs_casexattr_len(struct dentry *dentry,
+				char *attr_name,
+				int attr_name_len)
+{
+	char *name, *xattr_list = NULL;
+	ssize_t value_len = -ENOENT, xattr_list_len;
+
+	xattr_list_len = ksmbd_vfs_listxattr(dentry, &xattr_list);
+	if (xattr_list_len <= 0)
+		goto out;
+
+	for (name = xattr_list; name - xattr_list < xattr_list_len;
+			name += strlen(name) + 1) {
+		ksmbd_debug(VFS, "%s, len %zd\n", name, strlen(name));
+		if (strncasecmp(attr_name, name, attr_name_len))
+			continue;
+
+		value_len = ksmbd_vfs_xattr_len(dentry, name);
+		break;
+	}
+
+out:
+	ksmbd_vfs_xattr_free(xattr_list);
+	return value_len;
+}
+
+int ksmbd_vfs_xattr_stream_name(char *stream_name,
+				char **xattr_stream_name,
+				size_t *xattr_stream_name_size,
+				int s_type)
+{
+	int stream_name_size;
+	char *xattr_stream_name_buf;
+	char *type;
+	int type_len;
+
+	if (s_type == DIR_STREAM)
+		type = ":$INDEX_ALLOCATION";
+	else
+		type = ":$DATA";
+
+	type_len = strlen(type);
+	stream_name_size = strlen(stream_name);
+	*xattr_stream_name_size = stream_name_size + XATTR_NAME_STREAM_LEN + 1;
+	xattr_stream_name_buf = kmalloc(*xattr_stream_name_size + type_len,
+			GFP_KERNEL);
+	if (!xattr_stream_name_buf)
+		return -ENOMEM;
+
+	memcpy(xattr_stream_name_buf,
+		XATTR_NAME_STREAM,
+		XATTR_NAME_STREAM_LEN);
+
+	if (stream_name_size) {
+		memcpy(&xattr_stream_name_buf[XATTR_NAME_STREAM_LEN],
+			stream_name,
+			stream_name_size);
+	}
+	memcpy(&xattr_stream_name_buf[*xattr_stream_name_size - 1], type, type_len);
+		*xattr_stream_name_size += type_len;
+
+	xattr_stream_name_buf[*xattr_stream_name_size - 1] = '\0';
+	*xattr_stream_name = xattr_stream_name_buf;
+
+	return 0;
+}
+
+static int ksmbd_vfs_copy_file_range(struct file *file_in, loff_t pos_in,
+				struct file *file_out, loff_t pos_out,
+				size_t len)
+{
+	struct inode *inode_in = file_inode(file_in);
+	struct inode *inode_out = file_inode(file_out);
+	int ret;
+
+	ret = vfs_copy_file_range(file_in, pos_in, file_out, pos_out, len, 0);
+	/* do splice for the copy between different file systems */
+	if (ret != -EXDEV)
+		return ret;
+
+	if (S_ISDIR(inode_in->i_mode) || S_ISDIR(inode_out->i_mode))
+		return -EISDIR;
+	if (!S_ISREG(inode_in->i_mode) || !S_ISREG(inode_out->i_mode))
+		return -EINVAL;
+
+	if (!(file_in->f_mode & FMODE_READ) ||
+	    !(file_out->f_mode & FMODE_WRITE))
+		return -EBADF;
+
+	if (len == 0)
+		return 0;
+
+	file_start_write(file_out);
+
+	/*
+	 * skip the verification of the range of data. it will be done
+	 * in do_splice_direct
+	 */
+	ret = do_splice_direct(file_in, &pos_in, file_out, &pos_out,
+			len > MAX_RW_COUNT ? MAX_RW_COUNT : len, 0);
+	if (ret > 0) {
+		fsnotify_access(file_in);
+		add_rchar(current, ret);
+		fsnotify_modify(file_out);
+		add_wchar(current, ret);
+	}
+
+	inc_syscr(current);
+	inc_syscw(current);
+
+	file_end_write(file_out);
+	return ret;
+}
+
+int ksmbd_vfs_copy_file_ranges(struct ksmbd_work *work,
+				struct ksmbd_file *src_fp,
+				struct ksmbd_file *dst_fp,
+				struct srv_copychunk *chunks,
+				unsigned int chunk_count,
+				unsigned int *chunk_count_written,
+				unsigned int *chunk_size_written,
+				loff_t *total_size_written)
+{
+	unsigned int i;
+	loff_t src_off, dst_off, src_file_size;
+	size_t len;
+	int ret;
+
+	*chunk_count_written = 0;
+	*chunk_size_written = 0;
+	*total_size_written = 0;
+
+	if (!(src_fp->daccess & (FILE_READ_DATA_LE | FILE_EXECUTE_LE))) {
+		ksmbd_err("no right to read(%s)\n", FP_FILENAME(src_fp));
+		return -EACCES;
+	}
+	if (!(dst_fp->daccess & (FILE_WRITE_DATA_LE | FILE_APPEND_DATA_LE))) {
+		ksmbd_err("no right to write(%s)\n", FP_FILENAME(dst_fp));
+		return -EACCES;
+	}
+
+	if (ksmbd_stream_fd(src_fp) || ksmbd_stream_fd(dst_fp))
+		return -EBADF;
+
+	smb_break_all_levII_oplock(work, dst_fp, 1);
+
+	for (i = 0; i < chunk_count; i++) {
+		src_off = le64_to_cpu(chunks[i].SourceOffset);
+		dst_off = le64_to_cpu(chunks[i].TargetOffset);
+		len = le32_to_cpu(chunks[i].Length);
+
+		if (check_lock_range(src_fp->filp, src_off,
+				src_off + len - 1, READ))
+			return -EAGAIN;
+		if (check_lock_range(dst_fp->filp, dst_off,
+				dst_off + len - 1, WRITE))
+			return -EAGAIN;
+	}
+
+	src_file_size = i_size_read(file_inode(src_fp->filp));
+
+	for (i = 0; i < chunk_count; i++) {
+		src_off = le64_to_cpu(chunks[i].SourceOffset);
+		dst_off = le64_to_cpu(chunks[i].TargetOffset);
+		len = le32_to_cpu(chunks[i].Length);
+
+		if (src_off + len > src_file_size)
+			return -E2BIG;
+
+		ret = ksmbd_vfs_copy_file_range(src_fp->filp, src_off,
+				dst_fp->filp, dst_off, len);
+		if (ret < 0)
+			return ret;
+
+		*chunk_count_written += 1;
+		*total_size_written += ret;
+	}
+	return 0;
+}
+
+int ksmbd_vfs_posix_lock_wait(struct file_lock *flock)
+{
+	return wait_event_interruptible(flock->fl_wait, !flock->fl_blocker);
+}
+
+int ksmbd_vfs_posix_lock_wait_timeout(struct file_lock *flock, long timeout)
+{
+	return wait_event_interruptible_timeout(flock->fl_wait,
+						!flock->fl_blocker,
+						timeout);
+}
+
+void ksmbd_vfs_posix_lock_unblock(struct file_lock *flock)
+{
+	locks_delete_block(flock);
+}
+
+int ksmbd_vfs_set_init_posix_acl(struct inode *inode)
+{
+	struct posix_acl_state acl_state;
+	struct posix_acl *acls;
+	int rc;
+
+	ksmbd_debug(SMB, "Set posix acls\n");
+	rc = init_acl_state(&acl_state, 1);
+	if (rc)
+		return rc;
+
+	/* Set default owner group */
+	acl_state.owner.allow = (inode->i_mode & 0700) >> 6;
+	acl_state.group.allow = (inode->i_mode & 0070) >> 3;
+	acl_state.other.allow = inode->i_mode & 0007;
+	acl_state.users->aces[acl_state.users->n].uid = inode->i_uid;
+	acl_state.users->aces[acl_state.users->n++].perms.allow =
+		acl_state.owner.allow;
+	acl_state.groups->aces[acl_state.groups->n].gid = inode->i_gid;
+	acl_state.groups->aces[acl_state.groups->n++].perms.allow =
+		acl_state.group.allow;
+	acl_state.mask.allow = 0x07;
+
+	acls = ksmbd_vfs_posix_acl_alloc(6, GFP_KERNEL);
+	if (!acls) {
+		free_acl_state(&acl_state);
+		return -ENOMEM;
+	}
+	posix_state_to_acl(&acl_state, acls->a_entries);
+	rc = ksmbd_vfs_set_posix_acl(inode, ACL_TYPE_ACCESS, acls);
+	if (rc < 0)
+		ksmbd_debug(SMB, "Set posix acl(ACL_TYPE_ACCESS) failed, rc : %d\n",
+				rc);
+	else if (S_ISDIR(inode->i_mode)) {
+		posix_state_to_acl(&acl_state, acls->a_entries);
+		rc = ksmbd_vfs_set_posix_acl(inode, ACL_TYPE_DEFAULT, acls);
+		if (rc < 0)
+			ksmbd_debug(SMB, "Set posix acl(ACL_TYPE_DEFAULT) failed, rc : %d\n",
+					rc);
+	}
+	free_acl_state(&acl_state);
+	posix_acl_release(acls);
+	return rc;
+}
+
+int ksmbd_vfs_inherit_posix_acl(struct inode *inode, struct inode *parent_inode)
+{
+	struct posix_acl *acls;
+	struct posix_acl_entry *pace;
+	int rc, i;
+
+	acls = ksmbd_vfs_get_acl(parent_inode, ACL_TYPE_DEFAULT);
+	if (!acls)
+		return -ENOENT;
+	pace = acls->a_entries;
+
+	for (i = 0; i < acls->a_count; i++, pace++) {
+		if (pace->e_tag == ACL_MASK) {
+			pace->e_perm = 0x07;
+			break;
+		}
+	}
+
+	rc = ksmbd_vfs_set_posix_acl(inode, ACL_TYPE_ACCESS, acls);
+	if (rc < 0)
+		ksmbd_debug(SMB, "Set posix acl(ACL_TYPE_ACCESS) failed, rc : %d\n",
+				rc);
+	if (S_ISDIR(inode->i_mode)) {
+		rc = ksmbd_vfs_set_posix_acl(inode, ACL_TYPE_DEFAULT, acls);
+		if (rc < 0)
+			ksmbd_debug(SMB, "Set posix acl(ACL_TYPE_DEFAULT) failed, rc : %d\n",
+					rc);
+	}
+	posix_acl_release(acls);
+	return rc;
+}
diff --git a/fs/cifsd/vfs.h b/fs/cifsd/vfs.h
new file mode 100644
index 000000000000..bbef5c20c146
--- /dev/null
+++ b/fs/cifsd/vfs.h
@@ -0,0 +1,314 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ *   Copyright (C) 2016 Namjae Jeon <linkinjeon@kernel.org>
+ *   Copyright (C) 2018 Samsung Electronics Co., Ltd.
+ */
+
+#ifndef __KSMBD_VFS_H__
+#define __KSMBD_VFS_H__
+
+#include <linux/file.h>
+#include <linux/fs.h>
+#include <linux/namei.h>
+#include <uapi/linux/xattr.h>
+#include <linux/posix_acl.h>
+
+#include "smbacl.h"
+
+/* STREAM XATTR PREFIX */
+#define STREAM_PREFIX			"DosStream."
+#define STREAM_PREFIX_LEN		(sizeof(STREAM_PREFIX) - 1)
+#define XATTR_NAME_STREAM		(XATTR_USER_PREFIX STREAM_PREFIX)
+#define XATTR_NAME_STREAM_LEN		(sizeof(XATTR_NAME_STREAM) - 1)
+
+enum {
+	XATTR_DOSINFO_ATTRIB		= 0x00000001,
+	XATTR_DOSINFO_EA_SIZE		= 0x00000002,
+	XATTR_DOSINFO_SIZE		= 0x00000004,
+	XATTR_DOSINFO_ALLOC_SIZE	= 0x00000008,
+	XATTR_DOSINFO_CREATE_TIME	= 0x00000010,
+	XATTR_DOSINFO_CHANGE_TIME	= 0x00000020,
+	XATTR_DOSINFO_ITIME		= 0x00000040
+};
+
+struct xattr_dos_attrib {
+	__u16	version;
+	__u32	flags;
+	__u32	attr;
+	__u32	ea_size;
+	__u64	size;
+	__u64	alloc_size;
+	__u64	create_time;
+	__u64	change_time;
+	__u64	itime;
+};
+
+/* DOS ATTRIBUITE XATTR PREFIX */
+#define DOS_ATTRIBUTE_PREFIX		"DOSATTRIB"
+#define DOS_ATTRIBUTE_PREFIX_LEN	(sizeof(DOS_ATTRIBUTE_PREFIX) - 1)
+#define XATTR_NAME_DOS_ATTRIBUTE	\
+		(XATTR_USER_PREFIX DOS_ATTRIBUTE_PREFIX)
+#define XATTR_NAME_DOS_ATTRIBUTE_LEN	\
+		(sizeof(XATTR_USER_PREFIX DOS_ATTRIBUTE_PREFIX) - 1)
+
+#define XATTR_SD_HASH_TYPE_SHA256	0x1
+#define XATTR_SD_HASH_SIZE		64
+
+#define SMB_ACL_READ			4
+#define SMB_ACL_WRITE			2
+#define SMB_ACL_EXECUTE			1
+
+enum {
+	SMB_ACL_TAG_INVALID = 0,
+	SMB_ACL_USER,
+	SMB_ACL_USER_OBJ,
+	SMB_ACL_GROUP,
+	SMB_ACL_GROUP_OBJ,
+	SMB_ACL_OTHER,
+	SMB_ACL_MASK
+};
+
+struct xattr_acl_entry {
+	int type;
+	uid_t uid;
+	gid_t gid;
+	mode_t perm;
+};
+
+struct xattr_smb_acl {
+	int count;
+	int next;
+	struct xattr_acl_entry entries[0];
+};
+
+struct xattr_ntacl {
+	__u16	version;
+	void	*sd_buf;
+	__u32	sd_size;
+	__u16	hash_type;
+	__u8	desc[10];
+	__u16	desc_len;
+	__u64	current_time;
+	__u8	hash[XATTR_SD_HASH_SIZE];
+	__u8	posix_acl_hash[XATTR_SD_HASH_SIZE];
+};
+
+/* SECURITY DESCRIPTOR XATTR PREFIX */
+#define SD_PREFIX			"NTACL"
+#define SD_PREFIX_LEN	(sizeof(SD_PREFIX) - 1)
+#define XATTR_NAME_SD	\
+		(XATTR_SECURITY_PREFIX SD_PREFIX)
+#define XATTR_NAME_SD_LEN	\
+		(sizeof(XATTR_SECURITY_PREFIX SD_PREFIX) - 1)
+
+
+/* CreateOptions */
+/* Flag is set, it must not be a file , valid for directory only */
+#define FILE_DIRECTORY_FILE_LE			cpu_to_le32(0x00000001)
+#define FILE_WRITE_THROUGH_LE			cpu_to_le32(0x00000002)
+#define FILE_SEQUENTIAL_ONLY_LE			cpu_to_le32(0x00000004)
+
+/* Should not buffer on server*/
+#define FILE_NO_INTERMEDIATE_BUFFERING_LE	cpu_to_le32(0x00000008)
+/* MBZ */
+#define FILE_SYNCHRONOUS_IO_ALERT_LE		cpu_to_le32(0x00000010)
+/* MBZ */
+#define FILE_SYNCHRONOUS_IO_NONALERT_LE		cpu_to_le32(0x00000020)
+
+/* Flaf must not be set for directory */
+#define FILE_NON_DIRECTORY_FILE_LE		cpu_to_le32(0x00000040)
+
+/* Should be zero */
+#define CREATE_TREE_CONNECTION			cpu_to_le32(0x00000080)
+#define FILE_COMPLETE_IF_OPLOCKED_LE		cpu_to_le32(0x00000100)
+#define FILE_NO_EA_KNOWLEDGE_LE			cpu_to_le32(0x00000200)
+#define FILE_OPEN_REMOTE_INSTANCE		cpu_to_le32(0x00000400)
+
+/**
+ * Doc says this is obsolete "open for recovery" flag should be zero
+ * in any case.
+ */
+#define CREATE_OPEN_FOR_RECOVERY		cpu_to_le32(0x00000400)
+#define FILE_RANDOM_ACCESS_LE			cpu_to_le32(0x00000800)
+#define FILE_DELETE_ON_CLOSE_LE			cpu_to_le32(0x00001000)
+#define FILE_OPEN_BY_FILE_ID_LE			cpu_to_le32(0x00002000)
+#define FILE_OPEN_FOR_BACKUP_INTENT_LE		cpu_to_le32(0x00004000)
+#define FILE_NO_COMPRESSION_LE			cpu_to_le32(0x00008000)
+
+/* Should be zero*/
+#define FILE_OPEN_REQUIRING_OPLOCK		cpu_to_le32(0x00010000)
+#define FILE_DISALLOW_EXCLUSIVE			cpu_to_le32(0x00020000)
+#define FILE_RESERVE_OPFILTER_LE		cpu_to_le32(0x00100000)
+#define FILE_OPEN_REPARSE_POINT_LE		cpu_to_le32(0x00200000)
+#define FILE_OPEN_NO_RECALL_LE			cpu_to_le32(0x00400000)
+
+/* Should be zero */
+#define FILE_OPEN_FOR_FREE_SPACE_QUERY_LE	cpu_to_le32(0x00800000)
+#define CREATE_OPTIONS_MASK			cpu_to_le32(0x00FFFFFF)
+#define CREATE_OPTION_READONLY			0x10000000
+/* system. NB not sent over wire */
+#define CREATE_OPTION_SPECIAL			0x20000000
+
+struct ksmbd_work;
+struct ksmbd_file;
+struct ksmbd_conn;
+
+struct ksmbd_dir_info {
+	const char	*name;
+	char		*wptr;
+	char		*rptr;
+	int		name_len;
+	int		out_buf_len;
+	int		num_entry;
+	int		data_count;
+	int		last_entry_offset;
+	bool		hide_dot_file;
+	int		flags;
+};
+
+struct ksmbd_readdir_data {
+	struct dir_context	ctx;
+	union {
+		void		*private;
+		char		*dirent;
+	};
+
+	unsigned int		used;
+	unsigned int		dirent_count;
+	unsigned int		file_attr;
+};
+
+/* ksmbd kstat wrapper to get valid create time when reading dir entry */
+struct ksmbd_kstat {
+	struct kstat		*kstat;
+	unsigned long long	create_time;
+	__le32			file_attributes;
+};
+
+struct ksmbd_fs_sector_size {
+	unsigned short	logical_sector_size;
+	unsigned int	physical_sector_size;
+	unsigned int	optimal_io_size;
+};
+
+int ksmbd_vfs_inode_permission(struct dentry *dentry, int acc_mode,
+		bool delete);
+int ksmbd_vfs_query_maximal_access(struct dentry *dentry, __le32 *daccess);
+int ksmbd_vfs_create(struct ksmbd_work *work, const char *name, umode_t mode);
+int ksmbd_vfs_mkdir(struct ksmbd_work *work, const char *name, umode_t mode);
+int ksmbd_vfs_read(struct ksmbd_work *work, struct ksmbd_file *fp,
+		 size_t count, loff_t *pos);
+int ksmbd_vfs_write(struct ksmbd_work *work, struct ksmbd_file *fp,
+	char *buf, size_t count, loff_t *pos, bool sync, ssize_t *written);
+int ksmbd_vfs_fsync(struct ksmbd_work *work, uint64_t fid, uint64_t p_id);
+int ksmbd_vfs_remove_file(struct ksmbd_work *work, char *name);
+int ksmbd_vfs_link(struct ksmbd_work *work,
+		const char *oldname, const char *newname);
+int ksmbd_vfs_getattr(struct path *path, struct kstat *stat);
+int ksmbd_vfs_symlink(const char *name, const char *symname);
+int ksmbd_vfs_readlink(struct path *path, char *buf, int lenp);
+
+int ksmbd_vfs_fp_rename(struct ksmbd_work *work, struct ksmbd_file *fp,
+		char *newname);
+int ksmbd_vfs_rename_slowpath(struct ksmbd_work *work,
+		char *oldname, char *newname);
+
+int ksmbd_vfs_truncate(struct ksmbd_work *work, const char *name,
+	struct ksmbd_file *fp, loff_t size);
+
+struct srv_copychunk;
+int ksmbd_vfs_copy_file_ranges(struct ksmbd_work *work,
+				struct ksmbd_file *src_fp,
+				struct ksmbd_file *dst_fp,
+				struct srv_copychunk *chunks,
+				unsigned int chunk_count,
+				unsigned int *chunk_count_written,
+				unsigned int *chunk_size_written,
+				loff_t  *total_size_written);
+
+struct ksmbd_file *ksmbd_vfs_dentry_open(struct ksmbd_work *work,
+					 const struct path *path,
+					 int flags,
+					 __le32 option,
+					 int fexist);
+
+ssize_t ksmbd_vfs_listxattr(struct dentry *dentry, char **list);
+ssize_t ksmbd_vfs_getxattr(struct dentry *dentry,
+			   char *xattr_name,
+			   char **xattr_buf);
+
+ssize_t ksmbd_vfs_casexattr_len(struct dentry *dentry,
+				char *attr_name,
+				int attr_name_len);
+
+int ksmbd_vfs_setxattr(struct dentry *dentry,
+		       const char *attr_name,
+		       const void *attr_value,
+		       size_t attr_size,
+		       int flags);
+
+int ksmbd_vfs_fsetxattr(const char *filename,
+			const char *attr_name,
+			const void *attr_value,
+			size_t attr_size,
+			int flags);
+
+int ksmbd_vfs_xattr_stream_name(char *stream_name,
+				char **xattr_stream_name,
+				size_t *xattr_stream_name_size,
+				int s_type);
+
+int ksmbd_vfs_truncate_xattr(struct dentry *dentry, int wo_streams);
+int ksmbd_vfs_remove_xattr(struct dentry *dentry, char *attr_name);
+void ksmbd_vfs_xattr_free(char *xattr);
+
+int ksmbd_vfs_kern_path(char *name, unsigned int flags, struct path *path,
+		bool caseless);
+int ksmbd_vfs_empty_dir(struct ksmbd_file *fp);
+void ksmbd_vfs_set_fadvise(struct file *filp, __le32 option);
+int ksmbd_vfs_lock(struct file *filp, int cmd, struct file_lock *flock);
+int ksmbd_vfs_readdir(struct file *file, struct ksmbd_readdir_data *rdata);
+int ksmbd_vfs_alloc_size(struct ksmbd_work *work,
+			 struct ksmbd_file *fp,
+			 loff_t len);
+int ksmbd_vfs_zero_data(struct ksmbd_work *work,
+			 struct ksmbd_file *fp,
+			 loff_t off,
+			 loff_t len);
+
+struct file_allocated_range_buffer;
+int ksmbd_vfs_fqar_lseek(struct ksmbd_file *fp, loff_t start, loff_t length,
+			struct file_allocated_range_buffer *ranges,
+			int in_count, int *out_count);
+int ksmbd_vfs_unlink(struct dentry *dir, struct dentry *dentry);
+unsigned short ksmbd_vfs_logical_sector_size(struct inode *inode);
+void ksmbd_vfs_smb2_sector_size(struct inode *inode,
+				struct ksmbd_fs_sector_size *fs_ss);
+void *ksmbd_vfs_init_kstat(char **p, struct ksmbd_kstat *ksmbd_kstat);
+
+int ksmbd_vfs_fill_dentry_attrs(struct ksmbd_work *work,
+				struct dentry *dentry,
+				struct ksmbd_kstat *ksmbd_kstat);
+
+int ksmbd_vfs_posix_lock_wait(struct file_lock *flock);
+int ksmbd_vfs_posix_lock_wait_timeout(struct file_lock *flock, long timeout);
+void ksmbd_vfs_posix_lock_unblock(struct file_lock *flock);
+
+int ksmbd_vfs_remove_acl_xattrs(struct dentry *dentry);
+int ksmbd_vfs_remove_sd_xattrs(struct dentry *dentry);
+int ksmbd_vfs_set_sd_xattr(struct ksmbd_conn *conn, struct dentry *dentry,
+		struct smb_ntsd *pntsd, int len);
+int ksmbd_vfs_get_sd_xattr(struct ksmbd_conn *conn, struct dentry *dentry,
+		struct smb_ntsd **pntsd);
+int ksmbd_vfs_set_dos_attrib_xattr(struct dentry *dentry,
+		struct xattr_dos_attrib *da);
+int ksmbd_vfs_get_dos_attrib_xattr(struct dentry *dentry,
+		struct xattr_dos_attrib *da);
+struct posix_acl *ksmbd_vfs_posix_acl_alloc(int count, gfp_t flags);
+struct posix_acl *ksmbd_vfs_get_acl(struct inode *inode, int type);
+int ksmbd_vfs_set_posix_acl(struct inode *inode, int type,
+		struct posix_acl *acl);
+int ksmbd_vfs_set_init_posix_acl(struct inode *inode);
+int ksmbd_vfs_inherit_posix_acl(struct inode *inode,
+		struct inode *parent_inode);
+#endif /* __KSMBD_VFS_H__ */
diff --git a/fs/cifsd/vfs_cache.c b/fs/cifsd/vfs_cache.c
new file mode 100644
index 000000000000..af92fab5b7ae
--- /dev/null
+++ b/fs/cifsd/vfs_cache.c
@@ -0,0 +1,851 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Copyright (C) 2016 Namjae Jeon <linkinjeon@kernel.org>
+ * Copyright (C) 2019 Samsung Electronics Co., Ltd.
+ */
+
+#include <linux/fs.h>
+#include <linux/slab.h>
+#include <linux/vmalloc.h>
+
+#include "glob.h"
+#include "vfs_cache.h"
+#include "buffer_pool.h"
+#include "oplock.h"
+#include "vfs.h"
+#include "connection.h"
+#include "mgmt/tree_connect.h"
+#include "mgmt/user_session.h"
+#include "smb_common.h"
+
+#define S_DEL_PENDING			1
+#define S_DEL_ON_CLS			2
+#define S_DEL_ON_CLS_STREAM		8
+
+static unsigned int inode_hash_mask __read_mostly;
+static unsigned int inode_hash_shift __read_mostly;
+static struct hlist_head *inode_hashtable __read_mostly;
+static DEFINE_RWLOCK(inode_hash_lock);
+
+static struct ksmbd_file_table global_ft;
+static atomic_long_t fd_limit;
+
+void ksmbd_set_fd_limit(unsigned long limit)
+{
+	limit = min(limit, get_max_files());
+	atomic_long_set(&fd_limit, limit);
+}
+
+static bool fd_limit_depleted(void)
+{
+	long v = atomic_long_dec_return(&fd_limit);
+
+	if (v >= 0)
+		return false;
+	atomic_long_inc(&fd_limit);
+	return true;
+}
+
+static void fd_limit_close(void)
+{
+	atomic_long_inc(&fd_limit);
+}
+
+/*
+ * INODE hash
+ */
+
+static unsigned long inode_hash(struct super_block *sb, unsigned long hashval)
+{
+	unsigned long tmp;
+
+	tmp = (hashval * (unsigned long)sb) ^ (GOLDEN_RATIO_PRIME + hashval) /
+		L1_CACHE_BYTES;
+	tmp = tmp ^ ((tmp ^ GOLDEN_RATIO_PRIME) >> inode_hash_shift);
+	return tmp & inode_hash_mask;
+}
+
+static struct ksmbd_inode *__ksmbd_inode_lookup(struct inode *inode)
+{
+	struct hlist_head *head = inode_hashtable +
+		inode_hash(inode->i_sb, inode->i_ino);
+	struct ksmbd_inode *ci = NULL, *ret_ci = NULL;
+
+	hlist_for_each_entry(ci, head, m_hash) {
+		if (ci->m_inode == inode) {
+			if (atomic_inc_not_zero(&ci->m_count))
+				ret_ci = ci;
+			break;
+		}
+	}
+	return ret_ci;
+}
+
+static struct ksmbd_inode *ksmbd_inode_lookup(struct ksmbd_file *fp)
+{
+	return __ksmbd_inode_lookup(FP_INODE(fp));
+}
+
+static struct ksmbd_inode *ksmbd_inode_lookup_by_vfsinode(struct inode *inode)
+{
+	struct ksmbd_inode *ci;
+
+	read_lock(&inode_hash_lock);
+	ci = __ksmbd_inode_lookup(inode);
+	read_unlock(&inode_hash_lock);
+	return ci;
+}
+
+int ksmbd_query_inode_status(struct inode *inode)
+{
+	struct ksmbd_inode *ci;
+	int ret = KSMBD_INODE_STATUS_UNKNOWN;
+
+	read_lock(&inode_hash_lock);
+	ci = __ksmbd_inode_lookup(inode);
+	if (ci) {
+		ret = KSMBD_INODE_STATUS_OK;
+		if (ci->m_flags & S_DEL_PENDING)
+			ret = KSMBD_INODE_STATUS_PENDING_DELETE;
+		atomic_dec(&ci->m_count);
+	}
+	read_unlock(&inode_hash_lock);
+	return ret;
+}
+
+bool ksmbd_inode_pending_delete(struct ksmbd_file *fp)
+{
+	return (fp->f_ci->m_flags & S_DEL_PENDING);
+}
+
+void ksmbd_set_inode_pending_delete(struct ksmbd_file *fp)
+{
+	fp->f_ci->m_flags |= S_DEL_PENDING;
+}
+
+void ksmbd_clear_inode_pending_delete(struct ksmbd_file *fp)
+{
+	fp->f_ci->m_flags &= ~S_DEL_PENDING;
+}
+
+void ksmbd_fd_set_delete_on_close(struct ksmbd_file *fp,
+				  int file_info)
+{
+	if (ksmbd_stream_fd(fp)) {
+		fp->f_ci->m_flags |= S_DEL_ON_CLS_STREAM;
+		return;
+	}
+
+	fp->f_ci->m_flags |= S_DEL_ON_CLS;
+}
+
+static void ksmbd_inode_hash(struct ksmbd_inode *ci)
+{
+	struct hlist_head *b = inode_hashtable +
+		inode_hash(ci->m_inode->i_sb, ci->m_inode->i_ino);
+
+	hlist_add_head(&ci->m_hash, b);
+}
+
+static void ksmbd_inode_unhash(struct ksmbd_inode *ci)
+{
+	write_lock(&inode_hash_lock);
+	hlist_del_init(&ci->m_hash);
+	write_unlock(&inode_hash_lock);
+}
+
+static int ksmbd_inode_init(struct ksmbd_inode *ci, struct ksmbd_file *fp)
+{
+	ci->m_inode = FP_INODE(fp);
+	atomic_set(&ci->m_count, 1);
+	atomic_set(&ci->op_count, 0);
+	atomic_set(&ci->sop_count, 0);
+	ci->m_flags = 0;
+	ci->m_fattr = 0;
+	INIT_LIST_HEAD(&ci->m_fp_list);
+	INIT_LIST_HEAD(&ci->m_op_list);
+	rwlock_init(&ci->m_lock);
+	return 0;
+}
+
+static struct ksmbd_inode *ksmbd_inode_get(struct ksmbd_file *fp)
+{
+	struct ksmbd_inode *ci, *tmpci;
+	int rc;
+
+	read_lock(&inode_hash_lock);
+	ci = ksmbd_inode_lookup(fp);
+	read_unlock(&inode_hash_lock);
+	if (ci)
+		return ci;
+
+	ci = kmalloc(sizeof(struct ksmbd_inode), GFP_KERNEL);
+	if (!ci)
+		return NULL;
+
+	rc = ksmbd_inode_init(ci, fp);
+	if (rc) {
+		ksmbd_err("inode initialized failed\n");
+		kfree(ci);
+		return NULL;
+	}
+
+	write_lock(&inode_hash_lock);
+	tmpci = ksmbd_inode_lookup(fp);
+	if (!tmpci) {
+		ksmbd_inode_hash(ci);
+	} else {
+		kfree(ci);
+		ci = tmpci;
+	}
+	write_unlock(&inode_hash_lock);
+	return ci;
+}
+
+static void ksmbd_inode_free(struct ksmbd_inode *ci)
+{
+	ksmbd_inode_unhash(ci);
+	kfree(ci);
+}
+
+static void ksmbd_inode_put(struct ksmbd_inode *ci)
+{
+	if (atomic_dec_and_test(&ci->m_count))
+		ksmbd_inode_free(ci);
+}
+
+int __init ksmbd_inode_hash_init(void)
+{
+	unsigned int loop;
+	unsigned long numentries = 16384;
+	unsigned long bucketsize = sizeof(struct hlist_head);
+	unsigned long size;
+
+	inode_hash_shift = ilog2(numentries);
+	inode_hash_mask = (1 << inode_hash_shift) - 1;
+
+	size = bucketsize << inode_hash_shift;
+
+	/* init master fp hash table */
+	inode_hashtable = vmalloc(size);
+	if (!inode_hashtable)
+		return -ENOMEM;
+
+	for (loop = 0; loop < (1U << inode_hash_shift); loop++)
+		INIT_HLIST_HEAD(&inode_hashtable[loop]);
+	return 0;
+}
+
+void __exit ksmbd_release_inode_hash(void)
+{
+	vfree(inode_hashtable);
+}
+
+static void __ksmbd_inode_close(struct ksmbd_file *fp)
+{
+	struct dentry *dir, *dentry;
+	struct ksmbd_inode *ci = fp->f_ci;
+	int err;
+	struct file *filp;
+
+	filp = fp->filp;
+	if (ksmbd_stream_fd(fp) && (ci->m_flags & S_DEL_ON_CLS_STREAM)) {
+		ci->m_flags &= ~S_DEL_ON_CLS_STREAM;
+		err = ksmbd_vfs_remove_xattr(filp->f_path.dentry,
+					     fp->stream.name);
+		if (err)
+			ksmbd_err("remove xattr failed : %s\n",
+				fp->stream.name);
+	}
+
+	if (atomic_dec_and_test(&ci->m_count)) {
+		write_lock(&ci->m_lock);
+		if (ci->m_flags & (S_DEL_ON_CLS | S_DEL_PENDING)) {
+			dentry = filp->f_path.dentry;
+			dir = dentry->d_parent;
+			ci->m_flags &= ~(S_DEL_ON_CLS | S_DEL_PENDING);
+			write_unlock(&ci->m_lock);
+			ksmbd_vfs_unlink(dir, dentry);
+			write_lock(&ci->m_lock);
+		}
+		write_unlock(&ci->m_lock);
+
+		ksmbd_inode_free(ci);
+	}
+}
+
+static void __ksmbd_remove_durable_fd(struct ksmbd_file *fp)
+{
+	if (!HAS_FILE_ID(fp->persistent_id))
+		return;
+
+	write_lock(&global_ft.lock);
+	idr_remove(global_ft.idr, fp->persistent_id);
+	write_unlock(&global_ft.lock);
+}
+
+static void __ksmbd_remove_fd(struct ksmbd_file_table *ft,
+			      struct ksmbd_file *fp)
+{
+	if (!HAS_FILE_ID(fp->volatile_id))
+		return;
+
+	write_lock(&fp->f_ci->m_lock);
+	list_del_init(&fp->node);
+	write_unlock(&fp->f_ci->m_lock);
+
+	write_lock(&ft->lock);
+	idr_remove(ft->idr, fp->volatile_id);
+	write_unlock(&ft->lock);
+}
+
+static void __ksmbd_close_fd(struct ksmbd_file_table *ft,
+			     struct ksmbd_file *fp)
+{
+	struct file *filp;
+
+	fd_limit_close();
+	__ksmbd_remove_durable_fd(fp);
+	__ksmbd_remove_fd(ft, fp);
+
+	close_id_del_oplock(fp);
+	filp = fp->filp;
+
+	__ksmbd_inode_close(fp);
+	if (!IS_ERR_OR_NULL(filp))
+		fput(filp);
+	kfree(fp->filename);
+	if (ksmbd_stream_fd(fp))
+		kfree(fp->stream.name);
+	ksmbd_free_file_struct(fp);
+}
+
+static struct ksmbd_file *ksmbd_fp_get(struct ksmbd_file *fp)
+{
+	if (!atomic_inc_not_zero(&fp->refcount))
+		return NULL;
+	return fp;
+}
+
+static struct ksmbd_file *__ksmbd_lookup_fd(struct ksmbd_file_table *ft,
+					    unsigned int id)
+{
+	bool unclaimed = true;
+	struct ksmbd_file *fp;
+
+	read_lock(&ft->lock);
+	fp = idr_find(ft->idr, id);
+	if (fp)
+		fp = ksmbd_fp_get(fp);
+
+	if (fp && fp->f_ci) {
+		read_lock(&fp->f_ci->m_lock);
+		unclaimed = list_empty(&fp->node);
+		read_unlock(&fp->f_ci->m_lock);
+	}
+	read_unlock(&ft->lock);
+
+	if (fp && unclaimed) {
+		atomic_dec(&fp->refcount);
+		return NULL;
+	}
+	return fp;
+}
+
+static void __put_fd_final(struct ksmbd_work *work,
+			   struct ksmbd_file *fp)
+{
+	__ksmbd_close_fd(&work->sess->file_table, fp);
+	atomic_dec(&work->conn->stats.open_files_count);
+}
+
+static void set_close_state_blocked_works(struct ksmbd_file *fp)
+{
+	struct ksmbd_work *cancel_work, *ctmp;
+
+	spin_lock(&fp->f_lock);
+	list_for_each_entry_safe(cancel_work, ctmp, &fp->blocked_works,
+			fp_entry) {
+		list_del(&cancel_work->fp_entry);
+		cancel_work->state = KSMBD_WORK_CLOSED;
+		cancel_work->cancel_fn(cancel_work->cancel_argv);
+	}
+	spin_unlock(&fp->f_lock);
+}
+
+int ksmbd_close_fd(struct ksmbd_work *work, unsigned int id)
+{
+	struct ksmbd_file	*fp;
+	struct ksmbd_file_table	*ft;
+
+	if (!HAS_FILE_ID(id))
+		return 0;
+
+	ft = &work->sess->file_table;
+	read_lock(&ft->lock);
+	fp = idr_find(ft->idr, id);
+	if (fp) {
+		set_close_state_blocked_works(fp);
+
+		if (!atomic_dec_and_test(&fp->refcount))
+			fp = NULL;
+	}
+	read_unlock(&ft->lock);
+
+	if (!fp)
+		return -EINVAL;
+
+	__put_fd_final(work, fp);
+	return 0;
+}
+
+void ksmbd_fd_put(struct ksmbd_work *work,
+		  struct ksmbd_file *fp)
+{
+	if (!fp)
+		return;
+
+	if (!atomic_dec_and_test(&fp->refcount))
+		return;
+	__put_fd_final(work, fp);
+}
+
+static bool __sanity_check(struct ksmbd_tree_connect *tcon,
+			   struct ksmbd_file *fp)
+{
+	if (!fp)
+		return false;
+	if (fp->tcon != tcon)
+		return false;
+	return true;
+}
+
+struct ksmbd_file *ksmbd_lookup_foreign_fd(struct ksmbd_work *work,
+					   unsigned int id)
+{
+	return __ksmbd_lookup_fd(&work->sess->file_table, id);
+}
+
+struct ksmbd_file *ksmbd_lookup_fd_fast(struct ksmbd_work *work,
+					unsigned int id)
+{
+	struct ksmbd_file *fp = __ksmbd_lookup_fd(&work->sess->file_table, id);
+
+	if (__sanity_check(work->tcon, fp))
+		return fp;
+
+	ksmbd_fd_put(work, fp);
+	return NULL;
+}
+
+struct ksmbd_file *ksmbd_lookup_fd_slow(struct ksmbd_work *work,
+					unsigned int id,
+					unsigned int pid)
+{
+	struct ksmbd_file *fp;
+
+	if (!HAS_FILE_ID(id)) {
+		id = work->compound_fid;
+		pid = work->compound_pfid;
+	}
+
+	if (!HAS_FILE_ID(id))
+		return NULL;
+
+	fp = __ksmbd_lookup_fd(&work->sess->file_table, id);
+	if (!__sanity_check(work->tcon, fp)) {
+		ksmbd_fd_put(work, fp);
+		return NULL;
+	}
+	if (fp->persistent_id != pid) {
+		ksmbd_fd_put(work, fp);
+		return NULL;
+	}
+	return fp;
+}
+
+struct ksmbd_file *ksmbd_lookup_durable_fd(unsigned long long id)
+{
+	return __ksmbd_lookup_fd(&global_ft, id);
+}
+
+int ksmbd_close_fd_app_id(struct ksmbd_work *work,
+			  char *app_id)
+{
+	struct ksmbd_file	*fp = NULL;
+	unsigned int		id;
+
+	read_lock(&global_ft.lock);
+	idr_for_each_entry(global_ft.idr, fp, id) {
+		if (!memcmp(fp->app_instance_id,
+			    app_id,
+			    SMB2_CREATE_GUID_SIZE)) {
+			if (!atomic_dec_and_test(&fp->refcount))
+				fp = NULL;
+			break;
+		}
+	}
+	read_unlock(&global_ft.lock);
+
+	if (!fp)
+		return -EINVAL;
+
+	__put_fd_final(work, fp);
+	return 0;
+}
+
+struct ksmbd_file *ksmbd_lookup_fd_cguid(char *cguid)
+{
+	struct ksmbd_file	*fp = NULL;
+	unsigned int		id;
+
+	read_lock(&global_ft.lock);
+	idr_for_each_entry(global_ft.idr, fp, id) {
+		if (!memcmp(fp->create_guid,
+			    cguid,
+			    SMB2_CREATE_GUID_SIZE)) {
+			fp = ksmbd_fp_get(fp);
+			break;
+		}
+	}
+	read_unlock(&global_ft.lock);
+
+	return fp;
+}
+
+struct ksmbd_file *ksmbd_lookup_fd_filename(struct ksmbd_work *work,
+					    char *filename)
+{
+	struct ksmbd_file	*fp = NULL;
+	unsigned int		id;
+
+	read_lock(&work->sess->file_table.lock);
+	idr_for_each_entry(work->sess->file_table.idr, fp, id) {
+		if (!strcmp(fp->filename, filename)) {
+			fp = ksmbd_fp_get(fp);
+			break;
+		}
+	}
+	read_unlock(&work->sess->file_table.lock);
+
+	return fp;
+}
+
+struct ksmbd_file *ksmbd_lookup_fd_inode(struct inode *inode)
+{
+	struct ksmbd_file	*lfp;
+	struct ksmbd_inode	*ci;
+	struct list_head	*cur;
+
+	ci = ksmbd_inode_lookup_by_vfsinode(inode);
+	if (!ci)
+		return NULL;
+
+	read_lock(&ci->m_lock);
+	list_for_each(cur, &ci->m_fp_list) {
+		lfp = list_entry(cur, struct ksmbd_file, node);
+		if (inode == FP_INODE(lfp)) {
+			atomic_dec(&ci->m_count);
+			read_unlock(&ci->m_lock);
+			return lfp;
+		}
+	}
+	atomic_dec(&ci->m_count);
+	read_unlock(&ci->m_lock);
+	return NULL;
+}
+
+#define OPEN_ID_TYPE_VOLATILE_ID	(0)
+#define OPEN_ID_TYPE_PERSISTENT_ID	(1)
+
+static void __open_id_set(struct ksmbd_file *fp, unsigned int id, int type)
+{
+	if (type == OPEN_ID_TYPE_VOLATILE_ID)
+		fp->volatile_id = id;
+	if (type == OPEN_ID_TYPE_PERSISTENT_ID)
+		fp->persistent_id = id;
+}
+
+static int __open_id(struct ksmbd_file_table *ft,
+		     struct ksmbd_file *fp,
+		     int type)
+{
+	unsigned int		id = 0;
+	int			ret;
+
+	if (type == OPEN_ID_TYPE_VOLATILE_ID && fd_limit_depleted()) {
+		__open_id_set(fp, KSMBD_NO_FID, type);
+		return -EMFILE;
+	}
+
+	idr_preload(GFP_KERNEL);
+	write_lock(&ft->lock);
+	ret = idr_alloc_cyclic(ft->idr, fp, 0, INT_MAX, GFP_NOWAIT);
+	if (ret >= 0) {
+		id = ret;
+		ret = 0;
+	} else {
+		id = KSMBD_NO_FID;
+		fd_limit_close();
+	}
+
+	__open_id_set(fp, id, type);
+	write_unlock(&ft->lock);
+	idr_preload_end();
+	return ret;
+}
+
+unsigned int ksmbd_open_durable_fd(struct ksmbd_file *fp)
+{
+	__open_id(&global_ft, fp, OPEN_ID_TYPE_PERSISTENT_ID);
+	return fp->persistent_id;
+}
+
+struct ksmbd_file *ksmbd_open_fd(struct ksmbd_work *work,
+				 struct file *filp)
+{
+	struct ksmbd_file	*fp;
+	int ret;
+
+	fp = ksmbd_alloc_file_struct();
+	if (!fp) {
+		ksmbd_err("Failed to allocate memory\n");
+		return ERR_PTR(-ENOMEM);
+	}
+
+	INIT_LIST_HEAD(&fp->blocked_works);
+	INIT_LIST_HEAD(&fp->node);
+	spin_lock_init(&fp->f_lock);
+	atomic_set(&fp->refcount, 1);
+
+	fp->filp		= filp;
+	fp->conn		= work->sess->conn;
+	fp->tcon		= work->tcon;
+	fp->volatile_id		= KSMBD_NO_FID;
+	fp->persistent_id	= KSMBD_NO_FID;
+	fp->f_ci		= ksmbd_inode_get(fp);
+
+	if (!fp->f_ci) {
+		ksmbd_free_file_struct(fp);
+		return ERR_PTR(-ENOMEM);
+	}
+
+	ret = __open_id(&work->sess->file_table, fp, OPEN_ID_TYPE_VOLATILE_ID);
+	if (ret) {
+		ksmbd_inode_put(fp->f_ci);
+		ksmbd_free_file_struct(fp);
+		return ERR_PTR(ret);
+	}
+
+	atomic_inc(&work->conn->stats.open_files_count);
+	return fp;
+}
+
+static inline bool is_reconnectable(struct ksmbd_file *fp)
+{
+	struct oplock_info *opinfo = opinfo_get(fp);
+	bool reconn = false;
+
+	if (!opinfo)
+		return false;
+
+	if (opinfo->op_state != OPLOCK_STATE_NONE) {
+		opinfo_put(opinfo);
+		return false;
+	}
+
+	if (fp->is_resilient || fp->is_persistent)
+		reconn = true;
+	else if (fp->is_durable && opinfo->is_lease &&
+			opinfo->o_lease->state & SMB2_LEASE_HANDLE_CACHING_LE)
+		reconn = true;
+
+	else if (fp->is_durable && opinfo->level == SMB2_OPLOCK_LEVEL_BATCH)
+		reconn = true;
+
+	opinfo_put(opinfo);
+	return reconn;
+}
+
+static int
+__close_file_table_ids(struct ksmbd_file_table *ft,
+		       struct ksmbd_tree_connect *tcon,
+		       bool (*skip)(struct ksmbd_tree_connect *tcon,
+				    struct ksmbd_file *fp))
+{
+	unsigned int			id;
+	struct ksmbd_file		*fp;
+	int				num = 0;
+
+	idr_for_each_entry(ft->idr, fp, id) {
+		if (skip(tcon, fp))
+			continue;
+
+		set_close_state_blocked_works(fp);
+
+		if (!atomic_dec_and_test(&fp->refcount))
+			continue;
+		__ksmbd_close_fd(ft, fp);
+		num++;
+	}
+	return num;
+}
+
+static bool tree_conn_fd_check(struct ksmbd_tree_connect *tcon,
+			       struct ksmbd_file *fp)
+{
+	return fp->tcon != tcon;
+}
+
+static bool session_fd_check(struct ksmbd_tree_connect *tcon,
+			     struct ksmbd_file *fp)
+{
+	if (!is_reconnectable(fp))
+		return false;
+
+	fp->conn = NULL;
+	fp->tcon = NULL;
+	fp->volatile_id = KSMBD_NO_FID;
+	return true;
+}
+
+void ksmbd_close_tree_conn_fds(struct ksmbd_work *work)
+{
+	int num = __close_file_table_ids(&work->sess->file_table,
+					 work->tcon,
+					 tree_conn_fd_check);
+
+	atomic_sub(num, &work->conn->stats.open_files_count);
+}
+
+void ksmbd_close_session_fds(struct ksmbd_work *work)
+{
+	int num = __close_file_table_ids(&work->sess->file_table,
+					 work->tcon,
+					 session_fd_check);
+
+	atomic_sub(num, &work->conn->stats.open_files_count);
+}
+
+int ksmbd_init_global_file_table(void)
+{
+	return ksmbd_init_file_table(&global_ft);
+}
+
+void ksmbd_free_global_file_table(void)
+{
+	struct ksmbd_file	*fp = NULL;
+	unsigned int		id;
+
+	idr_for_each_entry(global_ft.idr, fp, id) {
+		__ksmbd_remove_durable_fd(fp);
+		ksmbd_free_file_struct(fp);
+	}
+
+	ksmbd_destroy_file_table(&global_ft);
+}
+
+int ksmbd_reopen_durable_fd(struct ksmbd_work *work,
+			    struct ksmbd_file *fp)
+{
+	if (!fp->is_durable || fp->conn || fp->tcon) {
+		ksmbd_err("Invalid durable fd [%p:%p]\n",
+				fp->conn, fp->tcon);
+		return -EBADF;
+	}
+
+	if (HAS_FILE_ID(fp->volatile_id)) {
+		ksmbd_err("Still in use durable fd: %u\n", fp->volatile_id);
+		return -EBADF;
+	}
+
+	fp->conn = work->sess->conn;
+	fp->tcon = work->tcon;
+
+	__open_id(&work->sess->file_table, fp, OPEN_ID_TYPE_VOLATILE_ID);
+	if (!HAS_FILE_ID(fp->volatile_id)) {
+		fp->conn = NULL;
+		fp->tcon = NULL;
+		return -EBADF;
+	}
+	return 0;
+}
+
+static void close_fd_list(struct ksmbd_work *work, struct list_head *head)
+{
+	while (!list_empty(head)) {
+		struct ksmbd_file *fp;
+
+		fp = list_first_entry(head, struct ksmbd_file, node);
+		list_del_init(&fp->node);
+
+		__ksmbd_close_fd(&work->sess->file_table, fp);
+	}
+}
+
+int ksmbd_close_inode_fds(struct ksmbd_work *work, struct inode *inode)
+{
+	struct ksmbd_inode *ci;
+	bool unlinked = true;
+	struct ksmbd_file *fp, *fptmp;
+	LIST_HEAD(dispose);
+
+	ci = ksmbd_inode_lookup_by_vfsinode(inode);
+	if (!ci)
+		return true;
+
+	if (ci->m_flags & (S_DEL_ON_CLS | S_DEL_PENDING))
+		unlinked = false;
+
+	write_lock(&ci->m_lock);
+	list_for_each_entry_safe(fp, fptmp, &ci->m_fp_list, node) {
+		if (fp->conn)
+			continue;
+
+		list_del(&fp->node);
+		list_add(&fp->node, &dispose);
+	}
+	atomic_dec(&ci->m_count);
+	write_unlock(&ci->m_lock);
+
+	close_fd_list(work, &dispose);
+	return unlinked;
+}
+
+int ksmbd_file_table_flush(struct ksmbd_work *work)
+{
+	struct ksmbd_file	*fp = NULL;
+	unsigned int		id;
+	int			ret;
+
+	read_lock(&work->sess->file_table.lock);
+	idr_for_each_entry(work->sess->file_table.idr, fp, id) {
+		ret = ksmbd_vfs_fsync(work, fp->volatile_id, KSMBD_NO_FID);
+		if (ret)
+			break;
+	}
+	read_unlock(&work->sess->file_table.lock);
+	return ret;
+}
+
+int ksmbd_init_file_table(struct ksmbd_file_table *ft)
+{
+	ft->idr = ksmbd_alloc(sizeof(struct idr));
+	if (!ft->idr)
+		return -ENOMEM;
+
+	idr_init(ft->idr);
+	rwlock_init(&ft->lock);
+	return 0;
+}
+
+void ksmbd_destroy_file_table(struct ksmbd_file_table *ft)
+{
+	if (!ft->idr)
+		return;
+
+	__close_file_table_ids(ft, NULL, session_fd_check);
+	idr_destroy(ft->idr);
+	ksmbd_free(ft->idr);
+	ft->idr = NULL;
+}
diff --git a/fs/cifsd/vfs_cache.h b/fs/cifsd/vfs_cache.h
new file mode 100644
index 000000000000..7d23657c86c6
--- /dev/null
+++ b/fs/cifsd/vfs_cache.h
@@ -0,0 +1,213 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+/*
+ *   Copyright (C) 2019 Samsung Electronics Co., Ltd.
+ */
+
+#ifndef __VFS_CACHE_H__
+#define __VFS_CACHE_H__
+
+#include <linux/version.h>
+#include <linux/file.h>
+#include <linux/fs.h>
+#include <linux/rwsem.h>
+#include <linux/spinlock.h>
+#include <linux/idr.h>
+#include <linux/workqueue.h>
+
+#include "vfs.h"
+
+/* Windows style file permissions for extended response */
+#define	FILE_GENERIC_ALL	0x1F01FF
+#define	FILE_GENERIC_READ	0x120089
+#define	FILE_GENERIC_WRITE	0x120116
+#define	FILE_GENERIC_EXECUTE	0X1200a0
+
+#define KSMBD_START_FID		0
+#define KSMBD_NO_FID		(UINT_MAX)
+#define SMB2_NO_FID		(0xFFFFFFFFFFFFFFFFULL)
+
+#define FP_FILENAME(fp)		fp->filp->f_path.dentry->d_name.name
+#define FP_INODE(fp)		fp->filp->f_path.dentry->d_inode
+#define PARENT_INODE(fp)	fp->filp->f_path.dentry->d_parent->d_inode
+
+#define ATTR_FP(fp) (fp->attrib_only && \
+		(fp->cdoption != FILE_OVERWRITE_IF_LE && \
+		fp->cdoption != FILE_OVERWRITE_LE && \
+		fp->cdoption != FILE_SUPERSEDE_LE))
+
+struct ksmbd_conn;
+struct ksmbd_session;
+
+struct ksmbd_lock {
+	struct file_lock *fl;
+	struct list_head glist;
+	struct list_head llist;
+	unsigned int flags;
+	int cmd;
+	int zero_len;
+	unsigned long long start;
+	unsigned long long end;
+};
+
+struct stream {
+	char *name;
+	ssize_t size;
+};
+
+struct ksmbd_inode {
+	rwlock_t			m_lock;
+	atomic_t			m_count;
+	atomic_t			op_count;
+	/* opinfo count for streams */
+	atomic_t			sop_count;
+	struct inode			*m_inode;
+	unsigned int			m_flags;
+	struct hlist_node		m_hash;
+	struct list_head		m_fp_list;
+	struct list_head		m_op_list;
+	struct oplock_info		*m_opinfo;
+	__le32				m_fattr;
+};
+
+struct ksmbd_file {
+	struct file			*filp;
+	char				*filename;
+	unsigned int			persistent_id;
+	unsigned int			volatile_id;
+
+	spinlock_t			f_lock;
+
+	struct ksmbd_inode		*f_ci;
+	struct ksmbd_inode		*f_parent_ci;
+	struct oplock_info __rcu	*f_opinfo;
+	struct ksmbd_conn		*conn;
+	struct ksmbd_tree_connect	*tcon;
+
+	atomic_t			refcount;
+	__le32				daccess;
+	__le32				saccess;
+	__le32				coption;
+	__le32				cdoption;
+	__u64				create_time;
+	__u64				itime;
+
+	bool				is_durable;
+	bool				is_resilient;
+	bool				is_persistent;
+	bool				is_nt_open;
+	bool				attrib_only;
+
+	char				client_guid[16];
+	char				create_guid[16];
+	char				app_instance_id[16];
+
+	struct stream			stream;
+	struct list_head		node;
+	struct list_head		blocked_works;
+
+	int				durable_timeout;
+
+	/* for SMB1 */
+	int				pid;
+
+	/* conflict lock fail count for SMB1 */
+	unsigned int			cflock_cnt;
+	/* last lock failure start offset for SMB1 */
+	unsigned long long		llock_fstart;
+
+	int				dirent_offset;
+
+	/* if ls is happening on directory, below is valid*/
+	struct ksmbd_readdir_data	readdir_data;
+	int				dot_dotdot[2];
+};
+
+static inline void set_ctx_actor(struct dir_context *ctx,
+				 filldir_t actor)
+{
+	ctx->actor = actor;
+}
+
+#define KSMBD_NR_OPEN_DEFAULT BITS_PER_LONG
+
+struct ksmbd_file_table {
+	rwlock_t		lock;
+	struct idr		*idr;
+};
+
+static inline bool HAS_FILE_ID(unsigned long long req)
+{
+	unsigned int id = (unsigned int)req;
+
+	return id < KSMBD_NO_FID;
+}
+
+static inline bool ksmbd_stream_fd(struct ksmbd_file *fp)
+{
+	return fp->stream.name != NULL;
+}
+
+int ksmbd_init_file_table(struct ksmbd_file_table *ft);
+void ksmbd_destroy_file_table(struct ksmbd_file_table *ft);
+
+int ksmbd_close_fd(struct ksmbd_work *work, unsigned int id);
+
+struct ksmbd_file *ksmbd_lookup_fd_fast(struct ksmbd_work *work,
+					unsigned int id);
+struct ksmbd_file *ksmbd_lookup_foreign_fd(struct ksmbd_work *work,
+					   unsigned int id);
+struct ksmbd_file *ksmbd_lookup_fd_slow(struct ksmbd_work *work,
+					unsigned int id,
+					unsigned int pid);
+
+void ksmbd_fd_put(struct ksmbd_work *work, struct ksmbd_file *fp);
+
+int ksmbd_close_fd_app_id(struct ksmbd_work *work, char *app_id);
+struct ksmbd_file *ksmbd_lookup_durable_fd(unsigned long long id);
+struct ksmbd_file *ksmbd_lookup_fd_cguid(char *cguid);
+struct ksmbd_file *ksmbd_lookup_fd_filename(struct ksmbd_work *work,
+					    char *filename);
+struct ksmbd_file *ksmbd_lookup_fd_inode(struct inode *inode);
+
+unsigned int ksmbd_open_durable_fd(struct ksmbd_file *fp);
+
+struct ksmbd_file *ksmbd_open_fd(struct ksmbd_work *work,
+				 struct file *filp);
+
+void ksmbd_close_tree_conn_fds(struct ksmbd_work *work);
+void ksmbd_close_session_fds(struct ksmbd_work *work);
+
+int ksmbd_close_inode_fds(struct ksmbd_work *work, struct inode *inode);
+
+int ksmbd_reopen_durable_fd(struct ksmbd_work *work,
+			    struct ksmbd_file *fp);
+
+int ksmbd_init_global_file_table(void);
+void ksmbd_free_global_file_table(void);
+
+int ksmbd_file_table_flush(struct ksmbd_work *work);
+
+void ksmbd_set_fd_limit(unsigned long limit);
+
+/*
+ * INODE hash
+ */
+
+int __init ksmbd_inode_hash_init(void);
+void __exit ksmbd_release_inode_hash(void);
+
+enum KSMBD_INODE_STATUS {
+	KSMBD_INODE_STATUS_OK,
+	KSMBD_INODE_STATUS_UNKNOWN,
+	KSMBD_INODE_STATUS_PENDING_DELETE,
+};
+
+int ksmbd_query_inode_status(struct inode *inode);
+
+bool ksmbd_inode_pending_delete(struct ksmbd_file *fp);
+void ksmbd_set_inode_pending_delete(struct ksmbd_file *fp);
+void ksmbd_clear_inode_pending_delete(struct ksmbd_file *fp);
+
+void ksmbd_fd_set_delete_on_close(struct ksmbd_file *fp,
+				  int file_info);
+#endif /* __VFS_CACHE_H__ */
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 4/5] cifsd: add Kconfig and Makefile
       [not found]   ` <CGME20210322052208epcas1p430b2e93761d5194844c533c61d43242d@epcas1p4.samsung.com>
@ 2021-03-22  5:13     ` Namjae Jeon
  0 siblings, 0 replies; 31+ messages in thread
From: Namjae Jeon @ 2021-03-22  5:13 UTC (permalink / raw)
  To: linux-fsdevel, linux-kernel, linux-cifs
  Cc: linux-cifsd-devel, smfrench, senozhatsky, hyc.lee, viro, hch,
	hch, ronniesahlberg, aurelien.aptel, aaptel, sandeen,
	dan.carpenter, colin.king, rdunlap, Namjae Jeon,
	Sergey Senozhatsky, Steve French

This adds the Kconfig and Makefile for cifsd.

Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com>
Acked-by: Ronnie Sahlberg <lsahlber@redhat.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/Kconfig        |  1 +
 fs/Makefile       |  1 +
 fs/cifsd/Kconfig  | 64 +++++++++++++++++++++++++++++++++++++++++++++++
 fs/cifsd/Makefile | 13 ++++++++++
 4 files changed, 79 insertions(+)
 create mode 100644 fs/cifsd/Kconfig
 create mode 100644 fs/cifsd/Makefile

diff --git a/fs/Kconfig b/fs/Kconfig
index a55bda4233bb..92deb66021d1 100644
--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -340,6 +340,7 @@ config NFS_V4_2_SSC_HELPER
 source "net/sunrpc/Kconfig"
 source "fs/ceph/Kconfig"
 source "fs/cifs/Kconfig"
+source "fs/cifsd/Kconfig"
 source "fs/coda/Kconfig"
 source "fs/afs/Kconfig"
 source "fs/9p/Kconfig"
diff --git a/fs/Makefile b/fs/Makefile
index 3215fe205256..62dc87f3ff94 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -97,6 +97,7 @@ obj-$(CONFIG_NLS)		+= nls/
 obj-$(CONFIG_UNICODE)		+= unicode/
 obj-$(CONFIG_SYSV_FS)		+= sysv/
 obj-$(CONFIG_CIFS)		+= cifs/
+obj-$(CONFIG_SMB_SERVER)	+= cifsd/
 obj-$(CONFIG_HPFS_FS)		+= hpfs/
 obj-$(CONFIG_NTFS_FS)		+= ntfs/
 obj-$(CONFIG_UFS_FS)		+= ufs/
diff --git a/fs/cifsd/Kconfig b/fs/cifsd/Kconfig
new file mode 100644
index 000000000000..6e78960be5b1
--- /dev/null
+++ b/fs/cifsd/Kconfig
@@ -0,0 +1,64 @@
+config SMB_SERVER
+	tristate "SMB server support (EXPERIMENTAL)"
+	depends on INET
+	select NLS
+	select NLS_UTF8
+	select CRYPTO
+	select CRYPTO_MD4
+	select CRYPTO_MD5
+	select CRYPTO_HMAC
+	select CRYPTO_ARC4
+	select CRYPTO_ECB
+	select CRYPTO_LIB_DES
+	select CRYPTO_SHA256
+	select CRYPTO_CMAC
+	select CRYPTO_SHA512
+	select CRYPTO_AEAD2
+	select CRYPTO_CCM
+	select CRYPTO_GCM
+	default n
+	help
+	  Choose Y here if you want to allow SMB3 compliant clients
+	  to access files residing on this system using SMB3 protocol.
+	  To compile the SMB3 server support as a module,
+	  choose M here: the module will be called ksmbd.
+
+	  You may choose to use a samba server instead, in which
+	  case you can choose N here.
+
+	  You also need to install user space programs which can be found
+	  in cifsd-tools, available from
+	  https://github.com/cifsd-team/cifsd-tools.
+	  More detail about how to run the cifsd kernel server is
+	  available via README file
+	  (https://github.com/cifsd-team/cifsd-tools/blob/master/README).
+
+	  cifsd kernel server includes support for auto-negotiation,
+	  Secure negotiate, Pre-authentication integrity, oplock/lease,
+	  compound requests, multi-credit, packet signing, RDMA(smbdirect),
+	  smb3 encryption, copy-offload, secure per-user session
+	  establishment via NTLM or NTLMv2.
+
+config SMB_SERVER_SMBDIRECT
+	bool "Support for SMB Direct protocol"
+	depends on SMB_SERVER=m && INFINIBAND && INFINIBAND_ADDR_TRANS || SMB_SERVER=y && INFINIBAND=y && INFINIBAND_ADDR_TRANS=y
+	default n
+
+	help
+	  Enables SMB Direct support for SMB 3.0, 3.02 and 3.1.1.
+
+	  SMB Direct allows transferring SMB packets over RDMA. If unsure,
+	  say N.
+
+config SMB_SERVER_CHECK_CAP_NET_ADMIN
+	bool "Enable check network administration capability"
+	depends on SMB_SERVER
+	default y
+
+	help
+	  Prevent unprivileged processes to start the cifsd kernel server.
+
+config SMB_SERVER_KERBEROS5
+	bool "Support for Kerberos 5"
+	depends on SMB_SERVER
+	default n
diff --git a/fs/cifsd/Makefile b/fs/cifsd/Makefile
new file mode 100644
index 000000000000..a6c03c4ba51e
--- /dev/null
+++ b/fs/cifsd/Makefile
@@ -0,0 +1,13 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+#
+# Makefile for Linux SMB3 kernel server
+#
+obj-$(CONFIG_SMB_SERVER) += ksmbd.o
+
+ksmbd-y :=	unicode.o auth.o vfs.o vfs_cache.o server.o buffer_pool.o \
+		misc.o oplock.o connection.o ksmbd_work.o crypto_ctx.o \
+		mgmt/ksmbd_ida.o mgmt/user_config.o mgmt/share_config.o \
+		mgmt/tree_connect.o mgmt/user_session.o smb_common.o \
+		transport_tcp.o transport_ipc.o smbacl.o smb2pdu.o \
+		smb2ops.o smb2misc.o asn1.o netmisc.o ndr.o
+ksmbd-$(CONFIG_SMB_SERVER_SMBDIRECT) += transport_rdma.o
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* [PATCH 5/5] MAINTAINERS: add cifsd kernel server
       [not found]   ` <CGME20210322052209epcas1p377f1542bcc9ec50219d2e57aa92d944b@epcas1p3.samsung.com>
@ 2021-03-22  5:13     ` Namjae Jeon
  0 siblings, 0 replies; 31+ messages in thread
From: Namjae Jeon @ 2021-03-22  5:13 UTC (permalink / raw)
  To: linux-fsdevel, linux-kernel, linux-cifs
  Cc: linux-cifsd-devel, smfrench, senozhatsky, hyc.lee, viro, hch,
	hch, ronniesahlberg, aurelien.aptel, aaptel, sandeen,
	dan.carpenter, colin.king, rdunlap, Namjae Jeon,
	Sergey Senozhatsky, Steve French

Add myself, Steve French, Sergey Senozhatsky and Hyunchul Lee
as cifsd maintainer.

Signed-off-by: Namjae Jeon <namjae.jeon@samsung.com>
Signed-off-by: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Signed-off-by: Hyunchul Lee <hyc.lee@gmail.com>
Acked-by: Ronnie Sahlberg <lsahlber@redhat.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 MAINTAINERS | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/MAINTAINERS b/MAINTAINERS
index aa84121c5611..30f678f8b4d3 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4434,7 +4434,7 @@ F:	include/linux/clk/
 F:	include/linux/of_clk.h
 X:	drivers/clk/clkdev.c
 
-COMMON INTERNET FILE SYSTEM (CIFS)
+COMMON INTERNET FILE SYSTEM CLIENT (CIFS)
 M:	Steve French <sfrench@samba.org>
 L:	linux-cifs@vger.kernel.org
 L:	samba-technical@lists.samba.org (moderated for non-subscribers)
@@ -4444,6 +4444,16 @@ T:	git git://git.samba.org/sfrench/cifs-2.6.git
 F:	Documentation/admin-guide/cifs/
 F:	fs/cifs/
 
+COMMON INTERNET FILE SYSTEM SERVER (CIFSD)
+M:	Namjae Jeon <namjae.jeon@samsung.com>
+M:	Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
+M:	Steve French <sfrench@samba.org>
+M:	Hyunchul Lee <hyc.lee@gmail.com>
+L:	linux-cifs@vger.kernel.org
+L:	linux-cifsd-devel@lists.sourceforge.net
+S:	Maintained
+F:	fs/cifsd/
+
 COMPACTPCI HOTPLUG CORE
 M:	Scott Murray <scott@spiteful.org>
 L:	linux-pci@vger.kernel.org
-- 
2.17.1


^ permalink raw reply related	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/5] cifsd: add server-side procedures for SMB3
       [not found]     ` <20210322051344.1706-3-namjae.jeon@samsung.com>
@ 2021-03-22  6:47       ` Dan Carpenter
  2021-03-22  6:50         ` Christoph Hellwig
  2021-03-22 23:17         ` Namjae Jeon
  2021-03-22  8:34       ` Matthew Wilcox
  1 sibling, 2 replies; 31+ messages in thread
From: Dan Carpenter @ 2021-03-22  6:47 UTC (permalink / raw)
  To: Namjae Jeon
  Cc: linux-fsdevel, linux-kernel, linux-cifs, linux-cifsd-devel,
	smfrench, senozhatsky, hyc.lee, viro, hch, hch, ronniesahlberg,
	aurelien.aptel, aaptel, sandeen, colin.king, rdunlap,
	Sergey Senozhatsky, Steve French

On Mon, Mar 22, 2021 at 02:13:41PM +0900, Namjae Jeon wrote:
> +static unsigned char
> +asn1_octet_decode(struct asn1_ctx *ctx, unsigned char *ch)
> +{
> +	if (ctx->pointer >= ctx->end) {
> +		ctx->error = ASN1_ERR_DEC_EMPTY;
> +		return 0;
> +	}
> +	*ch = *(ctx->pointer)++;
> +	return 1;
> +}


Make this bool.

> +
> +static unsigned char
> +asn1_tag_decode(struct asn1_ctx *ctx, unsigned int *tag)
> +{
> +	unsigned char ch;
> +
> +	*tag = 0;
> +
> +	do {
> +		if (!asn1_octet_decode(ctx, &ch))
> +			return 0;
> +		*tag <<= 7;
> +		*tag |= ch & 0x7F;
> +	} while ((ch & 0x80) == 0x80);
> +	return 1;
> +}

Bool.

> +
> +static unsigned char
> +asn1_id_decode(struct asn1_ctx *ctx,
> +	       unsigned int *cls, unsigned int *con, unsigned int *tag)
> +{
> +	unsigned char ch;
> +
> +	if (!asn1_octet_decode(ctx, &ch))
> +		return 0;
> +
> +	*cls = (ch & 0xC0) >> 6;
> +	*con = (ch & 0x20) >> 5;
> +	*tag = (ch & 0x1F);
> +
> +	if (*tag == 0x1F) {
> +		if (!asn1_tag_decode(ctx, tag))
> +			return 0;
> +	}
> +	return 1;
> +}


Same.

> +
> +static unsigned char
> +asn1_length_decode(struct asn1_ctx *ctx, unsigned int *def, unsigned int *len)
> +{
> +	unsigned char ch, cnt;
> +
> +	if (!asn1_octet_decode(ctx, &ch))
> +		return 0;
> +
> +	if (ch == 0x80)
> +		*def = 0;
> +	else {
> +		*def = 1;
> +
> +		if (ch < 0x80)
> +			*len = ch;
> +		else {
> +			cnt = (unsigned char) (ch & 0x7F);
> +			*len = 0;
> +
> +			while (cnt > 0) {
> +				if (!asn1_octet_decode(ctx, &ch))
> +					return 0;
> +				*len <<= 8;
> +				*len |= ch;
> +				cnt--;
> +			}
> +		}
> +	}
> +
> +	/* don't trust len bigger than ctx buffer */
> +	if (*len > ctx->end - ctx->pointer)
> +		return 0;
> +
> +	return 1;
> +}


Same etc for all.

> +
> +static unsigned char
> +asn1_header_decode(struct asn1_ctx *ctx,
> +		   unsigned char **eoc,
> +		   unsigned int *cls, unsigned int *con, unsigned int *tag)
> +{
> +	unsigned int def = 0;
> +	unsigned int len = 0;
> +
> +	if (!asn1_id_decode(ctx, cls, con, tag))
> +		return 0;
> +
> +	if (!asn1_length_decode(ctx, &def, &len))
> +		return 0;
> +
> +	/* primitive shall be definite, indefinite shall be constructed */
> +	if (*con == ASN1_PRI && !def)
> +		return 0;
> +
> +	if (def)
> +		*eoc = ctx->pointer + len;
> +	else
> +		*eoc = NULL;
> +	return 1;
> +}
> +
> +static unsigned char
> +asn1_eoc_decode(struct asn1_ctx *ctx, unsigned char *eoc)
> +{
> +	unsigned char ch;
> +
> +	if (!eoc) {
> +		if (!asn1_octet_decode(ctx, &ch))
> +			return 0;
> +
> +		if (ch != 0x00) {
> +			ctx->error = ASN1_ERR_DEC_EOC_MISMATCH;
> +			return 0;
> +		}
> +
> +		if (!asn1_octet_decode(ctx, &ch))
> +			return 0;
> +
> +		if (ch != 0x00) {
> +			ctx->error = ASN1_ERR_DEC_EOC_MISMATCH;
> +			return 0;
> +		}
> +	} else {
> +		if (ctx->pointer != eoc) {
> +			ctx->error = ASN1_ERR_DEC_LENGTH_MISMATCH;
> +			return 0;
> +		}
> +	}
> +	return 1;
> +}
> +
> +static unsigned char
> +asn1_subid_decode(struct asn1_ctx *ctx, unsigned long *subid)
> +{
> +	unsigned char ch;
> +
> +	*subid = 0;
> +
> +	do {
> +		if (!asn1_octet_decode(ctx, &ch))
> +			return 0;
> +
> +		*subid <<= 7;
> +		*subid |= ch & 0x7F;
> +	} while ((ch & 0x80) == 0x80);
> +	return 1;
> +}
> +
> +static int
> +asn1_oid_decode(struct asn1_ctx *ctx,
> +		unsigned char *eoc, unsigned long **oid, unsigned int *len)
> +{
> +	unsigned long subid;
> +	unsigned int size;
> +	unsigned long *optr;
> +
> +	size = eoc - ctx->pointer + 1;
> +
> +	/* first subid actually encodes first two subids */
> +	if (size < 2 || size > UINT_MAX/sizeof(unsigned long))
> +		return 0;
> +
> +	*oid = kmalloc(size * sizeof(unsigned long), GFP_KERNEL);
> +	if (!*oid)
> +		return 0;
> +
> +	optr = *oid;
> +
> +	if (!asn1_subid_decode(ctx, &subid)) {
> +		kfree(*oid);
> +		*oid = NULL;
> +		return 0;
> +	}
> +
> +	if (subid < 40) {
> +		optr[0] = 0;
> +		optr[1] = subid;
> +	} else if (subid < 80) {
> +		optr[0] = 1;
> +		optr[1] = subid - 40;
> +	} else {
> +		optr[0] = 2;
> +		optr[1] = subid - 80;
> +	}
> +
> +	*len = 2;
> +	optr += 2;
> +
> +	while (ctx->pointer < eoc) {
> +		if (++(*len) > size) {
> +			ctx->error = ASN1_ERR_DEC_BADVALUE;
> +			kfree(*oid);
> +			*oid = NULL;
> +			return 0;
> +		}
> +
> +		if (!asn1_subid_decode(ctx, optr++)) {
> +			kfree(*oid);
> +			*oid = NULL;
> +			return 0;
> +		}
> +	}
> +	return 1;
> +}

Still bool.

> +
> +static int
> +compare_oid(unsigned long *oid1, unsigned int oid1len,
> +	    unsigned long *oid2, unsigned int oid2len)
> +{
> +	unsigned int i;
> +
> +	if (oid1len != oid2len)
> +		return 0;
> +
> +	for (i = 0; i < oid1len; i++) {
> +		if (oid1[i] != oid2[i])
> +			return 0;
> +	}
> +	return 1;
> +}

Call this oid_eq()?


> +
> +/* BB check for endian conversion issues here */
> +
> +int
> +ksmbd_decode_negTokenInit(unsigned char *security_blob, int length,
> +		    struct ksmbd_conn *conn)
> +{
> +	struct asn1_ctx ctx;
> +	unsigned char *end;
> +	unsigned char *sequence_end;
> +	unsigned long *oid = NULL;
> +	unsigned int cls, con, tag, oidlen, rc, mechTokenlen;
> +	unsigned int mech_type;
> +
> +	ksmbd_debug(AUTH, "Received SecBlob: length %d\n", length);
> +
> +	asn1_open(&ctx, security_blob, length);
> +
> +	/* GSSAPI header */
> +	if (asn1_header_decode(&ctx, &end, &cls, &con, &tag) == 0) {
> +		ksmbd_debug(AUTH, "Error decoding negTokenInit header\n");
> +		return 0;
> +	} else if ((cls != ASN1_APL) || (con != ASN1_CON)

No need for else after a return 0;  Surely, checkpatch complains about
|| on the following line and the extra parentheses?

	if (asn1_header_decode(&ctx, &end, &cls, &con, &tag) == 0) {
		ksmbd_debug(AUTH, "Error decoding negTokenInit header\n");
		return false;
	}

	if (cls != ASN1_APL || con != ASN1_CON || tag != ASN1_EOC) {
		ksmbd_debug(AUTH, "cls = %d con = %d tag = %d\n", cls, con,
			    tag);
		return false;
	}

> +		   || (tag != ASN1_EOC)) {
> +		ksmbd_debug(AUTH, "cls = %d con = %d tag = %d\n", cls, con,
> +			tag);
> +		return 0;
> +	}
> +
> +	/* Check for SPNEGO OID -- remember to free obj->oid */
> +	rc = asn1_header_decode(&ctx, &end, &cls, &con, &tag);
> +	if (rc) {

This code confused the me at first.  I've always assumed "rc" stands for
"return code" but asn1_header_decode() doesn't return error codes, it
returns true false.  Alway do failure handling, instead of success
handling.  That way when you're reading the code you can just read the
code indented one tab to see what it does and the code indented two
tabs to see how the error handling works.

Good:

	frob();
	if (fail)
		clean up();
	frob();
	if (fail)
		clean up();

Bad:
	frob();
	if (success)
		frob();
		if (success)
			frob();
			if (success)
				frob();
		else
			fail = 1;
	if (fail)
		clean up();

So this code confused me.  Keep the ordering consistent with cls, con,
and tag.  In fact just write it exactly like the lines before.

	if (!asn1_header_decode(&ctx, &end, &cls, &con, &tag)) {
		ksmbd_debug(AUTH, "Error decoding negTokenInit header\n");
		return false;
	}

	if (cls != ASN1_UNI || con != ASN1_PRI || tag != ASN1_OJI) {
		ksmbd_debug(AUTH, "cls = %d con = %d tag = %d\n", cls, con,
			    tag);
		return false;
	}

	if (!asn1_oid_decode(&ctx, end, &oid, &oidlen))
		return false;
	if (!oid_equiv()) {
		free();
		return false;
	}

	kfree(oid); <-- I added this

Add a kfree(oid) to the success path to avoid a memory leak.

> +		if ((tag == ASN1_OJI) && (con == ASN1_PRI) &&
> +		    (cls == ASN1_UNI)) {
> +			rc = asn1_oid_decode(&ctx, end, &oid, &oidlen);
> +			if (rc) {
> +				rc = compare_oid(oid, oidlen, SPNEGO_OID,
> +						 SPNEGO_OID_LEN);
> +				kfree(oid);
> +			}
> +		} else
> +			rc = 0;
> +	}
> +
> +	/* SPNEGO OID not present or garbled -- bail out */
> +	if (!rc) {
> +		ksmbd_debug(AUTH, "Error decoding negTokenInit header\n");
> +		return 0;
> +	}
> +
> +	/* SPNEGO */
> +	if (asn1_header_decode(&ctx, &end, &cls, &con, &tag) == 0) {
> +		ksmbd_debug(AUTH, "Error decoding negTokenInit\n");
> +		return 0;
> +	} else if ((cls != ASN1_CTX) || (con != ASN1_CON)
> +		   || (tag != ASN1_EOC)) {
> +		ksmbd_debug(AUTH,
> +			"cls = %d con = %d tag = %d end = %p (%d) exit 0\n",
> +			cls, con, tag, end, *end);
> +		return 0;
> +	}
> +
> +	/* negTokenInit */
> +	if (asn1_header_decode(&ctx, &end, &cls, &con, &tag) == 0) {
> +		ksmbd_debug(AUTH, "Error decoding negTokenInit\n");
> +		return 0;
> +	} else if ((cls != ASN1_UNI) || (con != ASN1_CON)
> +		   || (tag != ASN1_SEQ)) {
> +		ksmbd_debug(AUTH,
> +			"cls = %d con = %d tag = %d end = %p (%d) exit 1\n",
> +			cls, con, tag, end, *end);
> +		return 0;
> +	}
> +
> +	/* sequence */
> +	if (asn1_header_decode(&ctx, &end, &cls, &con, &tag) == 0) {
> +		ksmbd_debug(AUTH, "Error decoding 2nd part of negTokenInit\n");
> +		return 0;
> +	} else if ((cls != ASN1_CTX) || (con != ASN1_CON)
> +		   || (tag != ASN1_EOC)) {
> +		ksmbd_debug(AUTH,
> +			"cls = %d con = %d tag = %d end = %p (%d) exit 0\n",
> +			cls, con, tag, end, *end);
> +		return 0;
> +	}
> +
> +	/* sequence of */
> +	if (asn1_header_decode
> +	    (&ctx, &sequence_end, &cls, &con, &tag) == 0) {


I just ran checkpatch.pl on your patch and I see that you actually fixed
all the normal checkpatch.pl warnings.  But I'm used to checkpatch.pl
--strict code because that's the default in net and staging.  This file
has 1249 little things like this where checkpatch would have said to
write it like:

	if (!asn1_header_decode(&ctx, &sequence_end, &cls, &con, &tag)) {

total: 1 errors, 1 warnings, 1249 checks, 24501 lines checked

Once a patch has over a thousand style issues then it's too much for
me to handle.  :P

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/5] cifsd: add server-side procedures for SMB3
  2021-03-22  6:47       ` [PATCH 2/5] cifsd: add server-side procedures for SMB3 Dan Carpenter
@ 2021-03-22  6:50         ` Christoph Hellwig
  2021-03-22 13:25           ` [Linux-cifsd-devel] " Stefan Metzmacher
  2021-03-22 23:20           ` Namjae Jeon
  2021-03-22 23:17         ` Namjae Jeon
  1 sibling, 2 replies; 31+ messages in thread
From: Christoph Hellwig @ 2021-03-22  6:50 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Namjae Jeon, linux-fsdevel, linux-kernel, linux-cifs,
	linux-cifsd-devel, smfrench, senozhatsky, hyc.lee, viro, hch,
	hch, ronniesahlberg, aurelien.aptel, aaptel, sandeen, colin.king,
	rdunlap, Sergey Senozhatsky, Steve French

On Mon, Mar 22, 2021 at 09:47:13AM +0300, Dan Carpenter wrote:
> On Mon, Mar 22, 2021 at 02:13:41PM +0900, Namjae Jeon wrote:
> > +static unsigned char
> > +asn1_octet_decode(struct asn1_ctx *ctx, unsigned char *ch)
> > +{
> > +	if (ctx->pointer >= ctx->end) {
> > +		ctx->error = ASN1_ERR_DEC_EMPTY;
> > +		return 0;
> > +	}
> > +	*ch = *(ctx->pointer)++;
> > +	return 1;
> > +}
> 
> 
> Make this bool.
>

More importantly don't add another ANS1 parser, but use the generic
one in lib/asn1_decoder.c instead.  CIFS should also really use it.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 3/5] cifsd: add file operations
  2021-03-22  5:13     ` [PATCH 3/5] cifsd: add file operations Namjae Jeon
@ 2021-03-22  6:55       ` Al Viro
  2021-03-23  0:12         ` Namjae Jeon
  2021-03-22  7:02       ` Al Viro
                         ` (2 subsequent siblings)
  3 siblings, 1 reply; 31+ messages in thread
From: Al Viro @ 2021-03-22  6:55 UTC (permalink / raw)
  To: Namjae Jeon
  Cc: linux-fsdevel, linux-kernel, linux-cifs, linux-cifsd-devel,
	smfrench, senozhatsky, hyc.lee, hch, hch, ronniesahlberg,
	aurelien.aptel, aaptel, sandeen, dan.carpenter, colin.king,
	rdunlap, Sergey Senozhatsky, Steve French

On Mon, Mar 22, 2021 at 02:13:42PM +0900, Namjae Jeon wrote:
> This adds file operations and buffer pool for cifsd.

Some random notes:

> +static void rollback_path_modification(char *filename)
> +{
> +	if (filename) {
> +		filename--;
> +		*filename = '/';
What an odd way to spell filename[-1] = '/';...

> +int ksmbd_vfs_inode_permission(struct dentry *dentry, int acc_mode, bool delete)
> +{

> +	if (delete) {
> +		struct dentry *parent;
> +
> +		parent = dget_parent(dentry);
> +		if (!parent)
> +			return -EINVAL;
> +
> +		if (inode_permission(&init_user_ns, d_inode(parent), MAY_EXEC | MAY_WRITE)) {
> +			dput(parent);
> +			return -EACCES;
> +		}
> +		dput(parent);

Who's to guarantee that parent is stable?  IOW, by the time of that
inode_permission() call dentry might very well not be a child of that thing...

> +	parent = dget_parent(dentry);
> +	if (!parent)
> +		return 0;
> +
> +	if (!inode_permission(&init_user_ns, d_inode(parent), MAY_EXEC | MAY_WRITE))
> +		*daccess |= FILE_DELETE_LE;

Ditto.

> +int ksmbd_vfs_mkdir(struct ksmbd_work *work,
> +		    const char *name,
> +		    umode_t mode)


> +	err = vfs_mkdir(&init_user_ns, d_inode(path.dentry), dentry, mode);
> +	if (!err) {
> +		ksmbd_vfs_inherit_owner(work, d_inode(path.dentry),
> +			d_inode(dentry));

->mkdir() might very well return success, with dentry left unhashed negative.
Look at the callers of vfs_mkdir() to see how it should be handled.

> +static int check_lock_range(struct file *filp,
> +			    loff_t start,
> +			    loff_t end,
> +			    unsigned char type)
> +{
> +	struct file_lock *flock;
> +	struct file_lock_context *ctx = file_inode(filp)->i_flctx;
> +	int error = 0;
> +
> +	if (!ctx || list_empty_careful(&ctx->flc_posix))
> +		return 0;
> +
> +	spin_lock(&ctx->flc_lock);
> +	list_for_each_entry(flock, &ctx->flc_posix, fl_list) {
> +		/* check conflict locks */
> +		if (flock->fl_end >= start && end >= flock->fl_start) {
> +			if (flock->fl_type == F_RDLCK) {
> +				if (type == WRITE) {
> +					ksmbd_err("not allow write by shared lock\n");
> +					error = 1;
> +					goto out;
> +				}
> +			} else if (flock->fl_type == F_WRLCK) {
> +				/* check owner in lock */
> +				if (flock->fl_file != filp) {
> +					error = 1;
> +					ksmbd_err("not allow rw access by exclusive lock from other opens\n");
> +					goto out;
> +				}
> +			}
> +		}
> +	}
> +out:
> +	spin_unlock(&ctx->flc_lock);
> +	return error;
> +}

WTF is that doing in smbd?

> +	filp = fp->filp;
> +	inode = d_inode(filp->f_path.dentry);

That should be file_inode().  Try it on overlayfs, watch it do interesting things...

> +	nbytes = kernel_read(filp, rbuf, count, pos);
> +	if (nbytes < 0) {
> +		name = d_path(&filp->f_path, namebuf, sizeof(namebuf));
> +		if (IS_ERR(name))
> +			name = "(error)";
> +		ksmbd_err("smb read failed for (%s), err = %zd\n",
> +				name, nbytes);

Do you really want the full pathname here?  For (presumably) spew into syslog?

> +int ksmbd_vfs_remove_file(struct ksmbd_work *work, char *name)
> +{
> +	struct path parent;
> +	struct dentry *dir, *dentry;
> +	char *last;
> +	int err = -ENOENT;
> +
> +	last = extract_last_component(name);
> +	if (!last)
> +		return -ENOENT;

Yeccchhh...

> +	if (ksmbd_override_fsids(work))
> +		return -ENOMEM;
> +
> +	err = kern_path(name, LOOKUP_FOLLOW | LOOKUP_DIRECTORY, &parent);
> +	if (err) {
> +		ksmbd_debug(VFS, "can't get %s, err %d\n", name, err);
> +		ksmbd_revert_fsids(work);
> +		rollback_path_modification(last);
> +		return err;
> +	}
> +
> +	dir = parent.dentry;
> +	if (!d_inode(dir))
> +		goto out;

Really?  When does that happen?

> +static int __ksmbd_vfs_rename(struct ksmbd_work *work,
> +			      struct dentry *src_dent_parent,
> +			      struct dentry *src_dent,
> +			      struct dentry *dst_dent_parent,
> +			      struct dentry *trap_dent,
> +			      char *dst_name)
> +{
> +	struct dentry *dst_dent;
> +	int err;
> +
> +	spin_lock(&src_dent->d_lock);
> +	list_for_each_entry(dst_dent, &src_dent->d_subdirs, d_child) {
> +		struct ksmbd_file *child_fp;
> +
> +		if (d_really_is_negative(dst_dent))
> +			continue;
> +
> +		child_fp = ksmbd_lookup_fd_inode(d_inode(dst_dent));
> +		if (child_fp) {
> +			spin_unlock(&src_dent->d_lock);
> +			ksmbd_debug(VFS, "Forbid rename, sub file/dir is in use\n");
> +			return -EACCES;
> +		}
> +	}
> +	spin_unlock(&src_dent->d_lock);

Hard NAK right there.  That thing has no business poking at that level.
And I'm pretty certain that it's racy as hell.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 3/5] cifsd: add file operations
  2021-03-22  5:13     ` [PATCH 3/5] cifsd: add file operations Namjae Jeon
  2021-03-22  6:55       ` Al Viro
@ 2021-03-22  7:02       ` Al Viro
  2021-03-22  9:26         ` Sergey Senozhatsky
  2021-03-22  7:04       ` Dan Carpenter
  2021-03-22  8:15       ` Matthew Wilcox
  3 siblings, 1 reply; 31+ messages in thread
From: Al Viro @ 2021-03-22  7:02 UTC (permalink / raw)
  To: Namjae Jeon
  Cc: linux-fsdevel, linux-kernel, linux-cifs, linux-cifsd-devel,
	smfrench, senozhatsky, hyc.lee, hch, hch, ronniesahlberg,
	aurelien.aptel, aaptel, sandeen, dan.carpenter, colin.king,
	rdunlap, Sergey Senozhatsky, Steve French

On Mon, Mar 22, 2021 at 02:13:42PM +0900, Namjae Jeon wrote:

> +static struct ksmbd_file *__ksmbd_lookup_fd(struct ksmbd_file_table *ft,
> +					    unsigned int id)
> +{
> +	bool unclaimed = true;
> +	struct ksmbd_file *fp;
> +
> +	read_lock(&ft->lock);
> +	fp = idr_find(ft->idr, id);
> +	if (fp)
> +		fp = ksmbd_fp_get(fp);
> +
> +	if (fp && fp->f_ci) {
> +		read_lock(&fp->f_ci->m_lock);
> +		unclaimed = list_empty(&fp->node);
> +		read_unlock(&fp->f_ci->m_lock);
> +	}
> +	read_unlock(&ft->lock);
> +
> +	if (fp && unclaimed) {
> +		atomic_dec(&fp->refcount);
> +		return NULL;
> +	}

Can that atomic_dec() end up dropping the last remaining reference?
If not, what's to prevent that?

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 3/5] cifsd: add file operations
  2021-03-22  5:13     ` [PATCH 3/5] cifsd: add file operations Namjae Jeon
  2021-03-22  6:55       ` Al Viro
  2021-03-22  7:02       ` Al Viro
@ 2021-03-22  7:04       ` Dan Carpenter
  2021-03-22  9:39         ` Sergey Senozhatsky
  2021-03-22  8:15       ` Matthew Wilcox
  3 siblings, 1 reply; 31+ messages in thread
From: Dan Carpenter @ 2021-03-22  7:04 UTC (permalink / raw)
  To: Namjae Jeon
  Cc: linux-fsdevel, linux-kernel, linux-cifs, linux-cifsd-devel,
	smfrench, senozhatsky, hyc.lee, viro, hch, hch, ronniesahlberg,
	aurelien.aptel, aaptel, sandeen, colin.king, rdunlap,
	Sergey Senozhatsky, Steve French

On Mon, Mar 22, 2021 at 02:13:42PM +0900, Namjae Jeon wrote:
> +void *ksmbd_alloc(size_t size)
> +{
> +	return kvmalloc(size, GFP_KERNEL | __GFP_ZERO);


This patch adds a bunch of wrappers around kvmalloc().  Don't do that.
Just use kvmalloc() directly instead.  They just make the code hard to
read.  kvmalloc() is not appropriate for small allocations.  This
wrapper hides a GFP_KERNEL which may lead to scheduling in atomic bugs
and the secret ZEROing makes the code hard to read.

> +}
> +
> +void ksmbd_free(void *ptr)
> +{
> +	kvfree(ptr);
> +}
> +
> +static struct wm *wm_alloc(size_t sz, gfp_t flags)
> +{
> +	struct wm *wm;
> +	size_t alloc_sz = sz + sizeof(struct wm);
                          ^^^^^^^^^^^^^^^^^^^^^^

Check for integer overflow.

> +
> +	wm = kvmalloc(alloc_sz, flags);
> +	if (!wm)
> +		return NULL;
> +	wm->sz = sz;
> +	return wm;
> +}
> +
> +static int register_wm_size_class(size_t sz)
> +{
> +	struct wm_list *l, *nl;
> +
> +	nl = kvmalloc(sizeof(struct wm_list), GFP_KERNEL);

Just use kmalloc() for small allocations.

> +	if (!nl)
> +		return -ENOMEM;
> +
> +	nl->sz = sz;
> +	spin_lock_init(&nl->wm_lock);
> +	INIT_LIST_HEAD(&nl->idle_wm);
> +	INIT_LIST_HEAD(&nl->list);
> +	init_waitqueue_head(&nl->wm_wait);
> +	nl->avail_wm = 0;
> +
> +	write_lock(&wm_lists_lock);
> +	list_for_each_entry(l, &wm_lists, list) {
> +		if (l->sz == sz) {
> +			write_unlock(&wm_lists_lock);
> +			kvfree(nl);
> +			return 0;
> +		}
> +	}
> +
> +	list_add(&nl->list, &wm_lists);
> +	write_unlock(&wm_lists_lock);
> +	return 0;
> +}
> +
> +static struct wm_list *match_wm_list(size_t size)
> +{
> +	struct wm_list *l, *rl = NULL;
> +
> +	read_lock(&wm_lists_lock);
> +	list_for_each_entry(l, &wm_lists, list) {
> +		if (l->sz == size) {
> +			rl = l;
> +			break;
> +		}
> +	}
> +	read_unlock(&wm_lists_lock);
> +	return rl;
> +}
> +
> +static struct wm *find_wm(size_t size)
> +{
> +	struct wm_list *wm_list;
> +	struct wm *wm;
> +
> +	wm_list = match_wm_list(size);
> +	if (!wm_list) {
> +		if (register_wm_size_class(size))
> +			return NULL;
> +		wm_list = match_wm_list(size);
> +	}
> +
> +	if (!wm_list)
> +		return NULL;
> +
> +	while (1) {
> +		spin_lock(&wm_list->wm_lock);
> +		if (!list_empty(&wm_list->idle_wm)) {
> +			wm = list_entry(wm_list->idle_wm.next,
> +					struct wm,
> +					list);
> +			list_del(&wm->list);
> +			spin_unlock(&wm_list->wm_lock);
> +			return wm;
> +		}
> +
> +		if (wm_list->avail_wm > num_online_cpus()) {
> +			spin_unlock(&wm_list->wm_lock);
> +			wait_event(wm_list->wm_wait,
> +				   !list_empty(&wm_list->idle_wm));
> +			continue;
> +		}
> +
> +		wm_list->avail_wm++;

I don't think we should increment this until after the allocation
succeeds?

> +		spin_unlock(&wm_list->wm_lock);
> +
> +		wm = wm_alloc(size, GFP_KERNEL);
> +		if (!wm) {
> +			spin_lock(&wm_list->wm_lock);
> +			wm_list->avail_wm--;
> +			spin_unlock(&wm_list->wm_lock);
> +			wait_event(wm_list->wm_wait,
> +				   !list_empty(&wm_list->idle_wm));
> +			continue;
> +		}
> +		break;
> +	}
> +
> +	return wm;
> +}

regards,
dan carpenter


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 3/5] cifsd: add file operations
  2021-03-22  5:13     ` [PATCH 3/5] cifsd: add file operations Namjae Jeon
                         ` (2 preceding siblings ...)
  2021-03-22  7:04       ` Dan Carpenter
@ 2021-03-22  8:15       ` Matthew Wilcox
  2021-03-22  9:03         ` Sergey Senozhatsky
  3 siblings, 1 reply; 31+ messages in thread
From: Matthew Wilcox @ 2021-03-22  8:15 UTC (permalink / raw)
  To: Namjae Jeon
  Cc: linux-fsdevel, linux-kernel, linux-cifs, linux-cifsd-devel,
	smfrench, senozhatsky, hyc.lee, viro, hch, hch, ronniesahlberg,
	aurelien.aptel, aaptel, sandeen, dan.carpenter, colin.king,
	rdunlap, Sergey Senozhatsky, Steve French

On Mon, Mar 22, 2021 at 02:13:42PM +0900, Namjae Jeon wrote:
> This adds file operations and buffer pool for cifsd.

Why do you want this buffer pool?  Do you not trust the
slab allocator to be able to do its job?  Because what you have
here looks slower than the slab allocator.

Let's follow this through for the best-case scenario (a buffer of the right
size already exists):

> +void *ksmbd_find_buffer(size_t size)
> +{
> +	struct wm *wm;
> +
> +	wm = find_wm(size);
> +
> +	WARN_ON(!wm);
> +	if (wm)
> +		return wm->buffer;
> +	return NULL;
> +}

OK, simple, we just call find_wm().

> +static struct wm *find_wm(size_t size)
> +{
> +	struct wm_list *wm_list;
> +	struct wm *wm;
> +
> +	wm_list = match_wm_list(size);

First we find the list for this buffer ...

> +static struct wm_list *match_wm_list(size_t size)
> +{
> +	struct wm_list *l, *rl = NULL;
> +
> +	read_lock(&wm_lists_lock);
> +	list_for_each_entry(l, &wm_lists, list) {
> +		if (l->sz == size) {
> +			rl = l;
> +			break;
> +		}
> +	}
> +	read_unlock(&wm_lists_lock);
> +	return rl;
> +}

... by taking an rwlock, and walking a linked list?!  Uh ...

> +	while (1) {
> +		spin_lock(&wm_list->wm_lock);
> +		if (!list_empty(&wm_list->idle_wm)) {
> +			wm = list_entry(wm_list->idle_wm.next,
> +					struct wm,
> +					list);
> +			list_del(&wm->list);
> +			spin_unlock(&wm_list->wm_lock);
> +			return wm;

Great!  We found one!  And all it cost us was acquiring a global rwlock,
walking a linked list to find a wmlist, then a per-wmlist spinlock.

Meanwhile, there's no guarantee the buffer we found is on the local
NUMA node.

Compare to slub, allocating from a kmem_cache (assuming you create
one for each buffer size ...):

void *kmem_cache_alloc(struct kmem_cache *s, gfp_t gfpflags)
        void *ret = slab_alloc(s, gfpflags, _RET_IP_, s->object_size);

static __always_inline void *slab_alloc(struct kmem_cache *s,
                gfp_t gfpflags, unsigned long addr, size_t orig_size)
        return slab_alloc_node(s, gfpflags, NUMA_NO_NODE, addr, orig_size);

static __always_inline void *slab_alloc_node(struct kmem_cache *s,
                gfp_t gfpflags, int node, unsigned long addr, size_t orig_size)
        do {
                tid = this_cpu_read(s->cpu_slab->tid);
                c = raw_cpu_ptr(s->cpu_slab);
        } while (IS_ENABLED(CONFIG_PREEMPTION) &&
                 unlikely(tid != READ_ONCE(c->tid)));
        object = c->freelist;
        page = c->page;
        if (unlikely(!object || !page || !node_match(page, node))) {
                object = __slab_alloc(s, gfpflags, node, addr, c);
        } else {
                void *next_object = get_freepointer_safe(s, object);
                if (unlikely(!this_cpu_cmpxchg_double(
                                s->cpu_slab->freelist, s->cpu_slab->tid,
                                object, tid,
                                next_object, next_tid(tid)))) {

                        note_cmpxchg_failure("slab_alloc", s, tid);
                        goto redo;
                }
                prefetch_freepointer(s, next_object);
                stat(s, ALLOC_FASTPATH);

No lock, anywhere.  Lots of percpu goodness, so you get memory allocated
on your local node.

What's the scenario for which your allocator performs better than slub,
on a typical machine that serves enough SMB that it's worth having an
in-kernel SMBD?


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/5] cifsd: add server-side procedures for SMB3
       [not found]     ` <20210322051344.1706-3-namjae.jeon@samsung.com>
  2021-03-22  6:47       ` [PATCH 2/5] cifsd: add server-side procedures for SMB3 Dan Carpenter
@ 2021-03-22  8:34       ` Matthew Wilcox
  2021-03-22 10:27         ` Sergey Senozhatsky
  1 sibling, 1 reply; 31+ messages in thread
From: Matthew Wilcox @ 2021-03-22  8:34 UTC (permalink / raw)
  To: Namjae Jeon
  Cc: linux-fsdevel, linux-kernel, linux-cifs, linux-cifsd-devel,
	smfrench, senozhatsky, hyc.lee, viro, hch, hch, ronniesahlberg,
	aurelien.aptel, aaptel, sandeen, dan.carpenter, colin.king,
	rdunlap, Sergey Senozhatsky, Steve French

On Mon, Mar 22, 2021 at 02:13:41PM +0900, Namjae Jeon wrote:
> +++ b/fs/cifsd/mgmt/ksmbd_ida.c
> @@ -0,0 +1,69 @@
> +// SPDX-License-Identifier: GPL-2.0-or-later
> +/*
> + *   Copyright (C) 2018 Samsung Electronics Co., Ltd.
> + */
> +
> +#include "ksmbd_ida.h"
> +
> +struct ksmbd_ida *ksmbd_ida_alloc(void)
> +{
> +	struct ksmbd_ida *ida;
> +
> +	ida = kmalloc(sizeof(struct ksmbd_ida), GFP_KERNEL);
> +	if (!ida)
> +		return NULL;
> +
> +	ida_init(&ida->map);
> +	return ida;
> +}

... why?  Everywhere that you call ksmbd_ida_alloc(), you would
be better off just embedding the struct ida into the struct that
currently has a pointer to it.  Or declaring it statically.  Then
you can even initialise it statically using DEFINE_IDA() and
eliminate the initialiser functions.

I'd remove the ksmbd_ida abstraction, although I like this wrapper:

> +int ksmbd_acquire_smb2_tid(struct ksmbd_ida *ida)
> +{
> +	int id;
> +
> +	do {
> +		id = __acquire_id(ida, 0, 0);
> +	} while (id == 0xFFFF);
> +
> +	return id;

Very clever, given your constraint.  I might do it as:

	int id = ida_alloc(ida, GFP_KERNEL);
	if (id == 0xffff)
		id = ida_alloc(ida, GFP_KERNEL);
	return id;

Although ...

> +	tree_conn = ksmbd_alloc(sizeof(struct ksmbd_tree_connect));
> +	if (!tree_conn) {
> +		status.ret = -ENOMEM;
> +		goto out_error;
> +	}
> +
> +	tree_conn->id = ksmbd_acquire_tree_conn_id(sess);
> +	if (tree_conn->id < 0) {
> +		status.ret = -EINVAL;
> +		goto out_error;
> +	}
> +
> +	peer_addr = KSMBD_TCP_PEER_SOCKADDR(sess->conn);
> +	resp = ksmbd_ipc_tree_connect_request(sess,
> +					      sc,
> +					      tree_conn,
> +					      peer_addr);
> +	if (!resp) {
> +		status.ret = -EINVAL;
> +		goto out_error;
> +	}
> +
> +	status.ret = resp->status;
> +	if (status.ret != KSMBD_TREE_CONN_STATUS_OK)
> +		goto out_error;
> +
> +	tree_conn->flags = resp->connection_flags;
> +	tree_conn->user = sess->user;
> +	tree_conn->share_conf = sc;
> +	status.tree_conn = tree_conn;
> +
> +	list_add(&tree_conn->list, &sess->tree_conn_list);

This is basically the only function which calls that, and this is a relatively
common anti-pattern when using the IDA -- you've allocated a unique ID,
but then you stuff the object in a list and ...

> +struct ksmbd_tree_connect *ksmbd_tree_conn_lookup(struct ksmbd_session *sess,
> +						  unsigned int id)
> +{
> +	struct ksmbd_tree_connect *tree_conn;
> +	struct list_head *tmp;
> +
> +	list_for_each(tmp, &sess->tree_conn_list) {
> +		tree_conn = list_entry(tmp, struct ksmbd_tree_connect, list);
> +		if (tree_conn->id == id)
> +			return tree_conn;
> +	}

... walk the linked list looking for an ID match.  You'd be much better
off using an allocating XArray:
https://www.kernel.org/doc/html/latest/core-api/xarray.html

Then you could lookup tree connections in O(log(n)) time instead of
O(n) time.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 3/5] cifsd: add file operations
  2021-03-22  8:15       ` Matthew Wilcox
@ 2021-03-22  9:03         ` Sergey Senozhatsky
  2021-03-22 13:02           ` Matthew Wilcox
                             ` (2 more replies)
  0 siblings, 3 replies; 31+ messages in thread
From: Sergey Senozhatsky @ 2021-03-22  9:03 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Namjae Jeon, linux-fsdevel, linux-kernel, linux-cifs,
	linux-cifsd-devel, smfrench, senozhatsky, hyc.lee, viro, hch,
	hch, ronniesahlberg, aurelien.aptel, aaptel, sandeen,
	dan.carpenter, colin.king, rdunlap, Sergey Senozhatsky,
	Steve French

On (21/03/22 08:15), Matthew Wilcox wrote:
> 
> What's the scenario for which your allocator performs better than slub
> 

IIRC request and reply buffers can be up to 4M in size. So this stuff
just allocates a number of fat buffers and keeps them around so that
it doesn't have to vmalloc(4M) for every request and every response.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 3/5] cifsd: add file operations
  2021-03-22  7:02       ` Al Viro
@ 2021-03-22  9:26         ` Sergey Senozhatsky
  0 siblings, 0 replies; 31+ messages in thread
From: Sergey Senozhatsky @ 2021-03-22  9:26 UTC (permalink / raw)
  To: Al Viro
  Cc: Namjae Jeon, linux-fsdevel, linux-kernel, linux-cifs,
	linux-cifsd-devel, smfrench, senozhatsky, hyc.lee, hch, hch,
	ronniesahlberg, aurelien.aptel, aaptel, sandeen, dan.carpenter,
	colin.king, rdunlap, Sergey Senozhatsky, Steve French

On (21/03/22 07:02), Al Viro wrote:
> On Mon, Mar 22, 2021 at 02:13:42PM +0900, Namjae Jeon wrote:
> > +static struct ksmbd_file *__ksmbd_lookup_fd(struct ksmbd_file_table *ft,
> > +					    unsigned int id)
> > +{
> > +	bool unclaimed = true;
> > +	struct ksmbd_file *fp;
> > +
> > +	read_lock(&ft->lock);
> > +	fp = idr_find(ft->idr, id);
> > +	if (fp)
> > +		fp = ksmbd_fp_get(fp);
> > +
> > +	if (fp && fp->f_ci) {
> > +		read_lock(&fp->f_ci->m_lock);
> > +		unclaimed = list_empty(&fp->node);
> > +		read_unlock(&fp->f_ci->m_lock);
> > +	}
> > +	read_unlock(&ft->lock);
> > +
> > +	if (fp && unclaimed) {
> > +		atomic_dec(&fp->refcount);
> > +		return NULL;
> > +	}
>
> Can that atomic_dec() end up dropping the last remaining reference?

Yes, I think it should increment refcount only for "claimed" fp.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 3/5] cifsd: add file operations
  2021-03-22  7:04       ` Dan Carpenter
@ 2021-03-22  9:39         ` Sergey Senozhatsky
  0 siblings, 0 replies; 31+ messages in thread
From: Sergey Senozhatsky @ 2021-03-22  9:39 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: Namjae Jeon, linux-fsdevel, linux-kernel, linux-cifs,
	linux-cifsd-devel, smfrench, senozhatsky, hyc.lee, viro, hch,
	hch, ronniesahlberg, aurelien.aptel, aaptel, sandeen, colin.king,
	rdunlap, Sergey Senozhatsky, Steve French

On (21/03/22 10:04), Dan Carpenter wrote:
> 
> On Mon, Mar 22, 2021 at 02:13:42PM +0900, Namjae Jeon wrote:
> > +void *ksmbd_alloc(size_t size)
> > +{
> > +	return kvmalloc(size, GFP_KERNEL | __GFP_ZERO);
> 
> 
> This patch adds a bunch of wrappers around kvmalloc().  Don't do that.

Agreed. This was not cleaned up properly. The initial implementation
of that function (IIRC... it was sometime in 2018) basically contained
kvmalloc() implementation, because back in the days Samsung used kernel
version that simply didn't have kvmalloc() ( < KERNEL_VERSION(5, 0, 0))

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/5] cifsd: add server-side procedures for SMB3
  2021-03-22  8:34       ` Matthew Wilcox
@ 2021-03-22 10:27         ` Sergey Senozhatsky
  2021-03-22 13:12           ` Matthew Wilcox
  0 siblings, 1 reply; 31+ messages in thread
From: Sergey Senozhatsky @ 2021-03-22 10:27 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Namjae Jeon, linux-fsdevel, linux-kernel, linux-cifs,
	linux-cifsd-devel, smfrench, senozhatsky, hyc.lee, viro, hch,
	hch, ronniesahlberg, aurelien.aptel, aaptel, sandeen,
	dan.carpenter, colin.king, rdunlap, Sergey Senozhatsky,
	Steve French

On (21/03/22 08:34), Matthew Wilcox wrote:
> > +++ b/fs/cifsd/mgmt/ksmbd_ida.c
> > @@ -0,0 +1,69 @@
> > +// SPDX-License-Identifier: GPL-2.0-or-later
> > +/*
> > + *   Copyright (C) 2018 Samsung Electronics Co., Ltd.
> > + */
> > +
> > +#include "ksmbd_ida.h"
> > +
> > +struct ksmbd_ida *ksmbd_ida_alloc(void)
> > +{
> > +	struct ksmbd_ida *ida;
> > +
> > +	ida = kmalloc(sizeof(struct ksmbd_ida), GFP_KERNEL);
> > +	if (!ida)
> > +		return NULL;
> > +
> > +	ida_init(&ida->map);
> > +	return ida;
> > +}
>
> ... why?  Everywhere that you call ksmbd_ida_alloc(), you would
> be better off just embedding the struct ida into the struct that
> currently has a pointer to it.  Or declaring it statically.  Then
> you can even initialise it statically using DEFINE_IDA() and
> eliminate the initialiser functions.

IIRC this ida is per SMB session, so it probably cannot be static.
And Windows, IIRC, doesn't like "just any IDs". Some versions of Windows
would fail the session login if server would return the first id == 0,
instead of 1. Or vice versa. I don't remember all the details, the last
time I looked into this was in 2019.

[..]
> > +struct ksmbd_tree_connect *ksmbd_tree_conn_lookup(struct ksmbd_session *sess,
> > +						  unsigned int id)
> > +{
> > +	struct ksmbd_tree_connect *tree_conn;
> > +	struct list_head *tmp;
> > +
> > +	list_for_each(tmp, &sess->tree_conn_list) {
> > +		tree_conn = list_entry(tmp, struct ksmbd_tree_connect, list);
> > +		if (tree_conn->id == id)
> > +			return tree_conn;
> > +	}
>
> ... walk the linked list looking for an ID match.  You'd be much better
> off using an allocating XArray:
> https://www.kernel.org/doc/html/latest/core-api/xarray.html

I think cifsd code predates XArray ;)

> Then you could lookup tree connections in O(log(n)) time instead of
> O(n) time.

Agreed. Not sure I remember why the code does list traversal here.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 3/5] cifsd: add file operations
  2021-03-22  9:03         ` Sergey Senozhatsky
@ 2021-03-22 13:02           ` Matthew Wilcox
  2021-03-22 13:57           ` Christoph Hellwig
  2021-03-22 17:09           ` Matthew Wilcox
  2 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2021-03-22 13:02 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Namjae Jeon, linux-fsdevel, linux-kernel, linux-cifs,
	linux-cifsd-devel, Sergey Senozhatsky, linux-mm,
	Uladzislau Rezki

On Mon, Mar 22, 2021 at 06:03:21PM +0900, Sergey Senozhatsky wrote:
> On (21/03/22 08:15), Matthew Wilcox wrote:
> > 
> > What's the scenario for which your allocator performs better than slub
> > 
> 
> IIRC request and reply buffers can be up to 4M in size. So this stuff
> just allocates a number of fat buffers and keeps them around so that
> it doesn't have to vmalloc(4M) for every request and every response.

That makes a lot more sense; I was thrown off by the kvmalloc, which
is usually used for allocations that might be smaller than PAGE_SIZE.

So what this patch is really saying is that vmalloc() should include
some caching, so it can defer freeing until there's memory pressure
or it's built up a large (percpu) backlog of freed areas.

Vlad, have you thought about this?

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/5] cifsd: add server-side procedures for SMB3
  2021-03-22 10:27         ` Sergey Senozhatsky
@ 2021-03-22 13:12           ` Matthew Wilcox
  0 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2021-03-22 13:12 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Namjae Jeon, linux-fsdevel, linux-kernel, linux-cifs,
	linux-cifsd-devel, smfrench, hyc.lee, viro, hch, hch,
	ronniesahlberg, aurelien.aptel, aaptel, sandeen, dan.carpenter,
	colin.king, rdunlap, Sergey Senozhatsky, Steve French

On Mon, Mar 22, 2021 at 07:27:03PM +0900, Sergey Senozhatsky wrote:
> On (21/03/22 08:34), Matthew Wilcox wrote:
> > > +++ b/fs/cifsd/mgmt/ksmbd_ida.c
> > > @@ -0,0 +1,69 @@
> > > +// SPDX-License-Identifier: GPL-2.0-or-later
> > > +/*
> > > + *   Copyright (C) 2018 Samsung Electronics Co., Ltd.
> > > + */
> > > +
> > > +#include "ksmbd_ida.h"
> > > +
> > > +struct ksmbd_ida *ksmbd_ida_alloc(void)
> > > +{
> > > +	struct ksmbd_ida *ida;
> > > +
> > > +	ida = kmalloc(sizeof(struct ksmbd_ida), GFP_KERNEL);
> > > +	if (!ida)
> > > +		return NULL;
> > > +
> > > +	ida_init(&ida->map);
> > > +	return ida;
> > > +}
> >
> > ... why?  Everywhere that you call ksmbd_ida_alloc(), you would
> > be better off just embedding the struct ida into the struct that
> > currently has a pointer to it.  Or declaring it statically.  Then
> > you can even initialise it statically using DEFINE_IDA() and
> > eliminate the initialiser functions.
> 
> IIRC this ida is per SMB session, so it probably cannot be static.

Depends which IDA you're talking about.

+struct ksmbd_conn *ksmbd_conn_alloc(void)
+	conn->async_ida = ksmbd_ida_alloc();
Embed into 'conn'.

+static struct ksmbd_ida *ida;
+int ksmbd_ipc_init(void)
+	ida = ksmbd_ida_alloc();
Should be static.

> And Windows, IIRC, doesn't like "just any IDs". Some versions of Windows
> would fail the session login if server would return the first id == 0,
> instead of 1. Or vice versa. I don't remember all the details, the last
> time I looked into this was in 2019.

Sure, you can keep that logic.

> > ... walk the linked list looking for an ID match.  You'd be much better
> > off using an allocating XArray:
> > https://www.kernel.org/doc/html/latest/core-api/xarray.html
> 
> I think cifsd code predates XArray ;)

Sure, but you could have used an IDR ;-)

> > Then you could lookup tree connections in O(log(n)) time instead of
> > O(n) time.
> 
> Agreed. Not sure I remember why the code does list traversal here.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [Linux-cifsd-devel] [PATCH 2/5] cifsd: add server-side procedures for SMB3
  2021-03-22  6:50         ` Christoph Hellwig
@ 2021-03-22 13:25           ` Stefan Metzmacher
  2021-03-22 23:20           ` Namjae Jeon
  1 sibling, 0 replies; 31+ messages in thread
From: Stefan Metzmacher @ 2021-03-22 13:25 UTC (permalink / raw)
  To: Christoph Hellwig, Dan Carpenter
  Cc: linux-cifs, aurelien.aptel, linux-cifsd-devel, senozhatsky,
	rdunlap, sandeen, linux-kernel, aaptel, hch, viro,
	ronniesahlberg, linux-fsdevel, colin.king, Steve French


[-- Attachment #1.1: Type: text/plain, Size: 2414 bytes --]


Am 22.03.21 um 07:50 schrieb Christoph Hellwig:
> On Mon, Mar 22, 2021 at 09:47:13AM +0300, Dan Carpenter wrote:
>> On Mon, Mar 22, 2021 at 02:13:41PM +0900, Namjae Jeon wrote:
>>> +static unsigned char
>>> +asn1_octet_decode(struct asn1_ctx *ctx, unsigned char *ch)
>>> +{
>>> +	if (ctx->pointer >= ctx->end) {
>>> +		ctx->error = ASN1_ERR_DEC_EMPTY;
>>> +		return 0;
>>> +	}
>>> +	*ch = *(ctx->pointer)++;
>>> +	return 1;
>>> +}
>>
>>
>> Make this bool.
>>
> 
> More importantly don't add another ANS1 parser, but use the generic
> one in lib/asn1_decoder.c instead.  CIFS should also really use it.

I think the best would be to avoid asn1 completely in the kernel
and do the whole authentication in userspace.

The kernel can only deal this blobs here, I don't there's need to
look inside the blobs.

1. ksmbd-mount would provide a fixed initial blob that's always
   the same and will be returned in the
   "2.2.4 SMB2 NEGOTIATE Response" PDU as SecurityBuffer

2. The kernel just blindly forwards the SecurityBuffer
   of "2.2.5 SMB2 SESSION_SETUP Request" to userspace
   together with the client provided SessionId (from
   2.2.1.2 SMB2 Packet Header - SYNC) as well as
   negotiated signing and encryption algorithm ids
   and the latest preauth hash.

3. Userspace passes a NTSTATUS together with SecurityBuffer blob for the
   2.2.6 SMB2 SESSION_SETUP Response back to the kernel:

   - NT_STATUS_MORE_PROCESSING_REQUIRED (more authentication legs are required)
     SecurityBuffer is most likely a non empty buffer

   - NT_STATUS_OK - The authentication is complete:
     SecurityBuffer might be empty or not
     It also pass a channel signing key, a decryption and encrytion key
     as well as the unix token ( I guess in the current form it's only uid/gid)
     down to the kernel

   - Any other status means the authentication failed, which is a hard error for the client

The PDU definitions are defined here:
https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-smb2/6eaf6e75-9c23-4eda-be99-c9223c60b181

I think everything else belongs to userspace.

Such a "simple" design for the kernel part, would mean that ksmbd-mount would do what the
kernel part is currently doing, but it also means it will be trivial to plug the userspace
part to samba's winbindd in future order to get domain wide authentication.

metze


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 3/5] cifsd: add file operations
  2021-03-22  9:03         ` Sergey Senozhatsky
  2021-03-22 13:02           ` Matthew Wilcox
@ 2021-03-22 13:57           ` Christoph Hellwig
  2021-03-22 14:40             ` Matthew Wilcox
  2021-03-22 17:09           ` Matthew Wilcox
  2 siblings, 1 reply; 31+ messages in thread
From: Christoph Hellwig @ 2021-03-22 13:57 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Matthew Wilcox, Namjae Jeon, linux-fsdevel, linux-kernel,
	linux-cifs, linux-cifsd-devel, smfrench, hyc.lee, viro, hch, hch,
	ronniesahlberg, aurelien.aptel, aaptel, sandeen, dan.carpenter,
	colin.king, rdunlap, Sergey Senozhatsky, Steve French

On Mon, Mar 22, 2021 at 06:03:21PM +0900, Sergey Senozhatsky wrote:
> On (21/03/22 08:15), Matthew Wilcox wrote:
> > 
> > What's the scenario for which your allocator performs better than slub
> > 
> 
> IIRC request and reply buffers can be up to 4M in size. So this stuff
> just allocates a number of fat buffers and keeps them around so that
> it doesn't have to vmalloc(4M) for every request and every response.

Do we have any data suggesting it is faster than vmalloc?

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 3/5] cifsd: add file operations
  2021-03-22 13:57           ` Christoph Hellwig
@ 2021-03-22 14:40             ` Matthew Wilcox
  0 siblings, 0 replies; 31+ messages in thread
From: Matthew Wilcox @ 2021-03-22 14:40 UTC (permalink / raw)
  To: Christoph Hellwig
  Cc: Sergey Senozhatsky, Namjae Jeon, linux-fsdevel, linux-kernel,
	linux-cifs, linux-cifsd-devel, smfrench, hyc.lee, viro, hch,
	ronniesahlberg, aurelien.aptel, aaptel, sandeen, dan.carpenter,
	colin.king, rdunlap, Sergey Senozhatsky, Steve French

On Mon, Mar 22, 2021 at 02:57:18PM +0100, Christoph Hellwig wrote:
> On Mon, Mar 22, 2021 at 06:03:21PM +0900, Sergey Senozhatsky wrote:
> > On (21/03/22 08:15), Matthew Wilcox wrote:
> > > 
> > > What's the scenario for which your allocator performs better than slub
> > > 
> > 
> > IIRC request and reply buffers can be up to 4M in size. So this stuff
> > just allocates a number of fat buffers and keeps them around so that
> > it doesn't have to vmalloc(4M) for every request and every response.
> 
> Do we have any data suggesting it is faster than vmalloc?

Oh, I have no trouble believing it's faster than vmalloc.  Here's
the fast(!) path that always has memory available, never does retries.
I'm calling out the things I perceive as expensive on the right hand side.
Also, I'm taking the 4MB size as the example.

vmalloc()
  __vmalloc_node()
    __vmalloc_node_range()
      __get_vm_area_node()
				[allocates vm_struct]
	alloc_vmap_area()
				[allocates vmap_area]
				[takes free_vmap_area_lock]
	  __alloc_vmap_area()
	    find_vmap_lowest_match
				[walks free_vmap_area_root]
				[takes vmap_area_lock]
      __vmalloc_area_node()
				... array_size is 8KiB, we call __vmalloc_node
	__vmalloc_node
				[everything we did above, all over again,
				 two more allocations, two more lock acquire]
	alloc_pages_node(), 1024 times
	vmap_pages_range_noflush()
	  vmap_range_noflush()
				[allocate at least two pages for PTEs]

There's definitely some low handling fruit here.  __vmalloc_area_node()
should probably call kvmalloc_node() instead of __vmalloc_node() for
table sizes > 4KiB.  But a lot of this is inherent to how vmalloc works,
and we need to put a cache in front of it.  Just not this one.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 3/5] cifsd: add file operations
  2021-03-22  9:03         ` Sergey Senozhatsky
  2021-03-22 13:02           ` Matthew Wilcox
  2021-03-22 13:57           ` Christoph Hellwig
@ 2021-03-22 17:09           ` Matthew Wilcox
  2021-03-23  0:05             ` Sergey Senozhatsky
  2 siblings, 1 reply; 31+ messages in thread
From: Matthew Wilcox @ 2021-03-22 17:09 UTC (permalink / raw)
  To: Sergey Senozhatsky
  Cc: Namjae Jeon, linux-fsdevel, linux-kernel, linux-cifs,
	linux-cifsd-devel, smfrench, hyc.lee, viro, hch, hch,
	ronniesahlberg, aurelien.aptel, aaptel, sandeen, dan.carpenter,
	colin.king, rdunlap, Sergey Senozhatsky, Steve French

On Mon, Mar 22, 2021 at 06:03:21PM +0900, Sergey Senozhatsky wrote:
> On (21/03/22 08:15), Matthew Wilcox wrote:
> > 
> > What's the scenario for which your allocator performs better than slub
> > 
> 
> IIRC request and reply buffers can be up to 4M in size. So this stuff
> just allocates a number of fat buffers and keeps them around so that
> it doesn't have to vmalloc(4M) for every request and every response.

Hang on a minute, this speaks to a deeper design problem.  If we're doing
a 'request' or 'reply' that is that large, the I/O should be coming from
or going to the page cache.  If it goes via a 4MB virtually contiguous
buffer first, that's a memcpy that could/should be avoided.

But now i'm looking for how ksmbd_find_buffer() is used, and it isn't.
So it looks like someone came to the same conclusion I did, but forgot
to delete the wm code.

That said, there are plenty of opportunities to optimise the vmalloc code,
and that's worth pursuing.  And here's the receive path which contains
the memcpy that should be avoided (ok, it's actually the call to ->read;
we shouldn't be reading in the entire 4MB but rather the header):

+		conn->request_buf = ksmbd_alloc_request(size);
+		if (!conn->request_buf)
+			continue;
+
+		memcpy(conn->request_buf, hdr_buf, sizeof(hdr_buf));
+		if (!ksmbd_smb_request(conn))
+			break;
+
+		/*
+		 * We already read 4 bytes to find out PDU size, now
+		 * read in PDU
+		 */
+		size = t->ops->read(t, conn->request_buf + 4, pdu_size);


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 1/5] cifsd: add server handler and tranport layers
       [not found]     ` <20210322051344.1706-2-namjae.jeon@samsung.com>
@ 2021-03-22 22:18       ` Matthew Wilcox
  2021-03-23  3:01         ` Namjae Jeon
  0 siblings, 1 reply; 31+ messages in thread
From: Matthew Wilcox @ 2021-03-22 22:18 UTC (permalink / raw)
  To: Namjae Jeon
  Cc: linux-fsdevel, linux-kernel, linux-cifs, linux-cifsd-devel,
	smfrench, senozhatsky, hyc.lee, viro, hch, hch, ronniesahlberg,
	aurelien.aptel, aaptel, sandeen, dan.carpenter, colin.king,
	rdunlap, Sergey Senozhatsky, Steve French

On Mon, Mar 22, 2021 at 02:13:40PM +0900, Namjae Jeon wrote:
> +#define RESPONSE_BUF(w)		((void *)(w)->response_buf)
> +#define REQUEST_BUF(w)		((void *)(w)->request_buf)

Why do you do this obfuscation?

> +#define RESPONSE_BUF_NEXT(w)	\
> +	((void *)((w)->response_buf + (w)->next_smb2_rsp_hdr_off))
> +#define REQUEST_BUF_NEXT(w)	\
> +	((void *)((w)->request_buf + (w)->next_smb2_rcv_hdr_off))

These obfuscations aren't even used; delete them

> +#define RESPONSE_SZ(w)		((w)->response_sz)
> +
> +#define INIT_AUX_PAYLOAD(w)	((w)->aux_payload_buf = NULL)
> +#define HAS_AUX_PAYLOAD(w)	((w)->aux_payload_sz != 0)

I mean, do you really find it clearer to write:

	if (HAS_AUX_PAYLOAD(work))
than
	if (work->aux_payload_sz)

The unobfuscated version is actually shorter!


^ permalink raw reply	[flat|nested] 31+ messages in thread

* RE: [PATCH 2/5] cifsd: add server-side procedures for SMB3
  2021-03-22  6:47       ` [PATCH 2/5] cifsd: add server-side procedures for SMB3 Dan Carpenter
  2021-03-22  6:50         ` Christoph Hellwig
@ 2021-03-22 23:17         ` Namjae Jeon
  2021-03-23  7:19           ` Dan Carpenter
  1 sibling, 1 reply; 31+ messages in thread
From: Namjae Jeon @ 2021-03-22 23:17 UTC (permalink / raw)
  To: 'Dan Carpenter'
  Cc: linux-fsdevel, linux-kernel, linux-cifs, linux-cifsd-devel,
	smfrench, senozhatsky, hyc.lee, viro, hch, hch, ronniesahlberg,
	aurelien.aptel, aaptel, sandeen, colin.king, rdunlap,
	'Sergey Senozhatsky', 'Steve French'


> On Mon, Mar 22, 2021 at 02:13:41PM +0900, Namjae Jeon wrote:
> > +static unsigned char
> > +asn1_octet_decode(struct asn1_ctx *ctx, unsigned char *ch) {
> > +	if (ctx->pointer >= ctx->end) {
> > +		ctx->error = ASN1_ERR_DEC_EMPTY;
> > +		return 0;
> > +	}
> > +	*ch = *(ctx->pointer)++;
> > +	return 1;
> > +}
> 
> 
> Make this bool.
Okay.
> 
> > +
> > +static unsigned char
> > +asn1_tag_decode(struct asn1_ctx *ctx, unsigned int *tag) {
> > +	unsigned char ch;
> > +
> > +	*tag = 0;
> > +
> > +	do {
> > +		if (!asn1_octet_decode(ctx, &ch))
> > +			return 0;
> > +		*tag <<= 7;
> > +		*tag |= ch & 0x7F;
> > +	} while ((ch & 0x80) == 0x80);
> > +	return 1;
> > +}
> 
> Bool.
Okay.
> 
> > +
> > +static unsigned char
> > +asn1_id_decode(struct asn1_ctx *ctx,
> > +	       unsigned int *cls, unsigned int *con, unsigned int *tag) {
> > +	unsigned char ch;
> > +
> > +	if (!asn1_octet_decode(ctx, &ch))
> > +		return 0;
> > +
> > +	*cls = (ch & 0xC0) >> 6;
> > +	*con = (ch & 0x20) >> 5;
> > +	*tag = (ch & 0x1F);
> > +
> > +	if (*tag == 0x1F) {
> > +		if (!asn1_tag_decode(ctx, tag))
> > +			return 0;
> > +	}
> > +	return 1;
> > +}
> 
> 
> Same.
Okay.
> 
> > +
> > +static unsigned char
> > +asn1_length_decode(struct asn1_ctx *ctx, unsigned int *def, unsigned
> > +int *len) {
> > +	unsigned char ch, cnt;
> > +
> > +	if (!asn1_octet_decode(ctx, &ch))
> > +		return 0;
> > +
> > +	if (ch == 0x80)
> > +		*def = 0;
> > +	else {
> > +		*def = 1;
> > +
> > +		if (ch < 0x80)
> > +			*len = ch;
> > +		else {
> > +			cnt = (unsigned char) (ch & 0x7F);
> > +			*len = 0;
> > +
> > +			while (cnt > 0) {
> > +				if (!asn1_octet_decode(ctx, &ch))
> > +					return 0;
> > +				*len <<= 8;
> > +				*len |= ch;
> > +				cnt--;
> > +			}
> > +		}
> > +	}
> > +
> > +	/* don't trust len bigger than ctx buffer */
> > +	if (*len > ctx->end - ctx->pointer)
> > +		return 0;
> > +
> > +	return 1;
> > +}
> 
> 
> Same etc for all.
Okay.
> 
> > +
> > +static unsigned char
> > +asn1_header_decode(struct asn1_ctx *ctx,
> > +		   unsigned char **eoc,
> > +		   unsigned int *cls, unsigned int *con, unsigned int *tag) {
> > +	unsigned int def = 0;
> > +	unsigned int len = 0;
> > +
> > +	if (!asn1_id_decode(ctx, cls, con, tag))
> > +		return 0;
> > +
> > +	if (!asn1_length_decode(ctx, &def, &len))
> > +		return 0;
> > +
> > +	/* primitive shall be definite, indefinite shall be constructed */
> > +	if (*con == ASN1_PRI && !def)
> > +		return 0;
> > +
> > +	if (def)
> > +		*eoc = ctx->pointer + len;
> > +	else
> > +		*eoc = NULL;
> > +	return 1;
> > +}
> > +
> > +static unsigned char
> > +asn1_eoc_decode(struct asn1_ctx *ctx, unsigned char *eoc) {
> > +	unsigned char ch;
> > +
> > +	if (!eoc) {
> > +		if (!asn1_octet_decode(ctx, &ch))
> > +			return 0;
> > +
> > +		if (ch != 0x00) {
> > +			ctx->error = ASN1_ERR_DEC_EOC_MISMATCH;
> > +			return 0;
> > +		}
> > +
> > +		if (!asn1_octet_decode(ctx, &ch))
> > +			return 0;
> > +
> > +		if (ch != 0x00) {
> > +			ctx->error = ASN1_ERR_DEC_EOC_MISMATCH;
> > +			return 0;
> > +		}
> > +	} else {
> > +		if (ctx->pointer != eoc) {
> > +			ctx->error = ASN1_ERR_DEC_LENGTH_MISMATCH;
> > +			return 0;
> > +		}
> > +	}
> > +	return 1;
> > +}
> > +
> > +static unsigned char
> > +asn1_subid_decode(struct asn1_ctx *ctx, unsigned long *subid) {
> > +	unsigned char ch;
> > +
> > +	*subid = 0;
> > +
> > +	do {
> > +		if (!asn1_octet_decode(ctx, &ch))
> > +			return 0;
> > +
> > +		*subid <<= 7;
> > +		*subid |= ch & 0x7F;
> > +	} while ((ch & 0x80) == 0x80);
> > +	return 1;
> > +}
> > +
> > +static int
> > +asn1_oid_decode(struct asn1_ctx *ctx,
> > +		unsigned char *eoc, unsigned long **oid, unsigned int *len) {
> > +	unsigned long subid;
> > +	unsigned int size;
> > +	unsigned long *optr;
> > +
> > +	size = eoc - ctx->pointer + 1;
> > +
> > +	/* first subid actually encodes first two subids */
> > +	if (size < 2 || size > UINT_MAX/sizeof(unsigned long))
> > +		return 0;
> > +
> > +	*oid = kmalloc(size * sizeof(unsigned long), GFP_KERNEL);
> > +	if (!*oid)
> > +		return 0;
> > +
> > +	optr = *oid;
> > +
> > +	if (!asn1_subid_decode(ctx, &subid)) {
> > +		kfree(*oid);
> > +		*oid = NULL;
> > +		return 0;
> > +	}
> > +
> > +	if (subid < 40) {
> > +		optr[0] = 0;
> > +		optr[1] = subid;
> > +	} else if (subid < 80) {
> > +		optr[0] = 1;
> > +		optr[1] = subid - 40;
> > +	} else {
> > +		optr[0] = 2;
> > +		optr[1] = subid - 80;
> > +	}
> > +
> > +	*len = 2;
> > +	optr += 2;
> > +
> > +	while (ctx->pointer < eoc) {
> > +		if (++(*len) > size) {
> > +			ctx->error = ASN1_ERR_DEC_BADVALUE;
> > +			kfree(*oid);
> > +			*oid = NULL;
> > +			return 0;
> > +		}
> > +
> > +		if (!asn1_subid_decode(ctx, optr++)) {
> > +			kfree(*oid);
> > +			*oid = NULL;
> > +			return 0;
> > +		}
> > +	}
> > +	return 1;
> > +}
> 
> Still bool.
Okay.
> 
> > +
> > +static int
> > +compare_oid(unsigned long *oid1, unsigned int oid1len,
> > +	    unsigned long *oid2, unsigned int oid2len) {
> > +	unsigned int i;
> > +
> > +	if (oid1len != oid2len)
> > +		return 0;
> > +
> > +	for (i = 0; i < oid1len; i++) {
> > +		if (oid1[i] != oid2[i])
> > +			return 0;
> > +	}
> > +	return 1;
> > +}
> 
> Call this oid_eq()?
Why not compare_oid()? This code is come from cifs.
I need clear reason to change both cifs/cifsd...

> 
> 
> > +
> > +/* BB check for endian conversion issues here */
> > +
> > +int
> > +ksmbd_decode_negTokenInit(unsigned char *security_blob, int length,
> > +		    struct ksmbd_conn *conn)
> > +{
> > +	struct asn1_ctx ctx;
> > +	unsigned char *end;
> > +	unsigned char *sequence_end;
> > +	unsigned long *oid = NULL;
> > +	unsigned int cls, con, tag, oidlen, rc, mechTokenlen;
> > +	unsigned int mech_type;
> > +
> > +	ksmbd_debug(AUTH, "Received SecBlob: length %d\n", length);
> > +
> > +	asn1_open(&ctx, security_blob, length);
> > +
> > +	/* GSSAPI header */
> > +	if (asn1_header_decode(&ctx, &end, &cls, &con, &tag) == 0) {
> > +		ksmbd_debug(AUTH, "Error decoding negTokenInit header\n");
> > +		return 0;
> > +	} else if ((cls != ASN1_APL) || (con != ASN1_CON)
> 
> No need for else after a return 0;  Surely, checkpatch complains about
> || on the following line and the extra parentheses?
> 
> 	if (asn1_header_decode(&ctx, &end, &cls, &con, &tag) == 0) {
> 		ksmbd_debug(AUTH, "Error decoding negTokenInit header\n");
> 		return false;
> 	}
> 
> 	if (cls != ASN1_APL || con != ASN1_CON || tag != ASN1_EOC) {
> 		ksmbd_debug(AUTH, "cls = %d con = %d tag = %d\n", cls, con,
> 			    tag);
> 		return false;
> 	}
> 
> > +		   || (tag != ASN1_EOC)) {
> > +		ksmbd_debug(AUTH, "cls = %d con = %d tag = %d\n", cls, con,
> > +			tag);
> > +		return 0;
> > +	}
> > +
> > +	/* Check for SPNEGO OID -- remember to free obj->oid */
> > +	rc = asn1_header_decode(&ctx, &end, &cls, &con, &tag);
> > +	if (rc) {
> 
> This code confused the me at first.  I've always assumed "rc" stands for "return code" but
> asn1_header_decode() doesn't return error codes, it returns true false.  Alway do failure handling,
> instead of success handling.  That way when you're reading the code you can just read the code
> indented one tab to see what it does and the code indented two tabs to see how the error handling
> works.
> 
> Good:
> 
> 	frob();
> 	if (fail)
> 		clean up();
> 	frob();
> 	if (fail)
> 		clean up();
> 
> Bad:
> 	frob();
> 	if (success)
> 		frob();
> 		if (success)
> 			frob();
> 			if (success)
> 				frob();
> 		else
> 			fail = 1;
> 	if (fail)
> 		clean up();
> 
> So this code confused me.  Keep the ordering consistent with cls, con, and tag.  In fact just write it
> exactly like the lines before.
Okay.
> 
> 	if (!asn1_header_decode(&ctx, &end, &cls, &con, &tag)) {
> 		ksmbd_debug(AUTH, "Error decoding negTokenInit header\n");
> 		return false;
> 	}
> 
> 	if (cls != ASN1_UNI || con != ASN1_PRI || tag != ASN1_OJI) {
> 		ksmbd_debug(AUTH, "cls = %d con = %d tag = %d\n", cls, con,
> 			    tag);
> 		return false;
> 	}
> 
> 	if (!asn1_oid_decode(&ctx, end, &oid, &oidlen))
> 		return false;
> 	if (!oid_equiv()) {
> 		free();
> 		return false;
> 	}
> 
> 	kfree(oid); <-- I added this
> 
> Add a kfree(oid) to the success path to avoid a memory leak.
> 
> > +		if ((tag == ASN1_OJI) && (con == ASN1_PRI) &&
> > +		    (cls == ASN1_UNI)) {
> > +			rc = asn1_oid_decode(&ctx, end, &oid, &oidlen);
> > +			if (rc) {
> > +				rc = compare_oid(oid, oidlen, SPNEGO_OID,
> > +						 SPNEGO_OID_LEN);
> > +				kfree(oid);
> > +			}
> > +		} else
> > +			rc = 0;
> > +	}
> > +
> > +	/* SPNEGO OID not present or garbled -- bail out */
> > +	if (!rc) {
> > +		ksmbd_debug(AUTH, "Error decoding negTokenInit header\n");
> > +		return 0;
> > +	}
> > +
> > +	/* SPNEGO */
> > +	if (asn1_header_decode(&ctx, &end, &cls, &con, &tag) == 0) {
> > +		ksmbd_debug(AUTH, "Error decoding negTokenInit\n");
> > +		return 0;
> > +	} else if ((cls != ASN1_CTX) || (con != ASN1_CON)
> > +		   || (tag != ASN1_EOC)) {
> > +		ksmbd_debug(AUTH,
> > +			"cls = %d con = %d tag = %d end = %p (%d) exit 0\n",
> > +			cls, con, tag, end, *end);
> > +		return 0;
> > +	}
> > +
> > +	/* negTokenInit */
> > +	if (asn1_header_decode(&ctx, &end, &cls, &con, &tag) == 0) {
> > +		ksmbd_debug(AUTH, "Error decoding negTokenInit\n");
> > +		return 0;
> > +	} else if ((cls != ASN1_UNI) || (con != ASN1_CON)
> > +		   || (tag != ASN1_SEQ)) {
> > +		ksmbd_debug(AUTH,
> > +			"cls = %d con = %d tag = %d end = %p (%d) exit 1\n",
> > +			cls, con, tag, end, *end);
> > +		return 0;
> > +	}
> > +
> > +	/* sequence */
> > +	if (asn1_header_decode(&ctx, &end, &cls, &con, &tag) == 0) {
> > +		ksmbd_debug(AUTH, "Error decoding 2nd part of negTokenInit\n");
> > +		return 0;
> > +	} else if ((cls != ASN1_CTX) || (con != ASN1_CON)
> > +		   || (tag != ASN1_EOC)) {
> > +		ksmbd_debug(AUTH,
> > +			"cls = %d con = %d tag = %d end = %p (%d) exit 0\n",
> > +			cls, con, tag, end, *end);
> > +		return 0;
> > +	}
> > +
> > +	/* sequence of */
> > +	if (asn1_header_decode
> > +	    (&ctx, &sequence_end, &cls, &con, &tag) == 0) {
> 
> 
> I just ran checkpatch.pl on your patch and I see that you actually fixed all the normal checkpatch.pl
> warnings.  But I'm used to checkpatch.pl --strict code because that's the default in net and staging.
> This file has 1249 little things like this where checkpatch would have said to write it like:
> 
> 	if (!asn1_header_decode(&ctx, &sequence_end, &cls, &con, &tag)) {
> 
> total: 1 errors, 1 warnings, 1249 checks, 24501 lines checked
> 
> Once a patch has over a thousand style issues then it's too much for me to handle.  :P
Okay, I'll run it with that option:)

Thanks for your review!
> 
> regards,
> dan carpenter



^ permalink raw reply	[flat|nested] 31+ messages in thread

* RE: [PATCH 2/5] cifsd: add server-side procedures for SMB3
  2021-03-22  6:50         ` Christoph Hellwig
  2021-03-22 13:25           ` [Linux-cifsd-devel] " Stefan Metzmacher
@ 2021-03-22 23:20           ` Namjae Jeon
  1 sibling, 0 replies; 31+ messages in thread
From: Namjae Jeon @ 2021-03-22 23:20 UTC (permalink / raw)
  To: 'Christoph Hellwig'
  Cc: linux-fsdevel, linux-kernel, linux-cifs, linux-cifsd-devel,
	smfrench, senozhatsky, hyc.lee, viro, hch, ronniesahlberg,
	aurelien.aptel, aaptel, sandeen, colin.king, rdunlap,
	'Sergey Senozhatsky', 'Steve French',
	'Dan Carpenter'


> On Mon, Mar 22, 2021 at 09:47:13AM +0300, Dan Carpenter wrote:
> > On Mon, Mar 22, 2021 at 02:13:41PM +0900, Namjae Jeon wrote:
> > > +static unsigned char
> > > +asn1_octet_decode(struct asn1_ctx *ctx, unsigned char *ch) {
> > > +	if (ctx->pointer >= ctx->end) {
> > > +		ctx->error = ASN1_ERR_DEC_EMPTY;
> > > +		return 0;
> > > +	}
> > > +	*ch = *(ctx->pointer)++;
> > > +	return 1;
> > > +}
> >
> >
> > Make this bool.
> >
> 
> More importantly don't add another ANS1 parser, but use the generic one in lib/asn1_decoder.c instead.
> CIFS should also really use it.
Okay, Let me check it, cifs also...
Thanks!


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 3/5] cifsd: add file operations
  2021-03-22 17:09           ` Matthew Wilcox
@ 2021-03-23  0:05             ` Sergey Senozhatsky
  0 siblings, 0 replies; 31+ messages in thread
From: Sergey Senozhatsky @ 2021-03-23  0:05 UTC (permalink / raw)
  To: Matthew Wilcox
  Cc: Sergey Senozhatsky, Namjae Jeon, linux-fsdevel, linux-kernel,
	linux-cifs, linux-cifsd-devel, smfrench, hyc.lee, viro, hch, hch,
	ronniesahlberg, aurelien.aptel, aaptel, sandeen, dan.carpenter,
	colin.king, rdunlap, Steve French

On (21/03/22 17:09), Matthew Wilcox wrote:
> On Mon, Mar 22, 2021 at 06:03:21PM +0900, Sergey Senozhatsky wrote:
> > On (21/03/22 08:15), Matthew Wilcox wrote:
> > > 
> > > What's the scenario for which your allocator performs better than slub
> > > 
> > 
> > IIRC request and reply buffers can be up to 4M in size. So this stuff
> > just allocates a number of fat buffers and keeps them around so that
> > it doesn't have to vmalloc(4M) for every request and every response.
> 
> Hang on a minute, this speaks to a deeper design problem.  If we're doing
> a 'request' or 'reply' that is that large, the I/O should be coming from
> or going to the page cache.  If it goes via a 4MB virtually contiguous
> buffer first, that's a memcpy that could/should be avoided.

But huge vmalloc buffers are still needed. For instance, `ls -la` in
a directory with a huge number of entries.

> But now i'm looking for how ksmbd_find_buffer() is used, and it isn't.
> So it looks like someone came to the same conclusion I did, but forgot
> to delete the wm code.

Yes, I think it's disabled by default and requires some smb.conf
configuration. So I guess that wm code can be removed. Especially given
that

> That said, there are plenty of opportunities to optimise the vmalloc code,
> and that's worth pursuing.

That would be really interesting to see!

> And here's the receive path which contains
> the memcpy that should be avoided (ok, it's actually the call to ->read;
> we shouldn't be reading in the entire 4MB but rather the header):

> +		conn->request_buf = ksmbd_alloc_request(size);
> +		if (!conn->request_buf)
> +			continue;
> +
> +		memcpy(conn->request_buf, hdr_buf, sizeof(hdr_buf));
> +		if (!ksmbd_smb_request(conn))
> +			break;
> +
> +		/*
> +		 * We already read 4 bytes to find out PDU size, now
> +		 * read in PDU
> +		 */
> +		size = t->ops->read(t, conn->request_buf + 4, pdu_size);


// A side note, it seems that the maximum read/write/trans buffer size that
// windows supports is 8MB, not 4MB.

^ permalink raw reply	[flat|nested] 31+ messages in thread

* RE: [PATCH 3/5] cifsd: add file operations
  2021-03-22  6:55       ` Al Viro
@ 2021-03-23  0:12         ` Namjae Jeon
  0 siblings, 0 replies; 31+ messages in thread
From: Namjae Jeon @ 2021-03-23  0:12 UTC (permalink / raw)
  To: 'Al Viro'
  Cc: linux-fsdevel, linux-kernel, linux-cifs, linux-cifsd-devel,
	smfrench, senozhatsky, hyc.lee, hch, hch, ronniesahlberg,
	aurelien.aptel, aaptel, sandeen, dan.carpenter, colin.king,
	rdunlap, 'Sergey Senozhatsky', 'Steve French'

> On Mon, Mar 22, 2021 at 02:13:42PM +0900, Namjae Jeon wrote:
> > This adds file operations and buffer pool for cifsd.
> 
> Some random notes:
> 
> > +static void rollback_path_modification(char *filename) {
> > +	if (filename) {
> > +		filename--;
> > +		*filename = '/';
> What an odd way to spell filename[-1] = '/';...
Okay. Will fix.
> 
> > +int ksmbd_vfs_inode_permission(struct dentry *dentry, int acc_mode,
> > +bool delete) {
> 
> > +	if (delete) {
> > +		struct dentry *parent;
> > +
> > +		parent = dget_parent(dentry);
> > +		if (!parent)
> > +			return -EINVAL;
> > +
> > +		if (inode_permission(&init_user_ns, d_inode(parent), MAY_EXEC | MAY_WRITE)) {
> > +			dput(parent);
> > +			return -EACCES;
> > +		}
> > +		dput(parent);
> 
> Who's to guarantee that parent is stable?  IOW, by the time of that
> inode_permission() call dentry might very well not be a child of that thing...
Okay, Will fix.
> 
> > +	parent = dget_parent(dentry);
> > +	if (!parent)
> > +		return 0;
> > +
> > +	if (!inode_permission(&init_user_ns, d_inode(parent), MAY_EXEC | MAY_WRITE))
> > +		*daccess |= FILE_DELETE_LE;
> 
> Ditto.
Okay.
> 
> > +int ksmbd_vfs_mkdir(struct ksmbd_work *work,
> > +		    const char *name,
> > +		    umode_t mode)
> 
> 
> > +	err = vfs_mkdir(&init_user_ns, d_inode(path.dentry), dentry, mode);
> > +	if (!err) {
> > +		ksmbd_vfs_inherit_owner(work, d_inode(path.dentry),
> > +			d_inode(dentry));
> 
> ->mkdir() might very well return success, with dentry left unhashed negative.
> Look at the callers of vfs_mkdir() to see how it should be handled.
Okay, Will fix.
> 
> > +static int check_lock_range(struct file *filp,
> > +			    loff_t start,
> > +			    loff_t end,
> > +			    unsigned char type)
> > +{
> > +	struct file_lock *flock;
> > +	struct file_lock_context *ctx = file_inode(filp)->i_flctx;
> > +	int error = 0;
> > +
> > +	if (!ctx || list_empty_careful(&ctx->flc_posix))
> > +		return 0;
> > +
> > +	spin_lock(&ctx->flc_lock);
> > +	list_for_each_entry(flock, &ctx->flc_posix, fl_list) {
> > +		/* check conflict locks */
> > +		if (flock->fl_end >= start && end >= flock->fl_start) {
> > +			if (flock->fl_type == F_RDLCK) {
> > +				if (type == WRITE) {
> > +					ksmbd_err("not allow write by shared lock\n");
> > +					error = 1;
> > +					goto out;
> > +				}
> > +			} else if (flock->fl_type == F_WRLCK) {
> > +				/* check owner in lock */
> > +				if (flock->fl_file != filp) {
> > +					error = 1;
> > +					ksmbd_err("not allow rw access by exclusive lock from other
> opens\n");
> > +					goto out;
> > +				}
> > +			}
> > +		}
> > +	}
> > +out:
> > +	spin_unlock(&ctx->flc_lock);
> > +	return error;
> > +}
> 
> WTF is that doing in smbd?
This code was added to pass the smb2 lock test of samba's smbtorture.
Will fix it.
> 
> > +	filp = fp->filp;
> > +	inode = d_inode(filp->f_path.dentry);
> 
> That should be file_inode().  Try it on overlayfs, watch it do interesting things...
Okay.
> 
> > +	nbytes = kernel_read(filp, rbuf, count, pos);
> > +	if (nbytes < 0) {
> > +		name = d_path(&filp->f_path, namebuf, sizeof(namebuf));
> > +		if (IS_ERR(name))
> > +			name = "(error)";
> > +		ksmbd_err("smb read failed for (%s), err = %zd\n",
> > +				name, nbytes);
> 
> Do you really want the full pathname here?  For (presumably) spew into syslog?
No, Will fix.
> 
> > +int ksmbd_vfs_remove_file(struct ksmbd_work *work, char *name) {
> > +	struct path parent;
> > +	struct dentry *dir, *dentry;
> > +	char *last;
> > +	int err = -ENOENT;
> > +
> > +	last = extract_last_component(name);
> > +	if (!last)
> > +		return -ENOENT;
> 
> Yeccchhh...
I guess I change it err instead of -ENOENT.

> 
> > +	if (ksmbd_override_fsids(work))
> > +		return -ENOMEM;
> > +
> > +	err = kern_path(name, LOOKUP_FOLLOW | LOOKUP_DIRECTORY, &parent);
> > +	if (err) {
> > +		ksmbd_debug(VFS, "can't get %s, err %d\n", name, err);
> > +		ksmbd_revert_fsids(work);
> > +		rollback_path_modification(last);
> > +		return err;
> > +	}
> > +
> > +	dir = parent.dentry;
> > +	if (!d_inode(dir))
> > +		goto out;
> 
> Really?  When does that happen?
Will fix.
> 
> > +static int __ksmbd_vfs_rename(struct ksmbd_work *work,
> > +			      struct dentry *src_dent_parent,
> > +			      struct dentry *src_dent,
> > +			      struct dentry *dst_dent_parent,
> > +			      struct dentry *trap_dent,
> > +			      char *dst_name)
> > +{
> > +	struct dentry *dst_dent;
> > +	int err;
> > +
> > +	spin_lock(&src_dent->d_lock);
> > +	list_for_each_entry(dst_dent, &src_dent->d_subdirs, d_child) {
> > +		struct ksmbd_file *child_fp;
> > +
> > +		if (d_really_is_negative(dst_dent))
> > +			continue;
> > +
> > +		child_fp = ksmbd_lookup_fd_inode(d_inode(dst_dent));
> > +		if (child_fp) {
> > +			spin_unlock(&src_dent->d_lock);
> > +			ksmbd_debug(VFS, "Forbid rename, sub file/dir is in use\n");
> > +			return -EACCES;
> > +		}
> > +	}
> > +	spin_unlock(&src_dent->d_lock);
> 
> Hard NAK right there.  That thing has no business poking at that level.
> And I'm pretty certain that it's racy as hell.
Okay. It was same reason(smbtorture test), will fix it also.

Thanks for your review!


^ permalink raw reply	[flat|nested] 31+ messages in thread

* RE: [PATCH 1/5] cifsd: add server handler and tranport layers
  2021-03-22 22:18       ` [PATCH 1/5] cifsd: add server handler and tranport layers Matthew Wilcox
@ 2021-03-23  3:01         ` Namjae Jeon
  2021-03-23  3:12           ` Matthew Wilcox
  0 siblings, 1 reply; 31+ messages in thread
From: Namjae Jeon @ 2021-03-23  3:01 UTC (permalink / raw)
  To: 'Matthew Wilcox'
  Cc: linux-fsdevel, linux-kernel, linux-cifs, linux-cifsd-devel,
	smfrench, senozhatsky, hyc.lee, viro, hch, hch, ronniesahlberg,
	aurelien.aptel, aaptel, sandeen, dan.carpenter, colin.king,
	rdunlap, 'Sergey Senozhatsky', 'Steve French'

> On Mon, Mar 22, 2021 at 02:13:40PM +0900, Namjae Jeon wrote:
> > +#define RESPONSE_BUF(w)		((void *)(w)->response_buf)
> > +#define REQUEST_BUF(w)		((void *)(w)->request_buf)
> 
> Why do you do this obfuscation?
I don't remember exactly, but back then, It looked easier...
> 
> > +#define RESPONSE_BUF_NEXT(w)	\
> > +	((void *)((w)->response_buf + (w)->next_smb2_rsp_hdr_off))
> > +#define REQUEST_BUF_NEXT(w)	\
> > +	((void *)((w)->request_buf + (w)->next_smb2_rcv_hdr_off))
> 
> These obfuscations aren't even used; delete them
They are used in many place.
./smb2pdu.c:            *rsp = RESPONSE_BUF_NEXT(work);
./smb2pdu.c:            err_rsp = RESPONSE_BUF_NEXT(work);
./smb2pdu.c:            rsp_hdr = RESPONSE_BUF_NEXT(work);
./smb2pdu.c:    struct smb2_hdr *hdr = RESPONSE_BUF_NEXT(work);
./smb2pdu.c:    struct smb2_hdr *rsp = RESPONSE_BUF_NEXT(work);
./smb2pdu.c:    rsp_hdr = RESPONSE_BUF_NEXT(work);
./smb2pdu.c:            rsp = RESPONSE_BUF_NEXT(work);
./smb2pdu.c:            rsp = RESPONSE_BUF_NEXT(work);
./smb2pdu.c:            hdr = RESPONSE_BUF_NEXT(work);
./smb2pdu.c:            hdr = RESPONSE_BUF_NEXT(work);
./smb2pdu.c:            rsp = RESPONSE_BUF_NEXT(work);

./smb2pdu.c:            *req = REQUEST_BUF_NEXT(work);
./smb2pdu.c:            rcv_hdr = REQUEST_BUF_NEXT(work);
./smb2pdu.c:    struct smb2_hdr *req_hdr = REQUEST_BUF_NEXT(work);
./smb2pdu.c:    struct smb2_hdr *req = REQUEST_BUF_NEXT(work);
./smb2pdu.c:    rcv_hdr = REQUEST_BUF_NEXT(work);
./smb2pdu.c:    hdr = REQUEST_BUF_NEXT(work);
./smb2pdu.c:            req = REQUEST_BUF_NEXT(work);
./smb2pdu.c:            req = REQUEST_BUF_NEXT(work);
./smb2pdu.c:            hdr = REQUEST_BUF_NEXT(work);
./smb2pdu.c:    req_hdr = REQUEST_BUF_NEXT(work);
./smb2pdu.c:            hdr = REQUEST_BUF_NEXT(work);
./smb2pdu.c:    req_hdr = REQUEST_BUF_NEXT(work);
> 
> > +#define RESPONSE_SZ(w)		((w)->response_sz)
> > +
> > +#define INIT_AUX_PAYLOAD(w)	((w)->aux_payload_buf = NULL)
> > +#define HAS_AUX_PAYLOAD(w)	((w)->aux_payload_sz != 0)
> 
> I mean, do you really find it clearer to write:
> 
> 	if (HAS_AUX_PAYLOAD(work))
> than
> 	if (work->aux_payload_sz)
> 
> The unobfuscated version is actually shorter!
Yep, looks better, Will fix it.

Thanks for your review!



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 1/5] cifsd: add server handler and tranport layers
  2021-03-23  3:01         ` Namjae Jeon
@ 2021-03-23  3:12           ` Matthew Wilcox
  2021-03-23  3:16             ` Namjae Jeon
  0 siblings, 1 reply; 31+ messages in thread
From: Matthew Wilcox @ 2021-03-23  3:12 UTC (permalink / raw)
  To: Namjae Jeon
  Cc: linux-fsdevel, linux-kernel, linux-cifs, linux-cifsd-devel,
	smfrench, senozhatsky, hyc.lee, viro, hch, hch, ronniesahlberg,
	aurelien.aptel, aaptel, sandeen, dan.carpenter, colin.king,
	rdunlap, 'Sergey Senozhatsky', 'Steve French'

On Tue, Mar 23, 2021 at 12:01:22PM +0900, Namjae Jeon wrote:
> > On Mon, Mar 22, 2021 at 02:13:40PM +0900, Namjae Jeon wrote:
> > > +#define RESPONSE_BUF(w)		((void *)(w)->response_buf)
> > > +#define REQUEST_BUF(w)		((void *)(w)->request_buf)
> > 
> > Why do you do this obfuscation?
> I don't remember exactly, but back then, It looked easier...
> > 
> > > +#define RESPONSE_BUF_NEXT(w)	\
> > > +	((void *)((w)->response_buf + (w)->next_smb2_rsp_hdr_off))
> > > +#define REQUEST_BUF_NEXT(w)	\
> > > +	((void *)((w)->request_buf + (w)->next_smb2_rcv_hdr_off))
> > 
> > These obfuscations aren't even used; delete them
> They are used in many place.

Oh, argh.  patch 2/5 was too big, so it didn't make it into the mailing
list archive I was using to try to review this series.  Please break it
up into smaller pieces for next time!


^ permalink raw reply	[flat|nested] 31+ messages in thread

* RE: [PATCH 1/5] cifsd: add server handler and tranport layers
  2021-03-23  3:12           ` Matthew Wilcox
@ 2021-03-23  3:16             ` Namjae Jeon
  0 siblings, 0 replies; 31+ messages in thread
From: Namjae Jeon @ 2021-03-23  3:16 UTC (permalink / raw)
  To: 'Matthew Wilcox'
  Cc: linux-fsdevel, linux-kernel, linux-cifs, linux-cifsd-devel,
	smfrench, senozhatsky, hyc.lee, viro, hch, hch, ronniesahlberg,
	aurelien.aptel, aaptel, sandeen, dan.carpenter, colin.king,
	rdunlap, 'Sergey Senozhatsky', 'Steve French'

> On Tue, Mar 23, 2021 at 12:01:22PM +0900, Namjae Jeon wrote:
> > > On Mon, Mar 22, 2021 at 02:13:40PM +0900, Namjae Jeon wrote:
> > > > +#define RESPONSE_BUF(w)		((void *)(w)->response_buf)
> > > > +#define REQUEST_BUF(w)		((void *)(w)->request_buf)
> > >
> > > Why do you do this obfuscation?
> > I don't remember exactly, but back then, It looked easier...
> > >
> > > > +#define RESPONSE_BUF_NEXT(w)	\
> > > > +	((void *)((w)->response_buf + (w)->next_smb2_rsp_hdr_off))
> > > > +#define REQUEST_BUF_NEXT(w)	\
> > > > +	((void *)((w)->request_buf + (w)->next_smb2_rcv_hdr_off))
> > >
> > > These obfuscations aren't even used; delete them
> > They are used in many place.
> 
> Oh, argh.  patch 2/5 was too big, so it didn't make it into the mailing list archive I was using to
> try to review this series.  Please break it up into smaller pieces for next time!
Okay:)
Thanks!


^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/5] cifsd: add server-side procedures for SMB3
  2021-03-22 23:17         ` Namjae Jeon
@ 2021-03-23  7:19           ` Dan Carpenter
  2021-03-25  5:25             ` Sebastian Gottschall
  0 siblings, 1 reply; 31+ messages in thread
From: Dan Carpenter @ 2021-03-23  7:19 UTC (permalink / raw)
  To: Namjae Jeon
  Cc: linux-fsdevel, linux-kernel, linux-cifs, linux-cifsd-devel,
	smfrench, senozhatsky, hyc.lee, viro, hch, hch, ronniesahlberg,
	aurelien.aptel, aaptel, sandeen, colin.king, rdunlap,
	'Sergey Senozhatsky', 'Steve French'

On Tue, Mar 23, 2021 at 08:17:47AM +0900, Namjae Jeon wrote:
> > > +
> > > +static int
> > > +compare_oid(unsigned long *oid1, unsigned int oid1len,
> > > +	    unsigned long *oid2, unsigned int oid2len) {
> > > +	unsigned int i;
> > > +
> > > +	if (oid1len != oid2len)
> > > +		return 0;
> > > +
> > > +	for (i = 0; i < oid1len; i++) {
> > > +		if (oid1[i] != oid2[i])
> > > +			return 0;
> > > +	}
> > > +	return 1;
> > > +}
> > 
> > Call this oid_eq()?
> Why not compare_oid()? This code is come from cifs.
> I need clear reason to change both cifs/cifsd...
> 

Boolean functions should tell you what they are testing in the name.
Without any context you can't know what if (compare_oid(one, two)) {
means, but if (oid_equal(one, two)) { is readable.

regards,
dan carpenter



^ permalink raw reply	[flat|nested] 31+ messages in thread

* Re: [PATCH 2/5] cifsd: add server-side procedures for SMB3
  2021-03-23  7:19           ` Dan Carpenter
@ 2021-03-25  5:25             ` Sebastian Gottschall
  0 siblings, 0 replies; 31+ messages in thread
From: Sebastian Gottschall @ 2021-03-25  5:25 UTC (permalink / raw)
  To: Dan Carpenter, Namjae Jeon
  Cc: linux-fsdevel, linux-kernel, linux-cifs, linux-cifsd-devel,
	smfrench, senozhatsky, hyc.lee, viro, hch, hch, ronniesahlberg,
	aurelien.aptel, aaptel, sandeen, colin.king, rdunlap,
	'Sergey Senozhatsky', 'Steve French'


Am 23.03.2021 um 08:19 schrieb Dan Carpenter:
> On Tue, Mar 23, 2021 at 08:17:47AM +0900, Namjae Jeon wrote:
>>>> +
>>>> +static int
>>>> +compare_oid(unsigned long *oid1, unsigned int oid1len,
>>>> +	    unsigned long *oid2, unsigned int oid2len) {
>>>> +	unsigned int i;
>>>> +
>>>> +	if (oid1len != oid2len)
>>>> +		return 0;
>>>> +
>>>> +	for (i = 0; i < oid1len; i++) {
>>>> +		if (oid1[i] != oid2[i])
>>>> +			return 0;
>>>> +	}
>>>> +	return 1;
>>>> +}
>>> Call this oid_eq()?
>> Why not compare_oid()? This code is come from cifs.
>> I need clear reason to change both cifs/cifsd...
>>
> Boolean functions should tell you what they are testing in the name.
> Without any context you can't know what if (compare_oid(one, two)) {
> means, but if (oid_equal(one, two)) { is readable.
>
> regards,
> dan carpenter
ahm just a pointless comment. but
return !memcmp(oid1,oid2, sizeof(long*)*oid1len);
looks much more efficient than this "for" loop
>
>
>

^ permalink raw reply	[flat|nested] 31+ messages in thread

end of thread, other threads:[~2021-03-25  5:28 UTC | newest]

Thread overview: 31+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20210322052203epcas1p21fe2d04c4df5396c466c38f4d57d8bb8@epcas1p2.samsung.com>
2021-03-22  5:13 ` [PATCH 0/5] cifsd: introduce new SMB3 kernel server Namjae Jeon
     [not found]   ` <CGME20210322052207epcas1p3f0a5bdfd2c994a849a67b465479d0721@epcas1p3.samsung.com>
2021-03-22  5:13     ` [PATCH 3/5] cifsd: add file operations Namjae Jeon
2021-03-22  6:55       ` Al Viro
2021-03-23  0:12         ` Namjae Jeon
2021-03-22  7:02       ` Al Viro
2021-03-22  9:26         ` Sergey Senozhatsky
2021-03-22  7:04       ` Dan Carpenter
2021-03-22  9:39         ` Sergey Senozhatsky
2021-03-22  8:15       ` Matthew Wilcox
2021-03-22  9:03         ` Sergey Senozhatsky
2021-03-22 13:02           ` Matthew Wilcox
2021-03-22 13:57           ` Christoph Hellwig
2021-03-22 14:40             ` Matthew Wilcox
2021-03-22 17:09           ` Matthew Wilcox
2021-03-23  0:05             ` Sergey Senozhatsky
     [not found]   ` <CGME20210322052208epcas1p430b2e93761d5194844c533c61d43242d@epcas1p4.samsung.com>
2021-03-22  5:13     ` [PATCH 4/5] cifsd: add Kconfig and Makefile Namjae Jeon
     [not found]   ` <CGME20210322052209epcas1p377f1542bcc9ec50219d2e57aa92d944b@epcas1p3.samsung.com>
2021-03-22  5:13     ` [PATCH 5/5] MAINTAINERS: add cifsd kernel server Namjae Jeon
     [not found]   ` <CGME20210322052206epcas1p438f15851216f07540537c5547a0a2c02@epcas1p4.samsung.com>
     [not found]     ` <20210322051344.1706-3-namjae.jeon@samsung.com>
2021-03-22  6:47       ` [PATCH 2/5] cifsd: add server-side procedures for SMB3 Dan Carpenter
2021-03-22  6:50         ` Christoph Hellwig
2021-03-22 13:25           ` [Linux-cifsd-devel] " Stefan Metzmacher
2021-03-22 23:20           ` Namjae Jeon
2021-03-22 23:17         ` Namjae Jeon
2021-03-23  7:19           ` Dan Carpenter
2021-03-25  5:25             ` Sebastian Gottschall
2021-03-22  8:34       ` Matthew Wilcox
2021-03-22 10:27         ` Sergey Senozhatsky
2021-03-22 13:12           ` Matthew Wilcox
     [not found]   ` <CGME20210322052204epcas1p1382cadbfe958d156c0ad9f7fcb8532b7@epcas1p1.samsung.com>
     [not found]     ` <20210322051344.1706-2-namjae.jeon@samsung.com>
2021-03-22 22:18       ` [PATCH 1/5] cifsd: add server handler and tranport layers Matthew Wilcox
2021-03-23  3:01         ` Namjae Jeon
2021-03-23  3:12           ` Matthew Wilcox
2021-03-23  3:16             ` Namjae Jeon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).