Replace internal logic with an independent arc4 library.
Signed-off-by: John Sanpe <[email protected]>
---
fs/smb/Kconfig | 1 +
fs/smb/client/cifsencrypt.c | 7 ++--
fs/smb/common/Makefile | 1 -
fs/smb/common/arc4.h | 23 ------------
fs/smb/common/cifs_arc4.c | 74 -------------------------------------
fs/smb/server/auth.c | 6 +--
6 files changed, 7 insertions(+), 105 deletions(-)
delete mode 100644 fs/smb/common/arc4.h
delete mode 100644 fs/smb/common/cifs_arc4.c
diff --git a/fs/smb/Kconfig b/fs/smb/Kconfig
index ef425789fa6a..65e5a437898b 100644
--- a/fs/smb/Kconfig
+++ b/fs/smb/Kconfig
@@ -7,5 +7,6 @@ source "fs/smb/server/Kconfig"
config SMBFS
tristate
+ select CRYPTO_LIB_ARC4
default y if CIFS=y || SMB_SERVER=y
default m if CIFS=m || SMB_SERVER=m
diff --git a/fs/smb/client/cifsencrypt.c b/fs/smb/client/cifsencrypt.c
index ef4c2e3c9fa6..d8754c406b5f 100644
--- a/fs/smb/client/cifsencrypt.c
+++ b/fs/smb/client/cifsencrypt.c
@@ -21,7 +21,7 @@
#include <linux/random.h>
#include <linux/highmem.h>
#include <linux/fips.h>
-#include "../common/arc4.h"
+#include <crypto/arc4.h>
#include <crypto/aead.h>
/*
@@ -826,9 +826,8 @@ calc_seckey(struct cifs_ses *ses)
return -ENOMEM;
}
- cifs_arc4_setkey(ctx_arc4, ses->auth_key.response, CIFS_SESS_KEY_SIZE);
- cifs_arc4_crypt(ctx_arc4, ses->ntlmssp->ciphertext, sec_key,
- CIFS_CPHTXT_SIZE);
+ arc4_setkey(ctx_arc4, ses->auth_key.response, CIFS_SESS_KEY_SIZE);
+ arc4_crypt(ctx_arc4, ses->ntlmssp->ciphertext, sec_key, CIFS_CPHTXT_SIZE);
/* make secondary_key/nonce as session key */
memcpy(ses->auth_key.response, sec_key, CIFS_SESS_KEY_SIZE);
diff --git a/fs/smb/common/Makefile b/fs/smb/common/Makefile
index c66dbbc1469c..9e0730a385fb 100644
--- a/fs/smb/common/Makefile
+++ b/fs/smb/common/Makefile
@@ -3,5 +3,4 @@
# Makefile for Linux filesystem routines that are shared by client and server.
#
-obj-$(CONFIG_SMBFS) += cifs_arc4.o
obj-$(CONFIG_SMBFS) += cifs_md4.o
diff --git a/fs/smb/common/arc4.h b/fs/smb/common/arc4.h
deleted file mode 100644
index 12e71ec033a1..000000000000
--- a/fs/smb/common/arc4.h
+++ /dev/null
@@ -1,23 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0+ */
-/*
- * Common values for ARC4 Cipher Algorithm
- */
-
-#ifndef _CRYPTO_ARC4_H
-#define _CRYPTO_ARC4_H
-
-#include <linux/types.h>
-
-#define ARC4_MIN_KEY_SIZE 1
-#define ARC4_MAX_KEY_SIZE 256
-#define ARC4_BLOCK_SIZE 1
-
-struct arc4_ctx {
- u32 S[256];
- u32 x, y;
-};
-
-int cifs_arc4_setkey(struct arc4_ctx *ctx, const u8 *in_key, unsigned int key_len);
-void cifs_arc4_crypt(struct arc4_ctx *ctx, u8 *out, const u8 *in, unsigned int len);
-
-#endif /* _CRYPTO_ARC4_H */
diff --git a/fs/smb/common/cifs_arc4.c b/fs/smb/common/cifs_arc4.c
deleted file mode 100644
index 043e4cb839fa..000000000000
--- a/fs/smb/common/cifs_arc4.c
+++ /dev/null
@@ -1,74 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0-or-later
-/*
- * Cryptographic API
- *
- * ARC4 Cipher Algorithm
- *
- * Jon Oberheide <[email protected]>
- */
-
-#include <linux/module.h>
-#include "arc4.h"
-
-MODULE_LICENSE("GPL");
-
-int cifs_arc4_setkey(struct arc4_ctx *ctx, const u8 *in_key, unsigned int key_len)
-{
- int i, j = 0, k = 0;
-
- ctx->x = 1;
- ctx->y = 0;
-
- for (i = 0; i < 256; i++)
- ctx->S[i] = i;
-
- for (i = 0; i < 256; i++) {
- u32 a = ctx->S[i];
-
- j = (j + in_key[k] + a) & 0xff;
- ctx->S[i] = ctx->S[j];
- ctx->S[j] = a;
- if (++k >= key_len)
- k = 0;
- }
-
- return 0;
-}
-EXPORT_SYMBOL_GPL(cifs_arc4_setkey);
-
-void cifs_arc4_crypt(struct arc4_ctx *ctx, u8 *out, const u8 *in, unsigned int len)
-{
- u32 *const S = ctx->S;
- u32 x, y, a, b;
- u32 ty, ta, tb;
-
- if (len == 0)
- return;
-
- x = ctx->x;
- y = ctx->y;
-
- a = S[x];
- y = (y + a) & 0xff;
- b = S[y];
-
- do {
- S[y] = a;
- a = (a + b) & 0xff;
- S[x] = b;
- x = (x + 1) & 0xff;
- ta = S[x];
- ty = (y + ta) & 0xff;
- tb = S[ty];
- *out++ = *in++ ^ S[a];
- if (--len == 0)
- break;
- y = ty;
- a = ta;
- b = tb;
- } while (true);
-
- ctx->x = x;
- ctx->y = y;
-}
-EXPORT_SYMBOL_GPL(cifs_arc4_crypt);
diff --git a/fs/smb/server/auth.c b/fs/smb/server/auth.c
index 229a6527870d..5640196b313f 100644
--- a/fs/smb/server/auth.c
+++ b/fs/smb/server/auth.c
@@ -29,7 +29,7 @@
#include "mgmt/user_config.h"
#include "crypto_ctx.h"
#include "transport_ipc.h"
-#include "../common/arc4.h"
+#include <crypto/arc4.h>
/*
* Fixed format data defining GSS header and fixed string
@@ -362,9 +362,9 @@ int ksmbd_decode_ntlmssp_auth_blob(struct authenticate_message *authblob,
if (!ctx_arc4)
return -ENOMEM;
- cifs_arc4_setkey(ctx_arc4, sess->sess_key,
+ arc4_setkey(ctx_arc4, sess->sess_key,
SMB2_NTLMV2_SESSKEY_SIZE);
- cifs_arc4_crypt(ctx_arc4, sess->sess_key,
+ arc4_crypt(ctx_arc4, sess->sess_key,
(char *)authblob + sess_key_off, sess_key_len);
kfree_sensitive(ctx_arc4);
}
--
2.41.0
I thought that the whole point of kernel crypto guys was the reverse -
ie arc4 must be moved to cifs.ko since cifs/smb3 mounts had the only
approved use case. Ronnie may have additional context, but there was
a push to remove arc4 and md4 (see e.g. the emails threads about:
"crypto: remove MD4 generic shash"). I also want to be careful that
we don't accidentally disable smb3.1.1 mounts (which are highly
secure) because they have small dependencies on old algorithms (even
if that doesn't cause problems with typical reasonable length password
cases)
commit 71c02863246167b3d1639b8278681ca8ebedcb4e
Author: Ronnie Sahlberg <[email protected]>
Date: Thu Aug 19 20:34:59 2021 +1000
cifs: fork arc4 and create a separate module for it for cifs and other users
We can not drop ARC4 and basically destroy CIFS connectivity for
almost all CIFS users so create a new forked ARC4 module that CIFS and other
subsystems that have a hard dependency on ARC4 can use.
On Sun, Oct 22, 2023 at 1:39 PM John Sanpe <[email protected]> wrote:
>
> Replace internal logic with an independent arc4 library.
>
> Signed-off-by: John Sanpe <[email protected]>
> ---
> fs/smb/Kconfig | 1 +
> fs/smb/client/cifsencrypt.c | 7 ++--
> fs/smb/common/Makefile | 1 -
> fs/smb/common/arc4.h | 23 ------------
> fs/smb/common/cifs_arc4.c | 74 -------------------------------------
> fs/smb/server/auth.c | 6 +--
> 6 files changed, 7 insertions(+), 105 deletions(-)
> delete mode 100644 fs/smb/common/arc4.h
> delete mode 100644 fs/smb/common/cifs_arc4.c
>
> diff --git a/fs/smb/Kconfig b/fs/smb/Kconfig
> index ef425789fa6a..65e5a437898b 100644
> --- a/fs/smb/Kconfig
> +++ b/fs/smb/Kconfig
> @@ -7,5 +7,6 @@ source "fs/smb/server/Kconfig"
>
> config SMBFS
> tristate
> + select CRYPTO_LIB_ARC4
> default y if CIFS=y || SMB_SERVER=y
> default m if CIFS=m || SMB_SERVER=m
> diff --git a/fs/smb/client/cifsencrypt.c b/fs/smb/client/cifsencrypt.c
> index ef4c2e3c9fa6..d8754c406b5f 100644
> --- a/fs/smb/client/cifsencrypt.c
> +++ b/fs/smb/client/cifsencrypt.c
> @@ -21,7 +21,7 @@
> #include <linux/random.h>
> #include <linux/highmem.h>
> #include <linux/fips.h>
> -#include "../common/arc4.h"
> +#include <crypto/arc4.h>
> #include <crypto/aead.h>
>
> /*
> @@ -826,9 +826,8 @@ calc_seckey(struct cifs_ses *ses)
> return -ENOMEM;
> }
>
> - cifs_arc4_setkey(ctx_arc4, ses->auth_key.response, CIFS_SESS_KEY_SIZE);
> - cifs_arc4_crypt(ctx_arc4, ses->ntlmssp->ciphertext, sec_key,
> - CIFS_CPHTXT_SIZE);
> + arc4_setkey(ctx_arc4, ses->auth_key.response, CIFS_SESS_KEY_SIZE);
> + arc4_crypt(ctx_arc4, ses->ntlmssp->ciphertext, sec_key, CIFS_CPHTXT_SIZE);
>
> /* make secondary_key/nonce as session key */
> memcpy(ses->auth_key.response, sec_key, CIFS_SESS_KEY_SIZE);
> diff --git a/fs/smb/common/Makefile b/fs/smb/common/Makefile
> index c66dbbc1469c..9e0730a385fb 100644
> --- a/fs/smb/common/Makefile
> +++ b/fs/smb/common/Makefile
> @@ -3,5 +3,4 @@
> # Makefile for Linux filesystem routines that are shared by client and server.
> #
>
> -obj-$(CONFIG_SMBFS) += cifs_arc4.o
> obj-$(CONFIG_SMBFS) += cifs_md4.o
> diff --git a/fs/smb/common/arc4.h b/fs/smb/common/arc4.h
> deleted file mode 100644
> index 12e71ec033a1..000000000000
> --- a/fs/smb/common/arc4.h
> +++ /dev/null
> @@ -1,23 +0,0 @@
> -/* SPDX-License-Identifier: GPL-2.0+ */
> -/*
> - * Common values for ARC4 Cipher Algorithm
> - */
> -
> -#ifndef _CRYPTO_ARC4_H
> -#define _CRYPTO_ARC4_H
> -
> -#include <linux/types.h>
> -
> -#define ARC4_MIN_KEY_SIZE 1
> -#define ARC4_MAX_KEY_SIZE 256
> -#define ARC4_BLOCK_SIZE 1
> -
> -struct arc4_ctx {
> - u32 S[256];
> - u32 x, y;
> -};
> -
> -int cifs_arc4_setkey(struct arc4_ctx *ctx, const u8 *in_key, unsigned int key_len);
> -void cifs_arc4_crypt(struct arc4_ctx *ctx, u8 *out, const u8 *in, unsigned int len);
> -
> -#endif /* _CRYPTO_ARC4_H */
> diff --git a/fs/smb/common/cifs_arc4.c b/fs/smb/common/cifs_arc4.c
> deleted file mode 100644
> index 043e4cb839fa..000000000000
> --- a/fs/smb/common/cifs_arc4.c
> +++ /dev/null
> @@ -1,74 +0,0 @@
> -// SPDX-License-Identifier: GPL-2.0-or-later
> -/*
> - * Cryptographic API
> - *
> - * ARC4 Cipher Algorithm
> - *
> - * Jon Oberheide <[email protected]>
> - */
> -
> -#include <linux/module.h>
> -#include "arc4.h"
> -
> -MODULE_LICENSE("GPL");
> -
> -int cifs_arc4_setkey(struct arc4_ctx *ctx, const u8 *in_key, unsigned int key_len)
> -{
> - int i, j = 0, k = 0;
> -
> - ctx->x = 1;
> - ctx->y = 0;
> -
> - for (i = 0; i < 256; i++)
> - ctx->S[i] = i;
> -
> - for (i = 0; i < 256; i++) {
> - u32 a = ctx->S[i];
> -
> - j = (j + in_key[k] + a) & 0xff;
> - ctx->S[i] = ctx->S[j];
> - ctx->S[j] = a;
> - if (++k >= key_len)
> - k = 0;
> - }
> -
> - return 0;
> -}
> -EXPORT_SYMBOL_GPL(cifs_arc4_setkey);
> -
> -void cifs_arc4_crypt(struct arc4_ctx *ctx, u8 *out, const u8 *in, unsigned int len)
> -{
> - u32 *const S = ctx->S;
> - u32 x, y, a, b;
> - u32 ty, ta, tb;
> -
> - if (len == 0)
> - return;
> -
> - x = ctx->x;
> - y = ctx->y;
> -
> - a = S[x];
> - y = (y + a) & 0xff;
> - b = S[y];
> -
> - do {
> - S[y] = a;
> - a = (a + b) & 0xff;
> - S[x] = b;
> - x = (x + 1) & 0xff;
> - ta = S[x];
> - ty = (y + ta) & 0xff;
> - tb = S[ty];
> - *out++ = *in++ ^ S[a];
> - if (--len == 0)
> - break;
> - y = ty;
> - a = ta;
> - b = tb;
> - } while (true);
> -
> - ctx->x = x;
> - ctx->y = y;
> -}
> -EXPORT_SYMBOL_GPL(cifs_arc4_crypt);
> diff --git a/fs/smb/server/auth.c b/fs/smb/server/auth.c
> index 229a6527870d..5640196b313f 100644
> --- a/fs/smb/server/auth.c
> +++ b/fs/smb/server/auth.c
> @@ -29,7 +29,7 @@
> #include "mgmt/user_config.h"
> #include "crypto_ctx.h"
> #include "transport_ipc.h"
> -#include "../common/arc4.h"
> +#include <crypto/arc4.h>
>
> /*
> * Fixed format data defining GSS header and fixed string
> @@ -362,9 +362,9 @@ int ksmbd_decode_ntlmssp_auth_blob(struct authenticate_message *authblob,
> if (!ctx_arc4)
> return -ENOMEM;
>
> - cifs_arc4_setkey(ctx_arc4, sess->sess_key,
> + arc4_setkey(ctx_arc4, sess->sess_key,
> SMB2_NTLMV2_SESSKEY_SIZE);
> - cifs_arc4_crypt(ctx_arc4, sess->sess_key,
> + arc4_crypt(ctx_arc4, sess->sess_key,
> (char *)authblob + sess_key_off, sess_key_len);
> kfree_sensitive(ctx_arc4);
> }
> --
> 2.41.0
>
--
Thanks,
Steve
Adding Ronnie to cc: since he may have additional context why arc4 was
moved out of crypto to cifs common code
On Sun, Oct 22, 2023 at 2:38 PM Steve French <[email protected]> wrote:
>
> I thought that the whole point of kernel crypto guys was the reverse -
> ie arc4 must be moved to cifs.ko since cifs/smb3 mounts had the only
> approved use case. Ronnie may have additional context, but there was
> a push to remove arc4 and md4 (see e.g. the emails threads about:
> "crypto: remove MD4 generic shash"). I also want to be careful that
> we don't accidentally disable smb3.1.1 mounts (which are highly
> secure) because they have small dependencies on old algorithms (even
> if that doesn't cause problems with typical reasonable length password
> cases)
>
> commit 71c02863246167b3d1639b8278681ca8ebedcb4e
> Author: Ronnie Sahlberg <[email protected]>
> Date: Thu Aug 19 20:34:59 2021 +1000
>
> cifs: fork arc4 and create a separate module for it for cifs and other users
>
> We can not drop ARC4 and basically destroy CIFS connectivity for
> almost all CIFS users so create a new forked ARC4 module that CIFS and other
> subsystems that have a hard dependency on ARC4 can use.
>
> On Sun, Oct 22, 2023 at 1:39 PM John Sanpe <[email protected]> wrote:
> >
> > Replace internal logic with an independent arc4 library.
> >
> > Signed-off-by: John Sanpe <[email protected]>
> > ---
> > fs/smb/Kconfig | 1 +
> > fs/smb/client/cifsencrypt.c | 7 ++--
> > fs/smb/common/Makefile | 1 -
> > fs/smb/common/arc4.h | 23 ------------
> > fs/smb/common/cifs_arc4.c | 74 -------------------------------------
> > fs/smb/server/auth.c | 6 +--
> > 6 files changed, 7 insertions(+), 105 deletions(-)
> > delete mode 100644 fs/smb/common/arc4.h
> > delete mode 100644 fs/smb/common/cifs_arc4.c
> >
> > diff --git a/fs/smb/Kconfig b/fs/smb/Kconfig
> > index ef425789fa6a..65e5a437898b 100644
> > --- a/fs/smb/Kconfig
> > +++ b/fs/smb/Kconfig
> > @@ -7,5 +7,6 @@ source "fs/smb/server/Kconfig"
> >
> > config SMBFS
> > tristate
> > + select CRYPTO_LIB_ARC4
> > default y if CIFS=y || SMB_SERVER=y
> > default m if CIFS=m || SMB_SERVER=m
> > diff --git a/fs/smb/client/cifsencrypt.c b/fs/smb/client/cifsencrypt.c
> > index ef4c2e3c9fa6..d8754c406b5f 100644
> > --- a/fs/smb/client/cifsencrypt.c
> > +++ b/fs/smb/client/cifsencrypt.c
> > @@ -21,7 +21,7 @@
> > #include <linux/random.h>
> > #include <linux/highmem.h>
> > #include <linux/fips.h>
> > -#include "../common/arc4.h"
> > +#include <crypto/arc4.h>
> > #include <crypto/aead.h>
> >
> > /*
> > @@ -826,9 +826,8 @@ calc_seckey(struct cifs_ses *ses)
> > return -ENOMEM;
> > }
> >
> > - cifs_arc4_setkey(ctx_arc4, ses->auth_key.response, CIFS_SESS_KEY_SIZE);
> > - cifs_arc4_crypt(ctx_arc4, ses->ntlmssp->ciphertext, sec_key,
> > - CIFS_CPHTXT_SIZE);
> > + arc4_setkey(ctx_arc4, ses->auth_key.response, CIFS_SESS_KEY_SIZE);
> > + arc4_crypt(ctx_arc4, ses->ntlmssp->ciphertext, sec_key, CIFS_CPHTXT_SIZE);
> >
> > /* make secondary_key/nonce as session key */
> > memcpy(ses->auth_key.response, sec_key, CIFS_SESS_KEY_SIZE);
> > diff --git a/fs/smb/common/Makefile b/fs/smb/common/Makefile
> > index c66dbbc1469c..9e0730a385fb 100644
> > --- a/fs/smb/common/Makefile
> > +++ b/fs/smb/common/Makefile
> > @@ -3,5 +3,4 @@
> > # Makefile for Linux filesystem routines that are shared by client and server.
> > #
> >
> > -obj-$(CONFIG_SMBFS) += cifs_arc4.o
> > obj-$(CONFIG_SMBFS) += cifs_md4.o
> > diff --git a/fs/smb/common/arc4.h b/fs/smb/common/arc4.h
> > deleted file mode 100644
> > index 12e71ec033a1..000000000000
> > --- a/fs/smb/common/arc4.h
> > +++ /dev/null
> > @@ -1,23 +0,0 @@
> > -/* SPDX-License-Identifier: GPL-2.0+ */
> > -/*
> > - * Common values for ARC4 Cipher Algorithm
> > - */
> > -
> > -#ifndef _CRYPTO_ARC4_H
> > -#define _CRYPTO_ARC4_H
> > -
> > -#include <linux/types.h>
> > -
> > -#define ARC4_MIN_KEY_SIZE 1
> > -#define ARC4_MAX_KEY_SIZE 256
> > -#define ARC4_BLOCK_SIZE 1
> > -
> > -struct arc4_ctx {
> > - u32 S[256];
> > - u32 x, y;
> > -};
> > -
> > -int cifs_arc4_setkey(struct arc4_ctx *ctx, const u8 *in_key, unsigned int key_len);
> > -void cifs_arc4_crypt(struct arc4_ctx *ctx, u8 *out, const u8 *in, unsigned int len);
> > -
> > -#endif /* _CRYPTO_ARC4_H */
> > diff --git a/fs/smb/common/cifs_arc4.c b/fs/smb/common/cifs_arc4.c
> > deleted file mode 100644
> > index 043e4cb839fa..000000000000
> > --- a/fs/smb/common/cifs_arc4.c
> > +++ /dev/null
> > @@ -1,74 +0,0 @@
> > -// SPDX-License-Identifier: GPL-2.0-or-later
> > -/*
> > - * Cryptographic API
> > - *
> > - * ARC4 Cipher Algorithm
> > - *
> > - * Jon Oberheide <[email protected]>
> > - */
> > -
> > -#include <linux/module.h>
> > -#include "arc4.h"
> > -
> > -MODULE_LICENSE("GPL");
> > -
> > -int cifs_arc4_setkey(struct arc4_ctx *ctx, const u8 *in_key, unsigned int key_len)
> > -{
> > - int i, j = 0, k = 0;
> > -
> > - ctx->x = 1;
> > - ctx->y = 0;
> > -
> > - for (i = 0; i < 256; i++)
> > - ctx->S[i] = i;
> > -
> > - for (i = 0; i < 256; i++) {
> > - u32 a = ctx->S[i];
> > -
> > - j = (j + in_key[k] + a) & 0xff;
> > - ctx->S[i] = ctx->S[j];
> > - ctx->S[j] = a;
> > - if (++k >= key_len)
> > - k = 0;
> > - }
> > -
> > - return 0;
> > -}
> > -EXPORT_SYMBOL_GPL(cifs_arc4_setkey);
> > -
> > -void cifs_arc4_crypt(struct arc4_ctx *ctx, u8 *out, const u8 *in, unsigned int len)
> > -{
> > - u32 *const S = ctx->S;
> > - u32 x, y, a, b;
> > - u32 ty, ta, tb;
> > -
> > - if (len == 0)
> > - return;
> > -
> > - x = ctx->x;
> > - y = ctx->y;
> > -
> > - a = S[x];
> > - y = (y + a) & 0xff;
> > - b = S[y];
> > -
> > - do {
> > - S[y] = a;
> > - a = (a + b) & 0xff;
> > - S[x] = b;
> > - x = (x + 1) & 0xff;
> > - ta = S[x];
> > - ty = (y + ta) & 0xff;
> > - tb = S[ty];
> > - *out++ = *in++ ^ S[a];
> > - if (--len == 0)
> > - break;
> > - y = ty;
> > - a = ta;
> > - b = tb;
> > - } while (true);
> > -
> > - ctx->x = x;
> > - ctx->y = y;
> > -}
> > -EXPORT_SYMBOL_GPL(cifs_arc4_crypt);
> > diff --git a/fs/smb/server/auth.c b/fs/smb/server/auth.c
> > index 229a6527870d..5640196b313f 100644
> > --- a/fs/smb/server/auth.c
> > +++ b/fs/smb/server/auth.c
> > @@ -29,7 +29,7 @@
> > #include "mgmt/user_config.h"
> > #include "crypto_ctx.h"
> > #include "transport_ipc.h"
> > -#include "../common/arc4.h"
> > +#include <crypto/arc4.h>
> >
> > /*
> > * Fixed format data defining GSS header and fixed string
> > @@ -362,9 +362,9 @@ int ksmbd_decode_ntlmssp_auth_blob(struct authenticate_message *authblob,
> > if (!ctx_arc4)
> > return -ENOMEM;
> >
> > - cifs_arc4_setkey(ctx_arc4, sess->sess_key,
> > + arc4_setkey(ctx_arc4, sess->sess_key,
> > SMB2_NTLMV2_SESSKEY_SIZE);
> > - cifs_arc4_crypt(ctx_arc4, sess->sess_key,
> > + arc4_crypt(ctx_arc4, sess->sess_key,
> > (char *)authblob + sess_key_off, sess_key_len);
> > kfree_sensitive(ctx_arc4);
> > }
> > --
> > 2.41.0
> >
>
>
> --
> Thanks,
>
> Steve
--
Thanks,
Steve
You are right. The reason that arc4 and friend were moved into cifs
was because the crypto guys told us "we will delete these algorithms
from the crypto library"
On Mon, 23 Oct 2023 at 05:40, Steve French <[email protected]> wrote:
>
> Adding Ronnie to cc: since he may have additional context why arc4 was
> moved out of crypto to cifs common code
>
> On Sun, Oct 22, 2023 at 2:38 PM Steve French <[email protected]> wrote:
> >
> > I thought that the whole point of kernel crypto guys was the reverse -
> > ie arc4 must be moved to cifs.ko since cifs/smb3 mounts had the only
> > approved use case. Ronnie may have additional context, but there was
> > a push to remove arc4 and md4 (see e.g. the emails threads about:
> > "crypto: remove MD4 generic shash"). I also want to be careful that
> > we don't accidentally disable smb3.1.1 mounts (which are highly
> > secure) because they have small dependencies on old algorithms (even
> > if that doesn't cause problems with typical reasonable length password
> > cases)
> >
> > commit 71c02863246167b3d1639b8278681ca8ebedcb4e
> > Author: Ronnie Sahlberg <[email protected]>
> > Date: Thu Aug 19 20:34:59 2021 +1000
> >
> > cifs: fork arc4 and create a separate module for it for cifs and other users
> >
> > We can not drop ARC4 and basically destroy CIFS connectivity for
> > almost all CIFS users so create a new forked ARC4 module that CIFS and other
> > subsystems that have a hard dependency on ARC4 can use.
> >
> > On Sun, Oct 22, 2023 at 1:39 PM John Sanpe <[email protected]> wrote:
> > >
> > > Replace internal logic with an independent arc4 library.
> > >
> > > Signed-off-by: John Sanpe <[email protected]>
> > > ---
> > > fs/smb/Kconfig | 1 +
> > > fs/smb/client/cifsencrypt.c | 7 ++--
> > > fs/smb/common/Makefile | 1 -
> > > fs/smb/common/arc4.h | 23 ------------
> > > fs/smb/common/cifs_arc4.c | 74 -------------------------------------
> > > fs/smb/server/auth.c | 6 +--
> > > 6 files changed, 7 insertions(+), 105 deletions(-)
> > > delete mode 100644 fs/smb/common/arc4.h
> > > delete mode 100644 fs/smb/common/cifs_arc4.c
> > >
> > > diff --git a/fs/smb/Kconfig b/fs/smb/Kconfig
> > > index ef425789fa6a..65e5a437898b 100644
> > > --- a/fs/smb/Kconfig
> > > +++ b/fs/smb/Kconfig
> > > @@ -7,5 +7,6 @@ source "fs/smb/server/Kconfig"
> > >
> > > config SMBFS
> > > tristate
> > > + select CRYPTO_LIB_ARC4
> > > default y if CIFS=y || SMB_SERVER=y
> > > default m if CIFS=m || SMB_SERVER=m
> > > diff --git a/fs/smb/client/cifsencrypt.c b/fs/smb/client/cifsencrypt.c
> > > index ef4c2e3c9fa6..d8754c406b5f 100644
> > > --- a/fs/smb/client/cifsencrypt.c
> > > +++ b/fs/smb/client/cifsencrypt.c
> > > @@ -21,7 +21,7 @@
> > > #include <linux/random.h>
> > > #include <linux/highmem.h>
> > > #include <linux/fips.h>
> > > -#include "../common/arc4.h"
> > > +#include <crypto/arc4.h>
> > > #include <crypto/aead.h>
> > >
> > > /*
> > > @@ -826,9 +826,8 @@ calc_seckey(struct cifs_ses *ses)
> > > return -ENOMEM;
> > > }
> > >
> > > - cifs_arc4_setkey(ctx_arc4, ses->auth_key.response, CIFS_SESS_KEY_SIZE);
> > > - cifs_arc4_crypt(ctx_arc4, ses->ntlmssp->ciphertext, sec_key,
> > > - CIFS_CPHTXT_SIZE);
> > > + arc4_setkey(ctx_arc4, ses->auth_key.response, CIFS_SESS_KEY_SIZE);
> > > + arc4_crypt(ctx_arc4, ses->ntlmssp->ciphertext, sec_key, CIFS_CPHTXT_SIZE);
> > >
> > > /* make secondary_key/nonce as session key */
> > > memcpy(ses->auth_key.response, sec_key, CIFS_SESS_KEY_SIZE);
> > > diff --git a/fs/smb/common/Makefile b/fs/smb/common/Makefile
> > > index c66dbbc1469c..9e0730a385fb 100644
> > > --- a/fs/smb/common/Makefile
> > > +++ b/fs/smb/common/Makefile
> > > @@ -3,5 +3,4 @@
> > > # Makefile for Linux filesystem routines that are shared by client and server.
> > > #
> > >
> > > -obj-$(CONFIG_SMBFS) += cifs_arc4.o
> > > obj-$(CONFIG_SMBFS) += cifs_md4.o
> > > diff --git a/fs/smb/common/arc4.h b/fs/smb/common/arc4.h
> > > deleted file mode 100644
> > > index 12e71ec033a1..000000000000
> > > --- a/fs/smb/common/arc4.h
> > > +++ /dev/null
> > > @@ -1,23 +0,0 @@
> > > -/* SPDX-License-Identifier: GPL-2.0+ */
> > > -/*
> > > - * Common values for ARC4 Cipher Algorithm
> > > - */
> > > -
> > > -#ifndef _CRYPTO_ARC4_H
> > > -#define _CRYPTO_ARC4_H
> > > -
> > > -#include <linux/types.h>
> > > -
> > > -#define ARC4_MIN_KEY_SIZE 1
> > > -#define ARC4_MAX_KEY_SIZE 256
> > > -#define ARC4_BLOCK_SIZE 1
> > > -
> > > -struct arc4_ctx {
> > > - u32 S[256];
> > > - u32 x, y;
> > > -};
> > > -
> > > -int cifs_arc4_setkey(struct arc4_ctx *ctx, const u8 *in_key, unsigned int key_len);
> > > -void cifs_arc4_crypt(struct arc4_ctx *ctx, u8 *out, const u8 *in, unsigned int len);
> > > -
> > > -#endif /* _CRYPTO_ARC4_H */
> > > diff --git a/fs/smb/common/cifs_arc4.c b/fs/smb/common/cifs_arc4.c
> > > deleted file mode 100644
> > > index 043e4cb839fa..000000000000
> > > --- a/fs/smb/common/cifs_arc4.c
> > > +++ /dev/null
> > > @@ -1,74 +0,0 @@
> > > -// SPDX-License-Identifier: GPL-2.0-or-later
> > > -/*
> > > - * Cryptographic API
> > > - *
> > > - * ARC4 Cipher Algorithm
> > > - *
> > > - * Jon Oberheide <[email protected]>
> > > - */
> > > -
> > > -#include <linux/module.h>
> > > -#include "arc4.h"
> > > -
> > > -MODULE_LICENSE("GPL");
> > > -
> > > -int cifs_arc4_setkey(struct arc4_ctx *ctx, const u8 *in_key, unsigned int key_len)
> > > -{
> > > - int i, j = 0, k = 0;
> > > -
> > > - ctx->x = 1;
> > > - ctx->y = 0;
> > > -
> > > - for (i = 0; i < 256; i++)
> > > - ctx->S[i] = i;
> > > -
> > > - for (i = 0; i < 256; i++) {
> > > - u32 a = ctx->S[i];
> > > -
> > > - j = (j + in_key[k] + a) & 0xff;
> > > - ctx->S[i] = ctx->S[j];
> > > - ctx->S[j] = a;
> > > - if (++k >= key_len)
> > > - k = 0;
> > > - }
> > > -
> > > - return 0;
> > > -}
> > > -EXPORT_SYMBOL_GPL(cifs_arc4_setkey);
> > > -
> > > -void cifs_arc4_crypt(struct arc4_ctx *ctx, u8 *out, const u8 *in, unsigned int len)
> > > -{
> > > - u32 *const S = ctx->S;
> > > - u32 x, y, a, b;
> > > - u32 ty, ta, tb;
> > > -
> > > - if (len == 0)
> > > - return;
> > > -
> > > - x = ctx->x;
> > > - y = ctx->y;
> > > -
> > > - a = S[x];
> > > - y = (y + a) & 0xff;
> > > - b = S[y];
> > > -
> > > - do {
> > > - S[y] = a;
> > > - a = (a + b) & 0xff;
> > > - S[x] = b;
> > > - x = (x + 1) & 0xff;
> > > - ta = S[x];
> > > - ty = (y + ta) & 0xff;
> > > - tb = S[ty];
> > > - *out++ = *in++ ^ S[a];
> > > - if (--len == 0)
> > > - break;
> > > - y = ty;
> > > - a = ta;
> > > - b = tb;
> > > - } while (true);
> > > -
> > > - ctx->x = x;
> > > - ctx->y = y;
> > > -}
> > > -EXPORT_SYMBOL_GPL(cifs_arc4_crypt);
> > > diff --git a/fs/smb/server/auth.c b/fs/smb/server/auth.c
> > > index 229a6527870d..5640196b313f 100644
> > > --- a/fs/smb/server/auth.c
> > > +++ b/fs/smb/server/auth.c
> > > @@ -29,7 +29,7 @@
> > > #include "mgmt/user_config.h"
> > > #include "crypto_ctx.h"
> > > #include "transport_ipc.h"
> > > -#include "../common/arc4.h"
> > > +#include <crypto/arc4.h>
> > >
> > > /*
> > > * Fixed format data defining GSS header and fixed string
> > > @@ -362,9 +362,9 @@ int ksmbd_decode_ntlmssp_auth_blob(struct authenticate_message *authblob,
> > > if (!ctx_arc4)
> > > return -ENOMEM;
> > >
> > > - cifs_arc4_setkey(ctx_arc4, sess->sess_key,
> > > + arc4_setkey(ctx_arc4, sess->sess_key,
> > > SMB2_NTLMV2_SESSKEY_SIZE);
> > > - cifs_arc4_crypt(ctx_arc4, sess->sess_key,
> > > + arc4_crypt(ctx_arc4, sess->sess_key,
> > > (char *)authblob + sess_key_off, sess_key_len);
> > > kfree_sensitive(ctx_arc4);
> > > }
> > > --
> > > 2.41.0
> > >
> >
> >
> > --
> > Thanks,
> >
> > Steve
>
>
>
> --
> Thanks,
>
> Steve
hello together,
i get the following kernel dump when i try mount a cifs drive:
[ 83.380977] CIFS: Attempting to mount //dtometzki.file.core.windows.net/sadata
[ 83.530165] ------------[ cut here ]------------
[ 83.530171] WARNING: CPU: 7 PID: 4584 at fs/smb/client/cifsglob.h:2165 smb2_get_aead_req+0x3fc/0x420 [cifs]
[ 83.530271] Modules linked in: nls_utf8 cifs cifs_arc4 nls_ucs2_utils cifs_md4 dns_resolver fscache netfs uinput rfcomm snd_seq_dummy snd_hrtimer nf_conntrack_netlink xt_addrtype br_netfilter xt_CHECKSUM xt_MASQUERADE xt_conntrack ipt_REJECT nf_nat_tftp nf_conntrack_tftp nf_conntrack_netbios_ns nf_conntrack_broadcast nft_fib_inet bridge nft_fib_ipv6 nft_fib_ipv4 nft_fib stp llc nft_reject_inet nf_reject_ipv6 nft_reject nf_reject_ipv4 nft_ct nft_chain_nat overlay ip6table_nat ip6table_mangle ip6table_raw ip6table_security iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 iptable_mangle iptable_raw iptable_security ip_set nf_tables nfnetlink ip6table_filter iptable_filter qrtr bnep sunrpc binfmt_misc snd_ctl_led snd_soc_skl_hda_dsp snd_soc_hdac_hdmi snd_sof_probes snd_soc_intel_hda_dsp_common snd_soc_dmic snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic snd_sof_pci_intel_tgl snd_sof_intel_hda_common snd_sof_intel_hda soundwire_intel snd_sof_intel_hda_mlink soundwire_generic_allocation
[ 83.530352] soundwire_cadence snd_sof_pci snd_sof_xtensa_dsp intel_tcc_cooling iTCO_wdt snd_sof x86_pkg_temp_thermal intel_pmc_bxt intel_powerclamp iTCO_vendor_support snd_sof_utils snd_soc_hdac_hda coretemp snd_hda_ext_core snd_soc_acpi_intel_match mei_hdcp mei_pxp kvm_intel snd_soc_acpi soundwire_bus snd_soc_core kvm snd_compress intel_rapl_msr ac97_bus vfat snd_pcm_dmaengine fat iwlmvm pmt_telemetry pmt_class snd_hda_intel irqbypass snd_intel_dspcfg snd_intel_sdw_acpi rapl mac80211 snd_hda_codec btusb snd_hda_core uvcvideo btbcm libarc4 snd_hwdep processor_thermal_device_pci_legacy uvc btintel videobuf2_v4l2 intel_cstate videobuf2_vmalloc snd_seq videobuf2_memops processor_thermal_device snd_seq_device btrtl processor_thermal_power_floor videobuf2_common processor_thermal_wt_req intel_uncore snd_pcm iwlwifi videodev i2c_i801 btmtk processor_thermal_wt_hint think_lmi mc firmware_attributes_class wmi_bmof processor_thermal_rfim thinkpad_acpi bluetooth snd_timer i2c_smbus mei_me processor_thermal_mbox cfg80211
[ 83.530438] ledtrig_audio processor_thermal_rapl idma64 mei platform_profile intel_rapl_common thunderbolt intel_vsec igen6_edac intel_soc_dts_iosf rfkill snd int3403_thermal soundcore soc_button_array int340x_thermal_zone int3400_thermal intel_hid acpi_thermal_rel acpi_pad sparse_keymap acpi_tad joydev squashfs loop zram i915 crct10dif_pclmul crc32_pclmul crc32c_intel drm_buddy polyval_clmulni ttm polyval_generic i2c_algo_bit drm_display_helper cec ghash_clmulni_intel hid_multitouch sha512_ssse3 video nvme sha256_ssse3 ucsi_acpi sha1_ssse3 typec_ucsi nvme_core i2c_hid_acpi typec i2c_hid wmi pinctrl_tigerlake serio_raw scsi_dh_rdac scsi_dh_emc scsi_dh_alua ip6_tables ip_tables dm_multipath fuse
[ 83.530503] CPU: 7 PID: 4584 Comm: mount.cifs Tainted: G W 6.6.0 #61
[ 83.530508] Hardware name: LENOVO 20XWCTO1WW/20XWCTO1WW, BIOS N32ET86W (1.62 ) 07/12/2023
[ 83.530511] RIP: 0010:smb2_get_aead_req+0x3fc/0x420 [cifs]
[ 83.530631] Code: 08 48 8b 44 24 10 48 8b 8c 24 a8 00 00 00 48 89 01 48 8b 44 24 30 48 83 c4 70 5b 41 5c 41 5d 41 5e 41 5f 5d c3 cc cc cc cc cc <0f> 0b 41 bc fb ff ff ff 44 89 e0 31 ff 8b 74 24 24 48 3d 00 f0 ff
[ 83.530636] RSP: 0018:ffffc90007893678 EFLAGS: 00010293
[ 83.530642] RAX: 0000000000000010 RBX: 0000000000000000 RCX: ffffc900078937b0
[ 83.530646] RDX: 0000000000000002 RSI: ffff88819f594038 RDI: ffff8881a5ad5040
[ 83.530649] RBP: 0000000000000000 R08: ffffc90007893740 R09: ffffc90007893758
[ 83.530652] R10: ffffc90007893760 R11: ffffffff81791d20 R12: 0000000000000002
[ 83.530655] R13: 0000000000000000 R14: 0000000000000014 R15: ffff88819f594038
[ 83.530658] FS: 00007f654431a780(0000) GS:ffff88844f7c0000(0000) knlGS:0000000000000000
[ 83.530663] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 83.530666] CR2: 00007ffdeb730008 CR3: 000000019d246001 CR4: 0000000000f70ef0
[ 83.530671] PKRU: 55555554
[ 83.530673] Call Trace:
[ 83.530680] <TASK>
[ 83.530686] ? __warn+0xc8/0x1c0
[ 83.530696] ? smb2_get_aead_req+0x3fc/0x420 [cifs]
[ 83.530807] ? report_bug+0x163/0x200
[ 83.530814] ? handle_bug+0x42/0x70
[ 83.530821] ? exc_invalid_op+0x1a/0x50
[ 83.530829] ? asm_exc_invalid_op+0x1a/0x20
[ 83.530837] ? __pfx_crypto_ccm_setauthsize+0x10/0x10
[ 83.530846] ? smb2_get_aead_req+0x3fc/0x420 [cifs]
[ 83.530951] ? aes_set_key+0x5c/0x90
[ 83.530961] crypt_message+0x33e/0x550 [cifs]
[ 83.531080] smb3_init_transform_rq+0x27d/0x3f0 [cifs]
[ 83.531202] ? smb_send_rqst+0x74/0x160 [cifs]
[ 83.531329] smb_send_rqst+0xc7/0x160 [cifs]
[ 83.531453] compound_send_recv+0x3ca/0x9f0 [cifs]
[ 83.531589] ? preempt_count_add+0x67/0xb0
[ 83.531599] ? _raw_spin_lock+0x1d/0x40
[ 83.531609] cifs_send_recv+0x25/0x30 [cifs]
[ 83.531730] SMB2_tcon+0x38a/0x820 [cifs]
[ 83.531854] ? preempt_count_add+0x67/0xb0
[ 83.531864] cifs_get_smb_ses+0x69c/0xee0 [cifs]
[ 83.531991] cifs_mount_get_session+0x76/0x1d0 [cifs]
[ 83.532113] dfs_mount_share+0x74/0x9d0 [cifs]
[ 83.532232] ? smb3_fs_context_dup+0x134/0x1e0 [cifs]
[ 83.532352] ? __kmem_cache_alloc_node+0x181/0x280
[ 83.532361] ? slab_post_alloc_hook+0x78/0x360
[ 83.532372] cifs_mount+0x6e/0x2e0 [cifs]
[ 83.532495] cifs_smb3_do_mount+0x143/0x300 [cifs]
[ 83.532638] smb3_get_tree+0x15e/0x290 [cifs]
[ 83.532759] vfs_get_tree+0x2d/0xe0
[ 83.532767] do_new_mount+0x124/0x340
[ 83.532779] __se_sys_mount+0x143/0x1a0
[ 83.532788] do_syscall_64+0x68/0x100
[ 83.532797] ? preempt_count_add+0x5a/0xb0
[ 83.532806] ? up_read+0x43/0xd0
[ 83.532815] ? do_user_addr_fault+0x220/0x790
[ 83.532826] ? exc_page_fault+0x7a/0x1b0
[ 83.532834] entry_SYSCALL_64_after_hwframe+0x6e/0x76
[ 83.532843] RIP: 0033:0x7f65444338ee
[ 83.532921] Code: 48 8b 0d 45 15 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 12 15 0c 00 f7 d8 64 89 01 48
[ 83.532926] RSP: 002b:00007fffc0fa5308 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
[ 83.532934] RAX: ffffffffffffffda RBX: 000000000000000a RCX: 00007f65444338ee
[ 83.532938] RDX: 000055641db9a476 RSI: 000055641db9a4dc RDI: 00007fffc0fa573d
[ 83.532941] RBP: 00007fffc0fa53c0 R08: 000055641f5dceb0 R09: 0000000000000000
[ 83.532945] R10: 0000000000000000 R11: 0000000000000246 R12: 000055641db9a03f
[ 83.532949] R13: 000055641f5ddf40 R14: 00007fffc0fa573d R15: 00007f6544520000
[ 83.532957] </TASK>
[ 83.532959] ---[ end trace 0000000000000000 ]---
[ 83.532967] BUG: unable to handle page fault for address: 0000001fffffff40
[ 83.532974] #PF: supervisor read access in kernel mode
[ 83.532980] #PF: error_code(0x0000) - not-present page
[ 83.532985] PGD 0 P4D 0
[ 83.532993] Oops: 0000 [#1] PREEMPT SMP NOPTI
[ 83.533000] CPU: 7 PID: 4584 Comm: mount.cifs Tainted: G W 6.6.0 #61
[ 83.533008] Hardware name: LENOVO 20XWCTO1WW/20XWCTO1WW, BIOS N32ET86W (1.62 ) 07/12/2023
[ 83.533011] RIP: 0010:smb2_get_aead_req+0x282/0x420 [cifs]
[ 83.533134] Code: 8d 3c 0a 48 83 c7 07 48 83 e7 f8 48 89 7c 24 10 48 3d 00 f0 ff ff 0f 87 74 01 00 00 48 89 44 24 30 41 8d 4c 24 ff 48 c1 e1 05 <48> 8b 14 0f 48 83 e2 fc 48 83 ca 02 48 89 14 0f 85 f6 0f 84 06 01
[ 83.533140] RSP: 0018:ffffc90007893678 EFLAGS: 00010202
[ 83.533147] RAX: 00000000fffffffb RBX: 0000000000000000 RCX: 0000001fffffff40
[ 83.533152] RDX: 0000000000000002 RSI: 0000000000000002 RDI: 0000000000000000
[ 83.533159] RBP: 0000000000000000 R08: ffffc90007893740 R09: ffffc90007893758
[ 83.533163] R10: ffffc90007893760 R11: ffffffff81791d20 R12: 00000000fffffffb
[ 83.533168] R13: 0000000000000000 R14: 0000000000000014 R15: ffff88819f594038
[ 83.533173] FS: 00007f654431a780(0000) GS:ffff88844f7c0000(0000) knlGS:0000000000000000
[ 83.533179] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 83.533184] CR2: 0000001fffffff40 CR3: 000000019d246001 CR4: 0000000000f70ef0
[ 83.533189] PKRU: 55555554
[ 83.533193] Call Trace:
[ 83.533197] <TASK>
[ 83.533201] ? __die_body+0x68/0xb0
[ 83.533211] ? page_fault_oops+0x388/0x3f0
[ 83.533224] ? exc_page_fault+0x7a/0x1b0
[ 83.533233] ? asm_exc_page_fault+0x26/0x30
[ 83.533243] ? __pfx_crypto_ccm_setauthsize+0x10/0x10
[ 83.533253] ? smb2_get_aead_req+0x282/0x420 [cifs]
[ 83.533376] ? aes_set_key+0x5c/0x90
[ 83.533387] crypt_message+0x33e/0x550 [cifs]
[ 83.533515] smb3_init_transform_rq+0x27d/0x3f0 [cifs]
[ 83.533652] ? smb_send_rqst+0x74/0x160 [cifs]
[ 83.533801] smb_send_rqst+0xc7/0x160 [cifs]
[ 83.533950] compound_send_recv+0x3ca/0x9f0 [cifs]
[ 83.534102] ? preempt_count_add+0x67/0xb0
[ 83.534112] ? _raw_spin_lock+0x1d/0x40
[ 83.534121] cifs_send_recv+0x25/0x30 [cifs]
[ 83.534229] SMB2_tcon+0x38a/0x820 [cifs]
[ 83.534354] ? preempt_count_add+0x67/0xb0
[ 83.534363] cifs_get_smb_ses+0x69c/0xee0 [cifs]
[ 83.534487] cifs_mount_get_session+0x76/0x1d0 [cifs]
[ 83.534610] dfs_mount_share+0x74/0x9d0 [cifs]
[ 83.534704] ? smb3_fs_context_dup+0x134/0x1e0 [cifs]
[ 83.534790] ? __kmem_cache_alloc_node+0x181/0x280
[ 83.534797] ? slab_post_alloc_hook+0x78/0x360
[ 83.534804] cifs_mount+0x6e/0x2e0 [cifs]
[ 83.534893] cifs_smb3_do_mount+0x143/0x300 [cifs]
[ 83.534981] smb3_get_tree+0x15e/0x290 [cifs]
[ 83.535069] vfs_get_tree+0x2d/0xe0
[ 83.535073] do_new_mount+0x124/0x340
[ 83.535080] __se_sys_mount+0x143/0x1a0
[ 83.535087] do_syscall_64+0x68/0x100
[ 83.535093] ? preempt_count_add+0x5a/0xb0
[ 83.535100] ? up_read+0x43/0xd0
[ 83.535106] ? do_user_addr_fault+0x220/0x790
[ 83.535113] ? exc_page_fault+0x7a/0x1b0
[ 83.535119] entry_SYSCALL_64_after_hwframe+0x6e/0x76
[ 83.535125] RIP: 0033:0x7f65444338ee
[ 83.535144] Code: 48 8b 0d 45 15 0c 00 f7 d8 64 89 01 48 83 c8 ff c3 66 2e 0f 1f 84 00 00 00 00 00 90 f3 0f 1e fa 49 89 ca b8 a5 00 00 00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 12 15 0c 00 f7 d8 64 89 01 48
[ 83.535148] RSP: 002b:00007fffc0fa5308 EFLAGS: 00000246 ORIG_RAX: 00000000000000a5
[ 83.535153] RAX: ffffffffffffffda RBX: 000000000000000a RCX: 00007f65444338ee
[ 83.535156] RDX: 000055641db9a476 RSI: 000055641db9a4dc RDI: 00007fffc0fa573d
[ 83.535158] RBP: 00007fffc0fa53c0 R08: 000055641f5dceb0 R09: 0000000000000000
[ 83.535161] R10: 0000000000000000 R11: 0000000000000246 R12: 000055641db9a03f
[ 83.535163] R13: 000055641f5ddf40 R14: 00007fffc0fa573d R15: 00007f6544520000
[ 83.535168] </TASK>
[ 83.535170] Modules linked in: nls_utf8 cifs cifs_arc4 nls_ucs2_utils cifs_md4 dns_resolver fscache netfs uinput rfcomm snd_seq_dummy snd_hrtimer nf_conntrack_netlink xt_addrtype br_netfilter xt_CHECKSUM xt_MASQUERADE xt_conntrack ipt_REJECT nf_nat_tftp nf_conntrack_tftp nf_conntrack_netbios_ns nf_conntrack_broadcast nft_fib_inet bridge nft_fib_ipv6 nft_fib_ipv4 nft_fib stp llc nft_reject_inet nf_reject_ipv6 nft_reject nf_reject_ipv4 nft_ct nft_chain_nat overlay ip6table_nat ip6table_mangle ip6table_raw ip6table_security iptable_nat nf_nat nf_conntrack nf_defrag_ipv6 nf_defrag_ipv4 iptable_mangle iptable_raw iptable_security ip_set nf_tables nfnetlink ip6table_filter iptable_filter qrtr bnep sunrpc binfmt_misc snd_ctl_led snd_soc_skl_hda_dsp snd_soc_hdac_hdmi snd_sof_probes snd_soc_intel_hda_dsp_common snd_soc_dmic snd_hda_codec_hdmi snd_hda_codec_realtek snd_hda_codec_generic snd_sof_pci_intel_tgl snd_sof_intel_hda_common snd_sof_intel_hda soundwire_intel snd_sof_intel_hda_mlink soundwire_generic_allocation
[ 83.535241] soundwire_cadence snd_sof_pci snd_sof_xtensa_dsp intel_tcc_cooling iTCO_wdt snd_sof x86_pkg_temp_thermal intel_pmc_bxt intel_powerclamp iTCO_vendor_support snd_sof_utils snd_soc_hdac_hda coretemp snd_hda_ext_core snd_soc_acpi_intel_match mei_hdcp mei_pxp kvm_intel snd_soc_acpi soundwire_bus snd_soc_core kvm snd_compress intel_rapl_msr ac97_bus vfat snd_pcm_dmaengine fat iwlmvm pmt_telemetry pmt_class snd_hda_intel irqbypass snd_intel_dspcfg snd_intel_sdw_acpi rapl mac80211 snd_hda_codec btusb snd_hda_core uvcvideo btbcm libarc4 snd_hwdep processor_thermal_device_pci_legacy uvc btintel videobuf2_v4l2 intel_cstate videobuf2_vmalloc snd_seq videobuf2_memops processor_thermal_device snd_seq_device btrtl processor_thermal_power_floor videobuf2_common processor_thermal_wt_req intel_uncore snd_pcm iwlwifi videodev i2c_i801 btmtk processor_thermal_wt_hint think_lmi mc firmware_attributes_class wmi_bmof processor_thermal_rfim thinkpad_acpi bluetooth snd_timer i2c_smbus mei_me processor_thermal_mbox cfg80211
[ 83.535316] ledtrig_audio processor_thermal_rapl idma64 mei platform_profile intel_rapl_common thunderbolt intel_vsec igen6_edac intel_soc_dts_iosf rfkill snd int3403_thermal soundcore soc_button_array int340x_thermal_zone int3400_thermal intel_hid acpi_thermal_rel acpi_pad sparse_keymap acpi_tad joydev squashfs loop zram i915 crct10dif_pclmul crc32_pclmul crc32c_intel drm_buddy polyval_clmulni ttm polyval_generic i2c_algo_bit drm_display_helper cec ghash_clmulni_intel hid_multitouch sha512_ssse3 video nvme sha256_ssse3 ucsi_acpi sha1_ssse3 typec_ucsi nvme_core i2c_hid_acpi typec i2c_hid wmi pinctrl_tigerlake serio_raw scsi_dh_rdac scsi_dh_emc scsi_dh_alua ip6_tables ip_tables dm_multipath fuse
[ 83.535379] CR2: 0000001fffffff40
[ 83.535383] ---[ end trace 0000000000000000 ]---
[ 83.535386] RIP: 0010:smb2_get_aead_req+0x282/0x420 [cifs]
[ 83.535473] Code: 8d 3c 0a 48 83 c7 07 48 83 e7 f8 48 89 7c 24 10 48 3d 00 f0 ff ff 0f 87 74 01 00 00 48 89 44 24 30 41 8d 4c 24 ff 48 c1 e1 05 <48> 8b 14 0f 48 83 e2 fc 48 83 ca 02 48 89 14 0f 85 f6 0f 84 06 01
[ 83.535476] RSP: 0018:ffffc90007893678 EFLAGS: 00010202
[ 83.535480] RAX: 00000000fffffffb RBX: 0000000000000000 RCX: 0000001fffffff40
[ 83.535483] RDX: 0000000000000002 RSI: 0000000000000002 RDI: 0000000000000000
[ 83.535485] RBP: 0000000000000000 R08: ffffc90007893740 R09: ffffc90007893758
[ 83.535487] R10: ffffc90007893760 R11: ffffffff81791d20 R12: 00000000fffffffb
[ 83.535490] R13: 0000000000000000 R14: 0000000000000014 R15: ffff88819f594038
[ 83.535493] FS: 00007f654431a780(0000) GS:ffff88844f7c0000(0000) knlGS:0000000000000000
[ 83.535496] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 83.535499] CR2: 0000001fffffff40 CR3: 000000019d246001 CR4: 0000000000f70ef0
[ 83.535502] PKRU: 55555554
[ 83.535504] note: mount.cifs[4584] exited with irqs disabled
On Mon, Oct 23, 2023 at 05:42:11AM +1000, ronnie sahlberg wrote:
> You are right. The reason that arc4 and friend were moved into cifs
> was because the crypto guys told us "we will delete these algorithms
> from the crypto library"
This was suggested for md4 but not for arc4. arc4 still has multiple users in
the kernel, so having it as a library makes sense.
- Eric
On Sun, Nov 05, 2023 at 11:05:30AM -0700, Steve French wrote:
> maybe related to this recent crypto patch?
>
> https://git.samba.org/?p=sfrench/cifs-2.6.git;a=commit;h=783fa2c94f4150fe1b7f7d88b3baf6d98f82b41b
>
> On Sun, Nov 5, 2023, 10:32 Damian Tometzki <[email protected]> wrote:
> > [ 83.530503] CPU: 7 PID: 4584 Comm: mount.cifs Tainted: G W
> > 6.6.0 #61
> > [ 83.530508] Hardware name: LENOVO 20XWCTO1WW/20XWCTO1WW, BIOS N32ET86W
> > (1.62 ) 07/12/2023
The above suggests that this warning occurred on 6.6, not on 6.7 pre rc1.
- Eric
On Sun, 05. Nov 11:36, Eric Biggers wrote:
> On Sun, Nov 05, 2023 at 11:05:30AM -0700, Steve French wrote:
> > maybe related to this recent crypto patch?
> >
> > https://git.samba.org/?p=sfrench/cifs-2.6.git;a=commit;h=783fa2c94f4150fe1b7f7d88b3baf6d98f82b41b
> >
> > On Sun, Nov 5, 2023, 10:32 Damian Tometzki <[email protected]> wrote:
> > > [ 83.530503] CPU: 7 PID: 4584 Comm: mount.cifs Tainted: G W
> > > 6.6.0 #61
> > > [ 83.530508] Hardware name: LENOVO 20XWCTO1WW/20XWCTO1WW, BIOS N32ET86W
> > > (1.62 ) 07/12/2023
>
> The above suggests that this warning occurred on 6.6, not on 6.7 pre rc1.
>
> - Eric
Hello,
is little bit missleading but it is 6.6 from linus mainline git with all
the pull request.
Damian
On Sun, Nov 05, 2023 at 08:40:03PM +0100, Damian Tometzki wrote:
> On Sun, 05. Nov 11:36, Eric Biggers wrote:
> > On Sun, Nov 05, 2023 at 11:05:30AM -0700, Steve French wrote:
> > > maybe related to this recent crypto patch?
> > >
> > > https://git.samba.org/?p=sfrench/cifs-2.6.git;a=commit;h=783fa2c94f4150fe1b7f7d88b3baf6d98f82b41b
> > >
> > > On Sun, Nov 5, 2023, 10:32 Damian Tometzki <[email protected]> wrote:
> > > > [ 83.530503] CPU: 7 PID: 4584 Comm: mount.cifs Tainted: G W
> > > > 6.6.0 #61
> > > > [ 83.530508] Hardware name: LENOVO 20XWCTO1WW/20XWCTO1WW, BIOS N32ET86W
> > > > (1.62 ) 07/12/2023
> >
> > The above suggests that this warning occurred on 6.6, not on 6.7 pre rc1.
> >
> > - Eric
> Hello,
>
> is little bit missleading but it is 6.6 from linus mainline git with all
> the pull request.
>
> Damian
>
Okay, next time please mention the actual commit ID. Anyway, the warning is
'WARN_ON_ONCE(user_backed_iter(&rqst[i].rq_iter))', so maybe take a look at
changes from
commit f1b4cb650b9a0eeba206d8f069fcdc532bfbcd74
Author: David Howells <[email protected]>
Date: Mon Sep 25 13:03:03 2023 +0100
iov_iter: Derive user-backedness from the iterator type
and the pull request that contained it:
commit df9c65b5fc7ef1caabdb7a01a2415cbb8a00908d
Merge: 3b3f874cc1d07 b5f0e20f444cd
Author: Linus Torvalds <[email protected]>
Date: Mon Oct 30 09:24:21 2023 -1000
Merge tag 'vfs-6.7.iov_iter' of gitolite.kernel.org:pub/scm/linux/kernel/git/vfs/vfs
Pull iov_iter updates from Christian Brauner:
"This contain's David's iov_iter cleanup work to convert the iov_iter
iteration macros to inline functions:
On Sun, 05. Nov 12:15, Eric Biggers wrote:
> On Sun, Nov 05, 2023 at 08:40:03PM +0100, Damian Tometzki wrote:
> > On Sun, 05. Nov 11:36, Eric Biggers wrote:
> > > On Sun, Nov 05, 2023 at 11:05:30AM -0700, Steve French wrote:
> > > > maybe related to this recent crypto patch?
> > > >
> > > > https://git.samba.org/?p=sfrench/cifs-2.6.git;a=commit;h=783fa2c94f4150fe1b7f7d88b3baf6d98f82b41b
> > > >
> > > > On Sun, Nov 5, 2023, 10:32 Damian Tometzki <[email protected]> wrote:
> > > > > [ 83.530503] CPU: 7 PID: 4584 Comm: mount.cifs Tainted: G W
> > > > > 6.6.0 #61
> > > > > [ 83.530508] Hardware name: LENOVO 20XWCTO1WW/20XWCTO1WW, BIOS N32ET86W
> > > > > (1.62 ) 07/12/2023
> > >
> > > The above suggests that this warning occurred on 6.6, not on 6.7 pre rc1.
> > >
> > > - Eric
> > Hello,
> >
> > is little bit missleading but it is 6.6 from linus mainline git with all
> > the pull request.
> >
> > Damian
> >
>
> Okay, next time please mention the actual commit ID. Anyway, the warning is
> 'WARN_ON_ONCE(user_backed_iter(&rqst[i].rq_iter))', so maybe take a look at
> changes from
>
> commit f1b4cb650b9a0eeba206d8f069fcdc532bfbcd74
> Author: David Howells <[email protected]>
> Date: Mon Sep 25 13:03:03 2023 +0100
>
> iov_iter: Derive user-backedness from the iterator type
>
Hello Eric,
the revert of f1b4cb650b9a0eeba206d8f069fcdc532bfbcd74 solved the issue of the kernel dump.
diff --git a/include/linux/uio.h b/include/linux/uio.h
index b6214cbf2a43..02a8e5e6c458 100644
--- a/include/linux/uio.h
+++ b/include/linux/uio.h
@@ -43,6 +43,7 @@ struct iov_iter {
bool copy_mc;
bool nofault;
bool data_source;
+ bool user_backed;
size_t iov_offset;
/*
* Hack alert: overlay ubuf_iovec with iovec + count, so
@@ -139,7 +140,7 @@ static inline unsigned char iov_iter_rw(const struct iov_iter *i)
static inline bool user_backed_iter(const struct iov_iter *i)
{
- return iter_is_ubuf(i) || iter_is_iovec(i);
+ return i->user_backed;
}
/*
@@ -358,6 +359,7 @@ static inline void iov_iter_ubuf(struct iov_iter *i, unsigned int direction,
*i = (struct iov_iter) {
.iter_type = ITER_UBUF,
.copy_mc = false,
+ .user_backed = true,
.data_source = direction,
.ubuf = buf,
.count = count,
diff --git a/lib/iov_iter.c b/lib/iov_iter.c
index de7d11cf4c63..a077c15727b2 100644
--- a/lib/iov_iter.c
+++ b/lib/iov_iter.c
@@ -168,6 +168,7 @@ void iov_iter_init(struct iov_iter *i, unsigned int direction,
.iter_type = ITER_IOVEC,
.copy_mc = false,
.nofault = false,
+ .user_backed = true,
.data_source = direction,
.__iov = iov,
.nr_segs = nr_segs,
> and the pull request that contained it:
>
> commit df9c65b5fc7ef1caabdb7a01a2415cbb8a00908d
> Merge: 3b3f874cc1d07 b5f0e20f444cd
> Author: Linus Torvalds <[email protected]>
> Date: Mon Oct 30 09:24:21 2023 -1000
>
> Merge tag 'vfs-6.7.iov_iter' of gitolite.kernel.org:pub/scm/linux/kernel/git/vfs/vfs
>
> Pull iov_iter updates from Christian Brauner:
> "This contain's David's iov_iter cleanup work to convert the iov_iter
> iteration macros to inline functions:
Damian Tometzki <[email protected]> wrote:
> the revert of f1b4cb650b9a0eeba206d8f069fcdc532bfbcd74 solved the issue of
> the kernel dump.
That almost certainly did not fix the problem - merely hid the wanring.
Prior to f1b4cb650b9a0eeba206d8f069fcdc532bfbcd74, ->user_backed is explicitly
set if the iov_iter is initialised to a user-backed type, now it's just
inferred from the type being 0 or 1 - so I think that the iov_iter has not
been initialised somewhere.
Somewhere being from SMB2_tcon() and cifs_send_recv() on down.
David
Hi Damian,
Does the attached fix it for you?
David
---
cifs: Fix encryption of cleared, but unset rq_iter data buffers
Each smb_rqst struct contains two things: an array of kvecs (rq_iov) that
contains the protocol data for an RPC op and an iterator (rq_iter) that
contains the data payload of an RPC op. When an smb_rqst is allocated
rq_iter is it always cleared, but we don't set it up unless we're going to
use it.
The functions that determines the size of the ciphertext buffer that will
be needed to encrypt a request, cifs_get_num_sgs(), assumes that rq_iter is
always initialised - and employs user_backed_iter() to check that the
iterator isn't user-backed. This used to incidentally work, because
->user_backed was set to false because the iterator has never been
initialised, but with commit f1b4cb650b9a0eeba206d8f069fcdc532bfbcd74[1]
which changes user_backed_iter() to determine this based on the iterator
type insted, a warning is now emitted:
WARNING: CPU: 7 PID: 4584 at fs/smb/client/cifsglob.h:2165 smb2_get_aead_req+0x3fc/0x420 [cifs]
...
RIP: 0010:smb2_get_aead_req+0x3fc/0x420 [cifs]
...
crypt_message+0x33e/0x550 [cifs]
smb3_init_transform_rq+0x27d/0x3f0 [cifs]
smb_send_rqst+0xc7/0x160 [cifs]
compound_send_recv+0x3ca/0x9f0 [cifs]
cifs_send_recv+0x25/0x30 [cifs]
SMB2_tcon+0x38a/0x820 [cifs]
cifs_get_smb_ses+0x69c/0xee0 [cifs]
cifs_mount_get_session+0x76/0x1d0 [cifs]
dfs_mount_share+0x74/0x9d0 [cifs]
cifs_mount+0x6e/0x2e0 [cifs]
cifs_smb3_do_mount+0x143/0x300 [cifs]
smb3_get_tree+0x15e/0x290 [cifs]
vfs_get_tree+0x2d/0xe0
do_new_mount+0x124/0x340
__se_sys_mount+0x143/0x1a0
The problem is that rq_iter was never set, so the type is 0 (ie. ITER_UBUF)
which causes user_backed_iter() to return true. The code doesn't
malfunction because it checks the size of the iterator - which is 0.
Fix cifs_get_num_sgs() to ignore rq_iter if its count is 0, thereby
bypassing the warnings.
It might be better to explicitly initialise rq_iter to a zero-length
ITER_BVEC, say, as it can always be reinitialised later.
Fixes: d08089f649a0 ("cifs: Change the I/O paths to use an iterator rather than a page list")
Reported-by: Damian Tometzki <[email protected]>
Link: https://lore.kernel.org/r/[email protected]/
Signed-off-by: David Howells <[email protected]>
cc: Steve French <[email protected]>
cc: Shyam Prasad N <[email protected]>
cc: Rohith Surabattula <[email protected]>
cc: Paulo Alcantara <[email protected]>
cc: Namjae Jeon <[email protected]>
cc: Tom Talpey <[email protected]>
cc: Jeff Layton <[email protected]>
cc: Eric Biggers <[email protected]>
cc: [email protected]
cc: [email protected]
Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f1b4cb650b9a0eeba206d8f069fcdc532bfbcd74 [1]
---
fs/smb/client/cifsglob.h | 12 +++++++++---
1 file changed, 9 insertions(+), 3 deletions(-)
diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
index 02082621d8e0..c70760871606 100644
--- a/fs/smb/client/cifsglob.h
+++ b/fs/smb/client/cifsglob.h
@@ -2143,6 +2143,7 @@ static inline int cifs_get_num_sgs(const struct smb_rqst *rqst,
unsigned int len, skip;
unsigned int nents = 0;
unsigned long addr;
+ size_t data_size;
int i, j;
/*
@@ -2158,17 +2159,21 @@ static inline int cifs_get_num_sgs(const struct smb_rqst *rqst,
* rqst[1+].rq_iov[0+] data to be encrypted/decrypted
*/
for (i = 0; i < num_rqst; i++) {
+ data_size = iov_iter_count(&rqst[i].rq_iter);
+
/* We really don't want a mixture of pinned and unpinned pages
* in the sglist. It's hard to keep track of which is what.
* Instead, we convert to a BVEC-type iterator higher up.
*/
- if (WARN_ON_ONCE(user_backed_iter(&rqst[i].rq_iter)))
+ if (data_size &&
+ WARN_ON_ONCE(user_backed_iter(&rqst[i].rq_iter)))
return -EIO;
/* We also don't want to have any extra refs or pins to clean
* up in the sglist.
*/
- if (WARN_ON_ONCE(iov_iter_extract_will_pin(&rqst[i].rq_iter)))
+ if (data_size &&
+ WARN_ON_ONCE(iov_iter_extract_will_pin(&rqst[i].rq_iter)))
return -EIO;
for (j = 0; j < rqst[i].rq_nvec; j++) {
@@ -2184,7 +2189,8 @@ static inline int cifs_get_num_sgs(const struct smb_rqst *rqst,
}
skip = 0;
}
- nents += iov_iter_npages(&rqst[i].rq_iter, INT_MAX);
+ if (data_size)
+ nents += iov_iter_npages(&rqst[i].rq_iter, INT_MAX);
}
nents += DIV_ROUND_UP(offset_in_page(sig) + SMB2_SIGNATURE_SIZE, PAGE_SIZE);
return nents;
David Howells <[email protected]> writes:
> cifs: Fix encryption of cleared, but unset rq_iter data buffers
>
> Each smb_rqst struct contains two things: an array of kvecs (rq_iov) that
> contains the protocol data for an RPC op and an iterator (rq_iter) that
> contains the data payload of an RPC op. When an smb_rqst is allocated
> rq_iter is it always cleared, but we don't set it up unless we're going to
> use it.
>
> The functions that determines the size of the ciphertext buffer that will
> be needed to encrypt a request, cifs_get_num_sgs(), assumes that rq_iter is
> always initialised - and employs user_backed_iter() to check that the
> iterator isn't user-backed. This used to incidentally work, because
> ->user_backed was set to false because the iterator has never been
> initialised, but with commit f1b4cb650b9a0eeba206d8f069fcdc532bfbcd74[1]
> which changes user_backed_iter() to determine this based on the iterator
> type insted, a warning is now emitted:
>
> WARNING: CPU: 7 PID: 4584 at fs/smb/client/cifsglob.h:2165 smb2_get_aead_req+0x3fc/0x420 [cifs]
> ...
> RIP: 0010:smb2_get_aead_req+0x3fc/0x420 [cifs]
> ...
> crypt_message+0x33e/0x550 [cifs]
> smb3_init_transform_rq+0x27d/0x3f0 [cifs]
> smb_send_rqst+0xc7/0x160 [cifs]
> compound_send_recv+0x3ca/0x9f0 [cifs]
> cifs_send_recv+0x25/0x30 [cifs]
> SMB2_tcon+0x38a/0x820 [cifs]
> cifs_get_smb_ses+0x69c/0xee0 [cifs]
> cifs_mount_get_session+0x76/0x1d0 [cifs]
> dfs_mount_share+0x74/0x9d0 [cifs]
> cifs_mount+0x6e/0x2e0 [cifs]
> cifs_smb3_do_mount+0x143/0x300 [cifs]
> smb3_get_tree+0x15e/0x290 [cifs]
> vfs_get_tree+0x2d/0xe0
> do_new_mount+0x124/0x340
> __se_sys_mount+0x143/0x1a0
>
> The problem is that rq_iter was never set, so the type is 0 (ie. ITER_UBUF)
> which causes user_backed_iter() to return true. The code doesn't
> malfunction because it checks the size of the iterator - which is 0.
>
> Fix cifs_get_num_sgs() to ignore rq_iter if its count is 0, thereby
> bypassing the warnings.
>
> It might be better to explicitly initialise rq_iter to a zero-length
> ITER_BVEC, say, as it can always be reinitialised later.
Agreed. We could probably set those in ->init_transform_rq().
> Fixes: d08089f649a0 ("cifs: Change the I/O paths to use an iterator rather than a page list")
> Reported-by: Damian Tometzki <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]/
> Signed-off-by: David Howells <[email protected]>
> cc: Steve French <[email protected]>
> cc: Shyam Prasad N <[email protected]>
> cc: Rohith Surabattula <[email protected]>
> cc: Paulo Alcantara <[email protected]>
> cc: Namjae Jeon <[email protected]>
> cc: Tom Talpey <[email protected]>
> cc: Jeff Layton <[email protected]>
> cc: Eric Biggers <[email protected]>
> cc: [email protected]
> cc: [email protected]
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f1b4cb650b9a0eeba206d8f069fcdc532bfbcd74 [1]
> ---
> fs/smb/client/cifsglob.h | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
FWIW, the patch fixes mounts of shares with encryption enabled.
Reviewed-by: Paulo Alcantara (SUSE) <[email protected]>
On Mon, 06. Nov 14:40, David Howells wrote:
> Hi Damian,
>
> Does the attached fix it for you?
Hello David,
this fix my issue with the cifs mount.
Great many Thanks
Damian
>
> David
> ---
> cifs: Fix encryption of cleared, but unset rq_iter data buffers
>
> Each smb_rqst struct contains two things: an array of kvecs (rq_iov) that
> contains the protocol data for an RPC op and an iterator (rq_iter) that
> contains the data payload of an RPC op. When an smb_rqst is allocated
> rq_iter is it always cleared, but we don't set it up unless we're going to
> use it.
>
> The functions that determines the size of the ciphertext buffer that will
> be needed to encrypt a request, cifs_get_num_sgs(), assumes that rq_iter is
> always initialised - and employs user_backed_iter() to check that the
> iterator isn't user-backed. This used to incidentally work, because
> ->user_backed was set to false because the iterator has never been
> initialised, but with commit f1b4cb650b9a0eeba206d8f069fcdc532bfbcd74[1]
> which changes user_backed_iter() to determine this based on the iterator
> type insted, a warning is now emitted:
>
> WARNING: CPU: 7 PID: 4584 at fs/smb/client/cifsglob.h:2165 smb2_get_aead_req+0x3fc/0x420 [cifs]
> ...
> RIP: 0010:smb2_get_aead_req+0x3fc/0x420 [cifs]
> ...
> crypt_message+0x33e/0x550 [cifs]
> smb3_init_transform_rq+0x27d/0x3f0 [cifs]
> smb_send_rqst+0xc7/0x160 [cifs]
> compound_send_recv+0x3ca/0x9f0 [cifs]
> cifs_send_recv+0x25/0x30 [cifs]
> SMB2_tcon+0x38a/0x820 [cifs]
> cifs_get_smb_ses+0x69c/0xee0 [cifs]
> cifs_mount_get_session+0x76/0x1d0 [cifs]
> dfs_mount_share+0x74/0x9d0 [cifs]
> cifs_mount+0x6e/0x2e0 [cifs]
> cifs_smb3_do_mount+0x143/0x300 [cifs]
> smb3_get_tree+0x15e/0x290 [cifs]
> vfs_get_tree+0x2d/0xe0
> do_new_mount+0x124/0x340
> __se_sys_mount+0x143/0x1a0
>
> The problem is that rq_iter was never set, so the type is 0 (ie. ITER_UBUF)
> which causes user_backed_iter() to return true. The code doesn't
> malfunction because it checks the size of the iterator - which is 0.
>
> Fix cifs_get_num_sgs() to ignore rq_iter if its count is 0, thereby
> bypassing the warnings.
>
> It might be better to explicitly initialise rq_iter to a zero-length
> ITER_BVEC, say, as it can always be reinitialised later.
>
> Fixes: d08089f649a0 ("cifs: Change the I/O paths to use an iterator rather than a page list")
> Reported-by: Damian Tometzki <[email protected]>
> Link: https://lore.kernel.org/r/[email protected]/
> Signed-off-by: David Howells <[email protected]>
> cc: Steve French <[email protected]>
> cc: Shyam Prasad N <[email protected]>
> cc: Rohith Surabattula <[email protected]>
> cc: Paulo Alcantara <[email protected]>
> cc: Namjae Jeon <[email protected]>
> cc: Tom Talpey <[email protected]>
> cc: Jeff Layton <[email protected]>
> cc: Eric Biggers <[email protected]>
> cc: [email protected]
> cc: [email protected]
> Link: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=f1b4cb650b9a0eeba206d8f069fcdc532bfbcd74 [1]
> ---
> fs/smb/client/cifsglob.h | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h
> index 02082621d8e0..c70760871606 100644
> --- a/fs/smb/client/cifsglob.h
> +++ b/fs/smb/client/cifsglob.h
> @@ -2143,6 +2143,7 @@ static inline int cifs_get_num_sgs(const struct smb_rqst *rqst,
> unsigned int len, skip;
> unsigned int nents = 0;
> unsigned long addr;
> + size_t data_size;
> int i, j;
>
> /*
> @@ -2158,17 +2159,21 @@ static inline int cifs_get_num_sgs(const struct smb_rqst *rqst,
> * rqst[1+].rq_iov[0+] data to be encrypted/decrypted
> */
> for (i = 0; i < num_rqst; i++) {
> + data_size = iov_iter_count(&rqst[i].rq_iter);
> +
> /* We really don't want a mixture of pinned and unpinned pages
> * in the sglist. It's hard to keep track of which is what.
> * Instead, we convert to a BVEC-type iterator higher up.
> */
> - if (WARN_ON_ONCE(user_backed_iter(&rqst[i].rq_iter)))
> + if (data_size &&
> + WARN_ON_ONCE(user_backed_iter(&rqst[i].rq_iter)))
> return -EIO;
>
> /* We also don't want to have any extra refs or pins to clean
> * up in the sglist.
> */
> - if (WARN_ON_ONCE(iov_iter_extract_will_pin(&rqst[i].rq_iter)))
> + if (data_size &&
> + WARN_ON_ONCE(iov_iter_extract_will_pin(&rqst[i].rq_iter)))
> return -EIO;
>
> for (j = 0; j < rqst[i].rq_nvec; j++) {
> @@ -2184,7 +2189,8 @@ static inline int cifs_get_num_sgs(const struct smb_rqst *rqst,
> }
> skip = 0;
> }
> - nents += iov_iter_npages(&rqst[i].rq_iter, INT_MAX);
> + if (data_size)
> + nents += iov_iter_npages(&rqst[i].rq_iter, INT_MAX);
> }
> nents += DIV_ROUND_UP(offset_in_page(sig) + SMB2_SIGNATURE_SIZE, PAGE_SIZE);
> return nents;
>