Skip to content

Commit

Permalink
ksmbd: Fix multi-protocol negotiation
Browse files Browse the repository at this point in the history 
To negotiate either the SMB2 protocol or SMB protocol, a client must
send a SMB_COM_NEGOTIATE message containing the list of dialects it
supports, to which the server will respond with either a
SMB_COM_NEGOTIATE or a SMB2_NEGOTIATE response.

The current implementation responds with the highest common dialect,
rather than looking explicitly for "SMB 2.???" and "SMB 2.002", as
indicated in [MS-SMB2]:

  [MS-SMB2] 3.3.5.3.1:
    If the server does not implement the SMB 2.1 or 3.x dialect family,
    processing MUST continue as specified in 3.3.5.3.2.

    Otherwise, the server MUST scan the dialects provided for the dialect
    string "SMB 2.???". If the string is not present, continue to section
    3.3.5.3.2. If the string is present, the server MUST respond with an
    SMB2 NEGOTIATE Response as specified in 2.2.4.

  [MS-SMB2] 3.3.5.3.2:
    The server MUST scan the dialects provided for the dialect string "SMB
    2.002". If the string is present, the client understands SMB2, and the
    server MUST respond with an SMB2 NEGOTIATE Response.

This is an issue if a client attempts to negotiate SMB3.1.1 using
a SMB_COM_NEGOTIATE, as it will trigger the following NULL pointer
dereference:

  8<--- cut here ---
  Unable to handle kernel NULL pointer dereference at virtual address 00000000
  pgd = 1917455e
  [00000000] *pgd=00000000
  Internal error: Oops: 17 [#1] ARM
  CPU: 0 PID: 60 Comm: kworker/0:1 Not tainted 5.4.60-00027-g0518c02b5c5b #35
  Hardware name: Marvell Kirkwood (Flattened Device Tree)
  Workqueue: ksmbd-io handle_ksmbd_work
  PC is at ksmbd_gen_preauth_integrity_hash+0x24/0x190
  LR is at smb3_preauth_hash_rsp+0x50/0xa0
  pc : [<802b7044>] lr : [<802d6ac0>] psr: 40000013
  sp : bf199ed8 ip : 00000000 fp : 80d1edb0
  r10: 80a3471b r9 : 8091af16 r8 : 80d70640
  r7 : 00000072 r6 : be95e198 r5 : ca000000 r4 : b97fee00
  r3 : 00000000 r2 : 00000002 r1 : b97fea00 r0 : b97fee00
  Flags: nZcv IRQs on FIQs on Mode SVC_32 ISA ARM Segment user
  Control: 0005317f Table: 3e7f4000 DAC: 00000055
  Process kworker/0:1 (pid: 60, stack limit = 0x3dd1fdb4)
  Stack: (0xbf199ed8 to 0xbf19a000)
  9ec0: b97fee00 00000000
  9ee0: be95e198 00000072 80d70640 802d6ac0 b3da2680 b97fea00 424d53ff be95e140
  9f00: b97fee00 802bd7b0 bf10fa58 80128a78 00000000 000001c8 b6220000 bf0b7720
  9f20: be95e198 80d0c410 bf7e2a00 00000000 00000000 be95e19c 80d0c370 80123b90
  9f40: bf0b7720 be95e198 bf0b7720 bf0b7734 80d0c410 bf198000 80d0c424 80d116e0
  9f60: bf10fa58 801240c0 00000000 bf10fa40 bf1463a0 bf198000 bf0b7720 80123ed0
  9f80: bf077ee4 bf10fa58 00000000 80127f80 bf1463a0 80127e88 00000000 00000000
  9fa0: 00000000 00000000 00000000 801010d0 00000000 00000000 00000000 00000000
  9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
  9fe0: 00000000 00000000 00000000 00000000 00000013 00000000 00000000 00000000
  [<802b7044>] (ksmbd_gen_preauth_integrity_hash) from [<802d6ac0>] (smb3_preauth_hash_rsp+0x50/0xa0)
  [<802d6ac0>] (smb3_preauth_hash_rsp) from [<802bd7b0>] (handle_ksmbd_work+0x348/0x3f8)
  [<802bd7b0>] (handle_ksmbd_work) from [<80123b90>] (process_one_work+0x160/0x200)
  [<80123b90>] (process_one_work) from [<801240c0>] (worker_thread+0x1f0/0x2e4)
  [<801240c0>] (worker_thread) from [<80127f80>] (kthread+0xf8/0x10c)
  [<80127f80>] (kthread) from [<801010d0>] (ret_from_fork+0x14/0x24)
  Exception stack(0xbf199fb0 to 0xbf199ff8)
  9fa0: 00000000 00000000 00000000 00000000
  9fc0: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000
  9fe0: 00000000 00000000 00000000 00000000 00000013 00000000
  Code: e1855803 e5d13003 e1855c03 e5903094 (e1d330b0)
  ---[ end trace 8d03be3ed09e5699 ]---
  Kernel panic - not syncing: Fatal exception

smb3_preauth_hash_rsp() panics because conn->preauth_info is only allocated
when processing a SMB2 NEGOTIATE request.

Fix this by splitting the smb_protos array into two, each containing
only SMB1 and SMB2 dialects respectively.

While here, make ksmbd_negotiate_smb_dialect() static as it not
called from anywhere else.

Signed-off-by: Marios Makassikis <[email protected]>
Signed-off-by: Namjae Jeon <[email protected]>
  • Loading branch information
@namjaejeon
Marios Makassikis authored and namjaejeon committed Aug 4, 2021
1 parent 4a12b06 commit 4dd7096
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 24 deletions.
65 changes: 42 additions & 23 deletions smb_common.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ struct smb_protocol {
__u16 prot_id;
};

static struct smb_protocol smb_protos[] = {
static struct smb_protocol smb1_protos[] = {
#ifdef CONFIG_SMB_INSECURE_SERVER
{
SMB1_PROT,
Expand All @@ -53,18 +53,27 @@ static struct smb_protocol smb_protos[] = {
SMB20_PROT_ID
},
#endif
{
SMB21_PROT,
"\2SMB 2.1",
"SMB2_10",
SMB21_PROT_ID
},
{
SMB2X_PROT,
"\2SMB 2.???",
"SMB2_22",
SMB2X_PROT_ID
},
};

static struct smb_protocol smb2_protos[] = {
{
SMB2_PROT,
"\2SMB 2.002",
"SMB2_02",
SMB20_PROT_ID
},
{
SMB21_PROT,
"\2SMB 2.1",
"SMB2_10",
SMB21_PROT_ID
},
{
SMB30_PROT,
"\2SMB 3.0",
Expand Down Expand Up @@ -116,14 +125,24 @@ inline int ksmbd_max_protocol(void)

int ksmbd_lookup_protocol_idx(char *str)
{
int offt = ARRAY_SIZE(smb_protos) - 1;
int offt = ARRAY_SIZE(smb1_protos) - 1;
int len = strlen(str);

while (offt >= 0) {
if (!strncmp(str, smb_protos[offt].prot, len)) {
if (!strncmp(str, smb1_protos[offt].prot, len)) {
ksmbd_debug(SMB, "selected %s dialect idx = %d\n",
smb1_protos[offt].prot, offt);
return smb1_protos[offt].index;
}
offt--;
}

offt = ARRAY_SIZE(smb2_protos) - 1;
while (offt >= 0) {
if (!strncmp(str, smb2_protos[offt].prot, len)) {
ksmbd_debug(SMB, "selected %s dialect idx = %d\n",
smb_protos[offt].prot, offt);
return smb_protos[offt].index;
smb2_protos[offt].prot, offt);
return smb2_protos[offt].index;
}
offt--;
}
Expand Down Expand Up @@ -204,7 +223,7 @@ static int ksmbd_lookup_dialect_by_name(char *cli_dialects, __le16 byte_count)
int i, seq_num, bcount, next;
char *dialect;

for (i = ARRAY_SIZE(smb_protos) - 1; i >= 0; i--) {
for (i = ARRAY_SIZE(smb1_protos) - 1; i >= 0; i--) {
seq_num = 0;
next = 0;
dialect = cli_dialects;
Expand All @@ -213,14 +232,14 @@ static int ksmbd_lookup_dialect_by_name(char *cli_dialects, __le16 byte_count)
dialect = next_dialect(dialect, &next);
ksmbd_debug(SMB, "client requested dialect %s\n",
dialect);
if (!strcmp(dialect, smb_protos[i].name)) {
if (supported_protocol(smb_protos[i].index)) {
if (!strcmp(dialect, smb1_protos[i].name)) {
if (supported_protocol(smb1_protos[i].index)) {
ksmbd_debug(SMB,
"selected %s dialect\n",
smb_protos[i].name);
if (smb_protos[i].index == SMB1_PROT)
smb1_protos[i].name);
if (smb1_protos[i].index == SMB1_PROT)
return seq_num;
return smb_protos[i].prot_id;
return smb1_protos[i].prot_id;
}
}
seq_num++;
Expand All @@ -236,27 +255,27 @@ int ksmbd_lookup_dialect_by_id(__le16 *cli_dialects, __le16 dialects_count)
int i;
int count;

for (i = ARRAY_SIZE(smb_protos) - 1; i >= 0; i--) {
for (i = ARRAY_SIZE(smb2_protos) - 1; i >= 0; i--) {
count = le16_to_cpu(dialects_count);
while (--count >= 0) {
ksmbd_debug(SMB, "client requested dialect 0x%x\n",
le16_to_cpu(cli_dialects[count]));
if (le16_to_cpu(cli_dialects[count]) !=
smb_protos[i].prot_id)
smb2_protos[i].prot_id)
continue;

if (supported_protocol(smb_protos[i].index)) {
if (supported_protocol(smb2_protos[i].index)) {
ksmbd_debug(SMB, "selected %s dialect\n",
smb_protos[i].name);
return smb_protos[i].prot_id;
smb2_protos[i].name);
return smb2_protos[i].prot_id;
}
}
}

return BAD_PROT_ID;
}

int ksmbd_negotiate_smb_dialect(void *buf)
static int ksmbd_negotiate_smb_dialect(void *buf)
{
__le32 proto;

Expand Down
1 change: 0 additions & 1 deletion smb_common.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -501,7 +501,6 @@ bool ksmbd_smb_request(struct ksmbd_conn *conn);

int ksmbd_lookup_dialect_by_id(__le16 *cli_dialects, __le16 dialects_count);

int ksmbd_negotiate_smb_dialect(void *buf);
int ksmbd_init_smb_server(struct ksmbd_work *work);

bool ksmbd_pdu_size_has_room(unsigned int pdu);
Expand Down

0 comments on commit 4dd7096

Please sign in to comment.