[PATCH][CIFS] Do not build smb1ops.c if legacy support is disabled

[PATCH][CIFS] Do not build smb1ops.c if legacy support is disabled

[Steve French]

[-- Attachment #1: Type: text/plain, Size: 398 bytes --]

We should not be including unused SMB1/CIFS functions when legacy
support is disabled (CONFIG_CIFS_ALLOW_INSECURE_LEGACY turned
off), but especially obvious is not needing to build smb1ops.c
at all when legacy support is disabled. Over time we can move
more SMB1/CIFS and SMB2.0 legacy functions into ifdefs but this
is a good start (and shrinks the module size a few percent).

-- 
Thanks,

Steve

[-- Attachment #2: 0001-cifs-do-not-build-smb1ops-if-legacy-support-is-disab.patch --]
[-- Type: text/x-patch, Size: 1620 bytes --]

From 41db7a9e28f09d57c145df814f0fb6f200c8d2a6 Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Wed, 1 Jun 2022 21:25:43 -0500
Subject: [PATCH] cifs: do not build smb1ops if legacy support is disabled

We should not be including unused SMB1/CIFS functions when legacy
support is disabled (CONFIG_CIFS_ALLOW_INSECURE_LEGACY turned
off), but especially obvious is not needing to build smb1ops.c
at all when legacy support is disabled. Over time we can move
more SMB1/CIFS and SMB2.0 legacy functions into ifdefs but this
is a good start (and shrinks the module size a few percent).

Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/Makefile | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/fs/cifs/Makefile b/fs/cifs/Makefile
index cc8fdcb35b71..8c9f2c00be72 100644
--- a/fs/cifs/Makefile
+++ b/fs/cifs/Makefile
@@ -8,7 +8,7 @@ obj-$(CONFIG_CIFS) += cifs.o
 cifs-y := trace.o cifsfs.o cifssmb.o cifs_debug.o connect.o dir.o file.o \
 	  inode.o link.o misc.o netmisc.o smbencrypt.o transport.o \
 	  cifs_unicode.o nterr.o cifsencrypt.o \
-	  readdir.o ioctl.o sess.o export.o smb1ops.o unc.o winucase.o \
+	  readdir.o ioctl.o sess.o export.o unc.o winucase.o \
 	  smb2ops.o smb2maperror.o smb2transport.o \
 	  smb2misc.o smb2pdu.o smb2inode.o smb2file.o cifsacl.o fs_context.o \
 	  dns_resolve.o cifs_spnego_negtokeninit.asn1.o asn1.o
@@ -30,3 +30,5 @@ cifs-$(CONFIG_CIFS_FSCACHE) += fscache.o
 cifs-$(CONFIG_CIFS_SMB_DIRECT) += smbdirect.o
 
 cifs-$(CONFIG_CIFS_ROOT) += cifsroot.o
+
+cifs-$(CONFIG_CIFS_ALLOW_INSECURE_LEGACY) += smb1ops.o
-- 
2.34.1

Re: [PATCH][CIFS] Do not build smb1ops.c if legacy support is disabled

[Steve French]

[-- Attachment #1: Type: text/plain, Size: 592 bytes --]

Another minor one to remove some unneeded SMB20 code when legacy is disabled


On Wed, Jun 1, 2022 at 9:39 PM Steve French <smfrench@gmail.com> wrote:
>
> We should not be including unused SMB1/CIFS functions when legacy
> support is disabled (CONFIG_CIFS_ALLOW_INSECURE_LEGACY turned
> off), but especially obvious is not needing to build smb1ops.c
> at all when legacy support is disabled. Over time we can move
> more SMB1/CIFS and SMB2.0 legacy functions into ifdefs but this
> is a good start (and shrinks the module size a few percent).
>
> --
> Thanks,
>
> Steve



-- 
Thanks,

Steve

[-- Attachment #2: 0001-cifs-version-operations-for-smb20-unneeded-when-lega.patch --]
[-- Type: text/x-patch, Size: 3209 bytes --]

From 7ef93ffccd55fb0ba000ed16ef6a81cd7dee07b5 Mon Sep 17 00:00:00 2001
From: Steve French <stfrench@microsoft.com>
Date: Wed, 1 Jun 2022 22:08:46 -0500
Subject: [PATCH] cifs: version operations for smb20 unneeded when legacy
 support disabled

We should not be including unused smb20 specific code when legacy
support is disabled (CONFIG_CIFS_ALLOW_INSECURE_LEGACY turned
off).  For example smb2_operations and smb2_values aren't used
in that case.  Over time we can move more and more SMB1/CIFS and SMB2.0
code into the insecure legacy ifdefs

Reviewed-by: Ronnie Sahlberg <lsahlber@redhat.com>
Signed-off-by: Steve French <stfrench@microsoft.com>
---
 fs/cifs/cifsglob.h | 4 +++-
 fs/cifs/smb2ops.c  | 7 ++++++-
 2 files changed, 9 insertions(+), 2 deletions(-)

diff --git a/fs/cifs/cifsglob.h b/fs/cifs/cifsglob.h
index d589e687611d..f873379066c7 100644
--- a/fs/cifs/cifsglob.h
+++ b/fs/cifs/cifsglob.h
@@ -1963,11 +1963,13 @@ extern mempool_t *cifs_mid_poolp;
 
 /* Operations for different SMB versions */
 #define SMB1_VERSION_STRING	"1.0"
+#define SMB20_VERSION_STRING    "2.0"
+#ifdef CONFIG_CIFS_ALLOW_INSECURE_LEGACY
 extern struct smb_version_operations smb1_operations;
 extern struct smb_version_values smb1_values;
-#define SMB20_VERSION_STRING	"2.0"
 extern struct smb_version_operations smb20_operations;
 extern struct smb_version_values smb20_values;
+#endif /* CIFS_ALLOW_INSECURE_LEGACY */
 #define SMB21_VERSION_STRING	"2.1"
 extern struct smb_version_operations smb21_operations;
 extern struct smb_version_values smb21_values;
diff --git a/fs/cifs/smb2ops.c b/fs/cifs/smb2ops.c
index 03ab28c341c4..98a76fa791c0 100644
--- a/fs/cifs/smb2ops.c
+++ b/fs/cifs/smb2ops.c
@@ -4345,11 +4345,13 @@ smb3_set_oplock_level(struct cifsInodeInfo *cinode, __u32 oplock,
 	}
 }
 
+#ifdef CONFIG_CIFS_ALLOW_INSECURE_LEGACY
 static bool
 smb2_is_read_op(__u32 oplock)
 {
 	return oplock == SMB2_OPLOCK_LEVEL_II;
 }
+#endif /* CIFS_ALLOW_INSECURE_LEGACY */
 
 static bool
 smb21_is_read_op(__u32 oplock)
@@ -5448,7 +5450,7 @@ smb2_make_node(unsigned int xid, struct inode *inode,
 	return rc;
 }
 
-
+#ifdef CONFIG_CIFS_ALLOW_INSECURE_LEGACY
 struct smb_version_operations smb20_operations = {
 	.compare_fids = smb2_compare_fids,
 	.setup_request = smb2_setup_request,
@@ -5547,6 +5549,7 @@ struct smb_version_operations smb20_operations = {
 	.is_status_io_timeout = smb2_is_status_io_timeout,
 	.is_network_name_deleted = smb2_is_network_name_deleted,
 };
+#endif /* CIFS_ALLOW_INSECURE_LEGACY */
 
 struct smb_version_operations smb21_operations = {
 	.compare_fids = smb2_compare_fids,
@@ -5878,6 +5881,7 @@ struct smb_version_operations smb311_operations = {
 	.is_network_name_deleted = smb2_is_network_name_deleted,
 };
 
+#ifdef CONFIG_CIFS_ALLOW_INSECURE_LEGACY
 struct smb_version_values smb20_values = {
 	.version_string = SMB20_VERSION_STRING,
 	.protocol_id = SMB20_PROT_ID,
@@ -5898,6 +5902,7 @@ struct smb_version_values smb20_values = {
 	.signing_required = SMB2_NEGOTIATE_SIGNING_REQUIRED,
 	.create_lease_size = sizeof(struct create_lease),
 };
+#endif /* ALLOW_INSECURE_LEGACY */
 
 struct smb_version_values smb21_values = {
 	.version_string = SMB21_VERSION_STRING,
-- 
2.34.1

Re: [PATCH][CIFS] Do not build smb1ops.c if legacy support is disabled

[ronnie sahlberg]

looks good   reviewed by me

On Thu, 2 Jun 2022 at 13:47, Steve French via samba-technical
<samba-technical@lists.samba.org> wrote:
>
> Another minor one to remove some unneeded SMB20 code when legacy is disabled
>
>
> On Wed, Jun 1, 2022 at 9:39 PM Steve French <smfrench@gmail.com> wrote:
> >
> > We should not be including unused SMB1/CIFS functions when legacy
> > support is disabled (CONFIG_CIFS_ALLOW_INSECURE_LEGACY turned
> > off), but especially obvious is not needing to build smb1ops.c
> > at all when legacy support is disabled. Over time we can move
> > more SMB1/CIFS and SMB2.0 legacy functions into ifdefs but this
> > is a good start (and shrinks the module size a few percent).
> >
> > --
> > Thanks,
> >
> > Steve
>
>
>
> --
> Thanks,
>
> Steve

Re: [PATCH][CIFS] Do not build smb1ops.c if legacy support is disabled

[Tom Talpey]

LGTM, but I had some additional suggestions that I found when
researching how to yank the entire SMB1 code into a module.
Which actually looks quite possible, but for another day.

This patch doesn't actually stop building smb1ops.c and cifssmb.c
however. Don't you want to deselect them in the kconfig?

Feel free to add my
Reviewed-by: Tom Talpey <tom@talpey.com>


On 6/1/2022 11:45 PM, Steve French wrote:
> Another minor one to remove some unneeded SMB20 code when legacy is disabled
> 
> 
> On Wed, Jun 1, 2022 at 9:39 PM Steve French <smfrench@gmail.com> wrote:
>>
>> We should not be including unused SMB1/CIFS functions when legacy
>> support is disabled (CONFIG_CIFS_ALLOW_INSECURE_LEGACY turned
>> off), but especially obvious is not needing to build smb1ops.c
>> at all when legacy support is disabled. Over time we can move
>> more SMB1/CIFS and SMB2.0 legacy functions into ifdefs but this
>> is a good start (and shrinks the module size a few percent).
>>
>> --
>> Thanks,
>>
>> Steve
> 
> 
> 

Fwd: [PATCH][CIFS] Do not build smb1ops.c if legacy support is disabled

[Steve French]

---------- Forwarded message ---------
From: Steve French <smfrench@gmail.com>
Date: Thu, Jun 2, 2022 at 8:23 PM
Subject: Re: [PATCH][CIFS] Do not build smb1ops.c if legacy support is disabled
To: Tom Talpey <tom@talpey.com>
Cc: CIFS <linux-cifs@vger.kernel.org>, samba-technical
<samba-technical@lists.samba.org>


I was thinking about staging smb1 removal gradually over about a year
and a half (assuming that Samba has the SMB3.1.1 POSIX Extensions
merged in from jra's tree by then so no excuses functionally to
removing smb1)
- marking the CONFIG_CIFS_ALLOW_INSECURE_LEGACY as recommended 'N'
starting in next release 5.20 and move some additional SMB1 code into
the #ifdef
- in 5.21 pulling some of those smb1 specific pieces into a new helper
module (perhaps smb1.ko??)
- in 5.20 or 5.21 renaming fs/cifs directory to fs'smbfs_client (or
something similar)
- evaluate whether we can change the default module name to smb3.ko
from cifs.ko (we already have a module alias for cifs.ko of "smb3" and
have for years been able to "mount -t smb3" with cifs.ko)
- in 5.21 note that insecure legacy support is scheduled for removal
(perhaps a year later), and if anyone mounts with "vers=1.0" (or
vers=2.0) print a warning that it is scheduled for removal in a year.

Thoughts?


On Thu, Jun 2, 2022, 11:39 Tom Talpey <tom@talpey.com> wrote:
>
> LGTM, but I had some additional suggestions that I found when
> researching how to yank the entire SMB1 code into a module.
> Which actually looks quite possible, but for another day.
>
> This patch doesn't actually stop building smb1ops.c and cifssmb.c
> however. Don't you want to deselect them in the kconfig?
>
> Feel free to add my
> Reviewed-by: Tom Talpey <tom@talpey.com>
>
>
> On 6/1/2022 11:45 PM, Steve French wrote:
> > Another minor one to remove some unneeded SMB20 code when legacy is disabled
> >
> >
> > On Wed, Jun 1, 2022 at 9:39 PM Steve French <smfrench@gmail.com> wrote:
> >>
> >> We should not be including unused SMB1/CIFS functions when legacy
> >> support is disabled (CONFIG_CIFS_ALLOW_INSECURE_LEGACY turned
> >> off), but especially obvious is not needing to build smb1ops.c
> >> at all when legacy support is disabled. Over time we can move
> >> more SMB1/CIFS and SMB2.0 legacy functions into ifdefs but this
> >> is a good start (and shrinks the module size a few percent).
> >>
> >> --
> >> Thanks,
> >>
> >> Steve
> >
> >
> >



-- 
Thanks,

Steve