* [PATCH] cifs: fix mount on old smb servers
@ 2023-02-16 18:33 Paulo Alcantara
2023-02-16 21:08 ` ronnie sahlberg
0 siblings, 1 reply; 3+ messages in thread
From: Paulo Alcantara @ 2023-02-16 18:33 UTC (permalink / raw)
To: smfrench; +Cc: linux-cifs, Paulo Alcantara
The client was sending rfc1002 session request packet with a wrong
length field set, therefore failing to mount shares against old SMB
servers over port 139.
Fix this by calculating the correct length as specified in rfc1002.
Fixes: d7173623bf0b ("cifs: use ALIGN() and round_up() macros")
Signed-off-by: Paulo Alcantara (SUSE) <pc@manguebit.com>
---
fs/cifs/connect.c | 100 ++++++++++++++++++----------------------------
1 file changed, 38 insertions(+), 62 deletions(-)
diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
index b2a04b4e89a5..af49ae53aaf4 100644
--- a/fs/cifs/connect.c
+++ b/fs/cifs/connect.c
@@ -2843,72 +2843,48 @@ ip_rfc1001_connect(struct TCP_Server_Info *server)
* negprot - BB check reconnection in case where second
* sessinit is sent but no second negprot
*/
- struct rfc1002_session_packet *ses_init_buf;
- unsigned int req_noscope_len;
- struct smb_hdr *smb_buf;
+ struct rfc1002_session_packet req = {};
+ struct smb_hdr *smb_buf = (struct smb_hdr *)&req;
+ unsigned int len;
+
+ req.trailer.session_req.called_len = sizeof(req.trailer.session_req.called_name);
+
+ if (server->server_RFC1001_name[0] != 0)
+ rfc1002mangle(req.trailer.session_req.called_name,
+ server->server_RFC1001_name,
+ RFC1001_NAME_LEN_WITH_NULL);
+ else
+ rfc1002mangle(req.trailer.session_req.called_name,
+ DEFAULT_CIFS_CALLED_NAME,
+ RFC1001_NAME_LEN_WITH_NULL);
+
+ req.trailer.session_req.calling_len = sizeof(req.trailer.session_req.calling_name);
+
+ /* calling name ends in null (byte 16) from old smb convention */
+ if (server->workstation_RFC1001_name[0] != 0)
+ rfc1002mangle(req.trailer.session_req.calling_name,
+ server->workstation_RFC1001_name,
+ RFC1001_NAME_LEN_WITH_NULL);
+ else
+ rfc1002mangle(req.trailer.session_req.calling_name,
+ "LINUX_CIFS_CLNT",
+ RFC1001_NAME_LEN_WITH_NULL);
- ses_init_buf = kzalloc(sizeof(struct rfc1002_session_packet),
- GFP_KERNEL);
-
- if (ses_init_buf) {
- ses_init_buf->trailer.session_req.called_len = 32;
-
- if (server->server_RFC1001_name[0] != 0)
- rfc1002mangle(ses_init_buf->trailer.
- session_req.called_name,
- server->server_RFC1001_name,
- RFC1001_NAME_LEN_WITH_NULL);
- else
- rfc1002mangle(ses_init_buf->trailer.
- session_req.called_name,
- DEFAULT_CIFS_CALLED_NAME,
- RFC1001_NAME_LEN_WITH_NULL);
-
- ses_init_buf->trailer.session_req.calling_len = 32;
-
- /*
- * calling name ends in null (byte 16) from old smb
- * convention.
- */
- if (server->workstation_RFC1001_name[0] != 0)
- rfc1002mangle(ses_init_buf->trailer.
- session_req.calling_name,
- server->workstation_RFC1001_name,
- RFC1001_NAME_LEN_WITH_NULL);
- else
- rfc1002mangle(ses_init_buf->trailer.
- session_req.calling_name,
- "LINUX_CIFS_CLNT",
- RFC1001_NAME_LEN_WITH_NULL);
-
- ses_init_buf->trailer.session_req.scope1 = 0;
- ses_init_buf->trailer.session_req.scope2 = 0;
- smb_buf = (struct smb_hdr *)ses_init_buf;
-
- /* sizeof RFC1002_SESSION_REQUEST with no scopes */
- req_noscope_len = sizeof(struct rfc1002_session_packet) - 2;
+ /*
+ * As per rfc1002, @len must be the number of bytes that follows the
+ * length field of a rfc1002 session request payload.
+ */
+ len = sizeof(req) - offsetof(struct rfc1002_session_packet, trailer.session_req);
- /* == cpu_to_be32(0x81000044) */
- smb_buf->smb_buf_length =
- cpu_to_be32((RFC1002_SESSION_REQUEST << 24) | req_noscope_len);
- rc = smb_send(server, smb_buf, 0x44);
- kfree(ses_init_buf);
- /*
- * RFC1001 layer in at least one server
- * requires very short break before negprot
- * presumably because not expecting negprot
- * to follow so fast. This is a simple
- * solution that works without
- * complicating the code and causes no
- * significant slowing down on mount
- * for everyone else
- */
- usleep_range(1000, 2000);
- }
+ smb_buf->smb_buf_length = cpu_to_be32((RFC1002_SESSION_REQUEST << 24) | len);
+ rc = smb_send(server, smb_buf, len);
/*
- * else the negprot may still work without this
- * even though malloc failed
+ * RFC1001 layer in at least one server requires very short break before
+ * negprot presumably because not expecting negprot to follow so fast.
+ * This is a simple solution that works without complicating the code
+ * and causes no significant slowing down on mount for everyone else
*/
+ usleep_range(1000, 2000);
return rc;
}
--
2.39.1
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] cifs: fix mount on old smb servers
2023-02-16 18:33 [PATCH] cifs: fix mount on old smb servers Paulo Alcantara
@ 2023-02-16 21:08 ` ronnie sahlberg
2023-02-17 5:33 ` Steve French
0 siblings, 1 reply; 3+ messages in thread
From: ronnie sahlberg @ 2023-02-16 21:08 UTC (permalink / raw)
To: Paulo Alcantara; +Cc: smfrench, linux-cifs
Very nice cleanup.
Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com>
On Fri, 17 Feb 2023 at 04:44, Paulo Alcantara <pc@manguebit.com> wrote:
>
> The client was sending rfc1002 session request packet with a wrong
> length field set, therefore failing to mount shares against old SMB
> servers over port 139.
>
> Fix this by calculating the correct length as specified in rfc1002.
>
> Fixes: d7173623bf0b ("cifs: use ALIGN() and round_up() macros")
> Signed-off-by: Paulo Alcantara (SUSE) <pc@manguebit.com>
> ---
> fs/cifs/connect.c | 100 ++++++++++++++++++----------------------------
> 1 file changed, 38 insertions(+), 62 deletions(-)
>
> diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> index b2a04b4e89a5..af49ae53aaf4 100644
> --- a/fs/cifs/connect.c
> +++ b/fs/cifs/connect.c
> @@ -2843,72 +2843,48 @@ ip_rfc1001_connect(struct TCP_Server_Info *server)
> * negprot - BB check reconnection in case where second
> * sessinit is sent but no second negprot
> */
> - struct rfc1002_session_packet *ses_init_buf;
> - unsigned int req_noscope_len;
> - struct smb_hdr *smb_buf;
> + struct rfc1002_session_packet req = {};
> + struct smb_hdr *smb_buf = (struct smb_hdr *)&req;
> + unsigned int len;
> +
> + req.trailer.session_req.called_len = sizeof(req.trailer.session_req.called_name);
> +
> + if (server->server_RFC1001_name[0] != 0)
> + rfc1002mangle(req.trailer.session_req.called_name,
> + server->server_RFC1001_name,
> + RFC1001_NAME_LEN_WITH_NULL);
> + else
> + rfc1002mangle(req.trailer.session_req.called_name,
> + DEFAULT_CIFS_CALLED_NAME,
> + RFC1001_NAME_LEN_WITH_NULL);
> +
> + req.trailer.session_req.calling_len = sizeof(req.trailer.session_req.calling_name);
> +
> + /* calling name ends in null (byte 16) from old smb convention */
> + if (server->workstation_RFC1001_name[0] != 0)
> + rfc1002mangle(req.trailer.session_req.calling_name,
> + server->workstation_RFC1001_name,
> + RFC1001_NAME_LEN_WITH_NULL);
> + else
> + rfc1002mangle(req.trailer.session_req.calling_name,
> + "LINUX_CIFS_CLNT",
> + RFC1001_NAME_LEN_WITH_NULL);
>
> - ses_init_buf = kzalloc(sizeof(struct rfc1002_session_packet),
> - GFP_KERNEL);
> -
> - if (ses_init_buf) {
> - ses_init_buf->trailer.session_req.called_len = 32;
> -
> - if (server->server_RFC1001_name[0] != 0)
> - rfc1002mangle(ses_init_buf->trailer.
> - session_req.called_name,
> - server->server_RFC1001_name,
> - RFC1001_NAME_LEN_WITH_NULL);
> - else
> - rfc1002mangle(ses_init_buf->trailer.
> - session_req.called_name,
> - DEFAULT_CIFS_CALLED_NAME,
> - RFC1001_NAME_LEN_WITH_NULL);
> -
> - ses_init_buf->trailer.session_req.calling_len = 32;
> -
> - /*
> - * calling name ends in null (byte 16) from old smb
> - * convention.
> - */
> - if (server->workstation_RFC1001_name[0] != 0)
> - rfc1002mangle(ses_init_buf->trailer.
> - session_req.calling_name,
> - server->workstation_RFC1001_name,
> - RFC1001_NAME_LEN_WITH_NULL);
> - else
> - rfc1002mangle(ses_init_buf->trailer.
> - session_req.calling_name,
> - "LINUX_CIFS_CLNT",
> - RFC1001_NAME_LEN_WITH_NULL);
> -
> - ses_init_buf->trailer.session_req.scope1 = 0;
> - ses_init_buf->trailer.session_req.scope2 = 0;
> - smb_buf = (struct smb_hdr *)ses_init_buf;
> -
> - /* sizeof RFC1002_SESSION_REQUEST with no scopes */
> - req_noscope_len = sizeof(struct rfc1002_session_packet) - 2;
> + /*
> + * As per rfc1002, @len must be the number of bytes that follows the
> + * length field of a rfc1002 session request payload.
> + */
> + len = sizeof(req) - offsetof(struct rfc1002_session_packet, trailer.session_req);
>
> - /* == cpu_to_be32(0x81000044) */
> - smb_buf->smb_buf_length =
> - cpu_to_be32((RFC1002_SESSION_REQUEST << 24) | req_noscope_len);
> - rc = smb_send(server, smb_buf, 0x44);
> - kfree(ses_init_buf);
> - /*
> - * RFC1001 layer in at least one server
> - * requires very short break before negprot
> - * presumably because not expecting negprot
> - * to follow so fast. This is a simple
> - * solution that works without
> - * complicating the code and causes no
> - * significant slowing down on mount
> - * for everyone else
> - */
> - usleep_range(1000, 2000);
> - }
> + smb_buf->smb_buf_length = cpu_to_be32((RFC1002_SESSION_REQUEST << 24) | len);
> + rc = smb_send(server, smb_buf, len);
> /*
> - * else the negprot may still work without this
> - * even though malloc failed
> + * RFC1001 layer in at least one server requires very short break before
> + * negprot presumably because not expecting negprot to follow so fast.
> + * This is a simple solution that works without complicating the code
> + * and causes no significant slowing down on mount for everyone else
> */
> + usleep_range(1000, 2000);
>
> return rc;
> }
> --
> 2.39.1
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] cifs: fix mount on old smb servers
2023-02-16 21:08 ` ronnie sahlberg
@ 2023-02-17 5:33 ` Steve French
0 siblings, 0 replies; 3+ messages in thread
From: Steve French @ 2023-02-17 5:33 UTC (permalink / raw)
To: ronnie sahlberg; +Cc: Paulo Alcantara, linux-cifs
merged into cifs-2.6.git for-next
On Thu, Feb 16, 2023 at 3:08 PM ronnie sahlberg
<ronniesahlberg@gmail.com> wrote:
>
> Very nice cleanup.
>
> Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com>
>
> On Fri, 17 Feb 2023 at 04:44, Paulo Alcantara <pc@manguebit.com> wrote:
> >
> > The client was sending rfc1002 session request packet with a wrong
> > length field set, therefore failing to mount shares against old SMB
> > servers over port 139.
> >
> > Fix this by calculating the correct length as specified in rfc1002.
> >
> > Fixes: d7173623bf0b ("cifs: use ALIGN() and round_up() macros")
> > Signed-off-by: Paulo Alcantara (SUSE) <pc@manguebit.com>
> > ---
> > fs/cifs/connect.c | 100 ++++++++++++++++++----------------------------
> > 1 file changed, 38 insertions(+), 62 deletions(-)
> >
> > diff --git a/fs/cifs/connect.c b/fs/cifs/connect.c
> > index b2a04b4e89a5..af49ae53aaf4 100644
> > --- a/fs/cifs/connect.c
> > +++ b/fs/cifs/connect.c
> > @@ -2843,72 +2843,48 @@ ip_rfc1001_connect(struct TCP_Server_Info *server)
> > * negprot - BB check reconnection in case where second
> > * sessinit is sent but no second negprot
> > */
> > - struct rfc1002_session_packet *ses_init_buf;
> > - unsigned int req_noscope_len;
> > - struct smb_hdr *smb_buf;
> > + struct rfc1002_session_packet req = {};
> > + struct smb_hdr *smb_buf = (struct smb_hdr *)&req;
> > + unsigned int len;
> > +
> > + req.trailer.session_req.called_len = sizeof(req.trailer.session_req.called_name);
> > +
> > + if (server->server_RFC1001_name[0] != 0)
> > + rfc1002mangle(req.trailer.session_req.called_name,
> > + server->server_RFC1001_name,
> > + RFC1001_NAME_LEN_WITH_NULL);
> > + else
> > + rfc1002mangle(req.trailer.session_req.called_name,
> > + DEFAULT_CIFS_CALLED_NAME,
> > + RFC1001_NAME_LEN_WITH_NULL);
> > +
> > + req.trailer.session_req.calling_len = sizeof(req.trailer.session_req.calling_name);
> > +
> > + /* calling name ends in null (byte 16) from old smb convention */
> > + if (server->workstation_RFC1001_name[0] != 0)
> > + rfc1002mangle(req.trailer.session_req.calling_name,
> > + server->workstation_RFC1001_name,
> > + RFC1001_NAME_LEN_WITH_NULL);
> > + else
> > + rfc1002mangle(req.trailer.session_req.calling_name,
> > + "LINUX_CIFS_CLNT",
> > + RFC1001_NAME_LEN_WITH_NULL);
> >
> > - ses_init_buf = kzalloc(sizeof(struct rfc1002_session_packet),
> > - GFP_KERNEL);
> > -
> > - if (ses_init_buf) {
> > - ses_init_buf->trailer.session_req.called_len = 32;
> > -
> > - if (server->server_RFC1001_name[0] != 0)
> > - rfc1002mangle(ses_init_buf->trailer.
> > - session_req.called_name,
> > - server->server_RFC1001_name,
> > - RFC1001_NAME_LEN_WITH_NULL);
> > - else
> > - rfc1002mangle(ses_init_buf->trailer.
> > - session_req.called_name,
> > - DEFAULT_CIFS_CALLED_NAME,
> > - RFC1001_NAME_LEN_WITH_NULL);
> > -
> > - ses_init_buf->trailer.session_req.calling_len = 32;
> > -
> > - /*
> > - * calling name ends in null (byte 16) from old smb
> > - * convention.
> > - */
> > - if (server->workstation_RFC1001_name[0] != 0)
> > - rfc1002mangle(ses_init_buf->trailer.
> > - session_req.calling_name,
> > - server->workstation_RFC1001_name,
> > - RFC1001_NAME_LEN_WITH_NULL);
> > - else
> > - rfc1002mangle(ses_init_buf->trailer.
> > - session_req.calling_name,
> > - "LINUX_CIFS_CLNT",
> > - RFC1001_NAME_LEN_WITH_NULL);
> > -
> > - ses_init_buf->trailer.session_req.scope1 = 0;
> > - ses_init_buf->trailer.session_req.scope2 = 0;
> > - smb_buf = (struct smb_hdr *)ses_init_buf;
> > -
> > - /* sizeof RFC1002_SESSION_REQUEST with no scopes */
> > - req_noscope_len = sizeof(struct rfc1002_session_packet) - 2;
> > + /*
> > + * As per rfc1002, @len must be the number of bytes that follows the
> > + * length field of a rfc1002 session request payload.
> > + */
> > + len = sizeof(req) - offsetof(struct rfc1002_session_packet, trailer.session_req);
> >
> > - /* == cpu_to_be32(0x81000044) */
> > - smb_buf->smb_buf_length =
> > - cpu_to_be32((RFC1002_SESSION_REQUEST << 24) | req_noscope_len);
> > - rc = smb_send(server, smb_buf, 0x44);
> > - kfree(ses_init_buf);
> > - /*
> > - * RFC1001 layer in at least one server
> > - * requires very short break before negprot
> > - * presumably because not expecting negprot
> > - * to follow so fast. This is a simple
> > - * solution that works without
> > - * complicating the code and causes no
> > - * significant slowing down on mount
> > - * for everyone else
> > - */
> > - usleep_range(1000, 2000);
> > - }
> > + smb_buf->smb_buf_length = cpu_to_be32((RFC1002_SESSION_REQUEST << 24) | len);
> > + rc = smb_send(server, smb_buf, len);
> > /*
> > - * else the negprot may still work without this
> > - * even though malloc failed
> > + * RFC1001 layer in at least one server requires very short break before
> > + * negprot presumably because not expecting negprot to follow so fast.
> > + * This is a simple solution that works without complicating the code
> > + * and causes no significant slowing down on mount for everyone else
> > */
> > + usleep_range(1000, 2000);
> >
> > return rc;
> > }
> > --
> > 2.39.1
> >
--
Thanks,
Steve
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-02-17 5:33 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-02-16 18:33 [PATCH] cifs: fix mount on old smb servers Paulo Alcantara
2023-02-16 21:08 ` ronnie sahlberg
2023-02-17 5:33 ` Steve French
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.