| Message ID | 20210322051344.1706-1-namjae.jeon@samsung.com |
|---|---|
| Headers | show |
| Series | cifsd: introduce new SMB3 kernel server | expand |
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
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.
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.
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.
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.
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
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!
> 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
> 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!
> 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!
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!
> 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!
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
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 > > >
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