There is currently a large ifdef FIPS code section in proc.c.
Ostensibly it's there because the fips_enabled sysctl sits under
/proc/sys/crypto. However, no other crypto sysctls exist.
In fact, the whole ethos of the crypto API is against such user
interfaces so this patch moves all the FIPS sysctl code over to
fips.c.
Signed-off-by: Herbert Xu <[email protected]>
diff --git a/crypto/fips.c b/crypto/fips.c
index 0f65df9..9d627c1 100644
--- a/crypto/fips.c
+++ b/crypto/fips.c
@@ -13,7 +13,9 @@
#include <linux/export.h>
#include <linux/fips.h>
#include <linux/init.h>
+#include <linux/module.h>
#include <linux/kernel.h>
+#include <linux/sysctl.h>
int fips_enabled;
EXPORT_SYMBOL_GPL(fips_enabled);
@@ -28,3 +30,49 @@ static int fips_enable(char *str)
}
__setup("fips=", fips_enable);
+
+static struct ctl_table crypto_sysctl_table[] = {
+ {
+ .procname = "fips_enabled",
+ .data = &fips_enabled,
+ .maxlen = sizeof(int),
+ .mode = 0444,
+ .proc_handler = proc_dointvec
+ },
+ {}
+};
+
+static struct ctl_table crypto_dir_table[] = {
+ {
+ .procname = "crypto",
+ .mode = 0555,
+ .child = crypto_sysctl_table
+ },
+ {}
+};
+
+static struct ctl_table_header *crypto_sysctls;
+
+static void crypto_proc_fips_init(void)
+{
+ crypto_sysctls = register_sysctl_table(crypto_dir_table);
+}
+
+static void crypto_proc_fips_exit(void)
+{
+ unregister_sysctl_table(crypto_sysctls);
+}
+
+static int __init fips_init(void)
+{
+ crypto_proc_fips_init();
+ return 0;
+}
+
+static void __exit fips_exit(void)
+{
+ crypto_proc_fips_exit();
+}
+
+module_init(fips_init);
+module_exit(fips_exit);
diff --git a/crypto/proc.c b/crypto/proc.c
index 4ffe73b..2cc10c9 100644
--- a/crypto/proc.c
+++ b/crypto/proc.c
@@ -20,47 +20,8 @@
#include <linux/rwsem.h>
#include <linux/proc_fs.h>
#include <linux/seq_file.h>
-#include <linux/sysctl.h>
#include "internal.h"
-#ifdef CONFIG_CRYPTO_FIPS
-static struct ctl_table crypto_sysctl_table[] = {
- {
- .procname = "fips_enabled",
- .data = &fips_enabled,
- .maxlen = sizeof(int),
- .mode = 0444,
- .proc_handler = proc_dointvec
- },
- {}
-};
-
-static struct ctl_table crypto_dir_table[] = {
- {
- .procname = "crypto",
- .mode = 0555,
- .child = crypto_sysctl_table
- },
- {}
-};
-
-static struct ctl_table_header *crypto_sysctls;
-
-static void crypto_proc_fips_init(void)
-{
- crypto_sysctls = register_sysctl_table(crypto_dir_table);
-}
-
-static void crypto_proc_fips_exit(void)
-{
- if (crypto_sysctls)
- unregister_sysctl_table(crypto_sysctls);
-}
-#else
-#define crypto_proc_fips_init()
-#define crypto_proc_fips_exit()
-#endif
-
static void *c_start(struct seq_file *m, loff_t *pos)
{
down_read(&crypto_alg_sem);
@@ -148,11 +109,9 @@ static const struct file_operations proc_crypto_ops = {
void __init crypto_init_proc(void)
{
proc_create("crypto", 0, NULL, &proc_crypto_ops);
- crypto_proc_fips_init();
}
void __exit crypto_exit_proc(void)
{
- crypto_proc_fips_exit();
remove_proc_entry("crypto", NULL);
}
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Am Mittwoch, 22. April 2015, 13:02:22 schrieb Herbert Xu:
Hi Herbert,
> There is currently a large ifdef FIPS code section in proc.c.
> Ostensibly it's there because the fips_enabled sysctl sits under
> /proc/sys/crypto. However, no other crypto sysctls exist.
>
> In fact, the whole ethos of the crypto API is against such user
> interfaces so this patch moves all the FIPS sysctl code over to
> fips.c.
>
> Signed-off-by: Herbert Xu <[email protected]>
>
> diff --git a/crypto/fips.c b/crypto/fips.c
> index 0f65df9..9d627c1 100644
> --- a/crypto/fips.c
> +++ b/crypto/fips.c
> @@ -13,7 +13,9 @@
> #include <linux/export.h>
> #include <linux/fips.h>
> #include <linux/init.h>
> +#include <linux/module.h>
> #include <linux/kernel.h>
> +#include <linux/sysctl.h>
>
> int fips_enabled;
> EXPORT_SYMBOL_GPL(fips_enabled);
> @@ -28,3 +30,49 @@ static int fips_enable(char *str)
> }
>
> __setup("fips=", fips_enable);
> +
> +static struct ctl_table crypto_sysctl_table[] = {
> + {
> + .procname = "fips_enabled",
> + .data = &fips_enabled,
> + .maxlen = sizeof(int),
> + .mode = 0444,
> + .proc_handler = proc_dointvec
> + },
> + {}
> +};
> +
> +static struct ctl_table crypto_dir_table[] = {
> + {
> + .procname = "crypto",
> + .mode = 0555,
> + .child = crypto_sysctl_table
> + },
> + {}
> +};
> +
> +static struct ctl_table_header *crypto_sysctls;
> +
> +static void crypto_proc_fips_init(void)
> +{
> + crypto_sysctls = register_sysctl_table(crypto_dir_table);
> +}
> +
> +static void crypto_proc_fips_exit(void)
> +{
> + unregister_sysctl_table(crypto_sysctls);
> +}
> +
> +static int __init fips_init(void)
> +{
> + crypto_proc_fips_init();
> + return 0;
> +}
> +
> +static void __exit fips_exit(void)
> +{
> + crypto_proc_fips_exit();
> +}
> +
> +module_init(fips_init);
> +module_exit(fips_exit);
> diff --git a/crypto/proc.c b/crypto/proc.c
> index 4ffe73b..2cc10c9 100644
> --- a/crypto/proc.c
> +++ b/crypto/proc.c
> @@ -20,47 +20,8 @@
> #include <linux/rwsem.h>
> #include <linux/proc_fs.h>
> #include <linux/seq_file.h>
> -#include <linux/sysctl.h>
> #include "internal.h"
>
> -#ifdef CONFIG_CRYPTO_FIPS
> -static struct ctl_table crypto_sysctl_table[] = {
> - {
> - .procname = "fips_enabled",
> - .data = &fips_enabled,
> - .maxlen = sizeof(int),
> - .mode = 0444,
> - .proc_handler = proc_dointvec
> - },
> - {}
> -};
> -
> -static struct ctl_table crypto_dir_table[] = {
> - {
> - .procname = "crypto",
> - .mode = 0555,
> - .child = crypto_sysctl_table
> - },
> - {}
> -};
> -
> -static struct ctl_table_header *crypto_sysctls;
> -
> -static void crypto_proc_fips_init(void)
> -{
> - crypto_sysctls = register_sysctl_table(crypto_dir_table);
> -}
> -
> -static void crypto_proc_fips_exit(void)
> -{
> - if (crypto_sysctls)
> - unregister_sysctl_table(crypto_sysctls);
> -}
> -#else
> -#define crypto_proc_fips_init()
> -#define crypto_proc_fips_exit()
> -#endif
> -
> static void *c_start(struct seq_file *m, loff_t *pos)
> {
> down_read(&crypto_alg_sem);
> @@ -148,11 +109,9 @@ static const struct file_operations proc_crypto_ops = {
> void __init crypto_init_proc(void)
> {
> proc_create("crypto", 0, NULL, &proc_crypto_ops);
> - crypto_proc_fips_init();
> }
>
> void __exit crypto_exit_proc(void)
> {
> - crypto_proc_fips_exit();
> remove_proc_entry("crypto", NULL);
> }
With this removal of crypto_proc_fips_* from crypto_*_proc, wouldn't you have
broken the link from algapi.c? There, crypto_*_proc is called where now the
FIPS logic may not be initialized any more.
--
Ciao
Stephan
On Wed, Apr 22, 2015 at 02:33:03PM +0200, Stephan Mueller wrote:
>
> With this removal of crypto_proc_fips_* from crypto_*_proc, wouldn't you have
> broken the link from algapi.c? There, crypto_*_proc is called where now the
> FIPS logic may not be initialized any more.
This code does not provide the fips_enabled variable. All it does
is export it as a sysctl. So it has nothing to do with the crypto
API.
Cheers,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On 04/22/2015 07:02 AM, Herbert Xu wrote:
> There is currently a large ifdef FIPS code section in proc.c.
> Ostensibly it's there because the fips_enabled sysctl sits under
> /proc/sys/crypto. However, no other crypto sysctls exist.
>
> In fact, the whole ethos of the crypto API is against such user
> interfaces so this patch moves all the FIPS sysctl code over to
> fips.c.
>
> Signed-off-by: Herbert Xu <[email protected]>
...
>
> int fips_enabled;
> EXPORT_SYMBOL_GPL(fips_enabled);
While you're at it, this should also be marked as __read_mostly.
Thanks,
Daniel