[PATCH 1/2] dm crypt: replaced #if defined with IS_ENABLED

[PATCH 1/2] dm crypt: replaced #if defined with IS_ENABLED

[Ahmad Fatoum]

IS_ENABLED(CONFIG_ENCRYPTED_KEYS) is true whether the option is built-in
or a module, so use it instead of #if defined checking for each
separately.

The other #if was to avoid a static function defined, but unused
warning. As we now always build the callsite when the function
is defined, we can remove that first #if guard.

Suggested-by: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
Cc: Dmitry Baryshkov <dbaryshkov@gmail.com>
---
 drivers/md/dm-crypt.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)

diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 8c874710f0bc..7eeb9248eda5 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -2436,7 +2436,6 @@ static int set_key_user(struct crypt_config *cc, struct key *key)
 	return 0;
 }
 
-#if defined(CONFIG_ENCRYPTED_KEYS) || defined(CONFIG_ENCRYPTED_KEYS_MODULE)
 static int set_key_encrypted(struct crypt_config *cc, struct key *key)
 {
 	const struct encrypted_key_payload *ekp;
@@ -2452,7 +2451,6 @@ static int set_key_encrypted(struct crypt_config *cc, struct key *key)
 
 	return 0;
 }
-#endif /* CONFIG_ENCRYPTED_KEYS */
 
 static int crypt_set_keyring_key(struct crypt_config *cc, const char *key_string)
 {
@@ -2482,11 +2480,10 @@ static int crypt_set_keyring_key(struct crypt_config *cc, const char *key_string
 	} else if (!strncmp(key_string, "user:", key_desc - key_string + 1)) {
 		type = &key_type_user;
 		set_key = set_key_user;
-#if defined(CONFIG_ENCRYPTED_KEYS) || defined(CONFIG_ENCRYPTED_KEYS_MODULE)
-	} else if (!strncmp(key_string, "encrypted:", key_desc - key_string + 1)) {
+	} else if (IS_ENABLED(CONFIG_ENCRYPTED_KEYS) &&
+		   !strncmp(key_string, "encrypted:", key_desc - key_string + 1)) {
 		type = &key_type_encrypted;
 		set_key = set_key_encrypted;
-#endif
 	} else {
 		return -EINVAL;
 	}
-- 
2.30.0

[PATCH 2/2] dm crypt: support using trusted keys

[Ahmad Fatoum]

Commit 27f5411a718c ("dm crypt: support using encrypted keys") extended
dm-crypt to allow use of "encrypted" keys along with "user" and "logon".

Along the same lines, teach dm-crypt to support "trusted" keys as well.

Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
---
Unsure on whether target_type::version is something authors increment or
maintainers fix up. I can respin if needed.

Cc: Jan Lübbe <jlu@pengutronix.de>
Cc: linux-integrity@vger.kernel.org
Cc: keyrings@vger.kernel.org
Cc: Dmitry Baryshkov <dbaryshkov@gmail.com>
---
 .../admin-guide/device-mapper/dm-crypt.rst    |  2 +-
 drivers/md/Kconfig                            |  1 +
 drivers/md/dm-crypt.c                         | 23 ++++++++++++++++++-
 3 files changed, 24 insertions(+), 2 deletions(-)

diff --git a/Documentation/admin-guide/device-mapper/dm-crypt.rst b/Documentation/admin-guide/device-mapper/dm-crypt.rst
index 1a6753b76dbb..aa2d04d95df6 100644
--- a/Documentation/admin-guide/device-mapper/dm-crypt.rst
+++ b/Documentation/admin-guide/device-mapper/dm-crypt.rst
@@ -67,7 +67,7 @@ Parameters::
     the value passed in <key_size>.
 
 <key_type>
-    Either 'logon', 'user' or 'encrypted' kernel key type.
+    Either 'logon', 'user', 'encrypted' or 'trusted' kernel key type.
 
 <key_description>
     The kernel keyring key description crypt target should look for
diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
index 9e44c09f6410..f2014385d48b 100644
--- a/drivers/md/Kconfig
+++ b/drivers/md/Kconfig
@@ -270,6 +270,7 @@ config DM_CRYPT
 	tristate "Crypt target support"
 	depends on BLK_DEV_DM
 	depends on (ENCRYPTED_KEYS || ENCRYPTED_KEYS=n)
+	depends on (TRUSTED_KEYS || TRUSTED_KEYS=n)
 	select CRYPTO
 	select CRYPTO_CBC
 	select CRYPTO_ESSIV
diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
index 7eeb9248eda5..6c7c687e546c 100644
--- a/drivers/md/dm-crypt.c
+++ b/drivers/md/dm-crypt.c
@@ -37,6 +37,7 @@
 #include <linux/key-type.h>
 #include <keys/user-type.h>
 #include <keys/encrypted-type.h>
+#include <keys/trusted-type.h>
 
 #include <linux/device-mapper.h>
 
@@ -2452,6 +2453,22 @@ static int set_key_encrypted(struct crypt_config *cc, struct key *key)
 	return 0;
 }
 
+static int set_key_trusted(struct crypt_config *cc, struct key *key)
+{
+	const struct trusted_key_payload *tkp;
+
+	tkp = key->payload.data[0];
+	if (!tkp)
+		return -EKEYREVOKED;
+
+	if (cc->key_size != tkp->key_len)
+		return -EINVAL;
+
+	memcpy(cc->key, tkp->key, cc->key_size);
+
+	return 0;
+}
+
 static int crypt_set_keyring_key(struct crypt_config *cc, const char *key_string)
 {
 	char *new_key_string, *key_desc;
@@ -2484,6 +2501,10 @@ static int crypt_set_keyring_key(struct crypt_config *cc, const char *key_string
 		   !strncmp(key_string, "encrypted:", key_desc - key_string + 1)) {
 		type = &key_type_encrypted;
 		set_key = set_key_encrypted;
+	} else if (IS_ENABLED(CONFIG_TRUSTED_KEYS) &&
+	           !strncmp(key_string, "trusted:", key_desc - key_string + 1)) {
+		type = &key_type_trusted;
+		set_key = set_key_trusted;
 	} else {
 		return -EINVAL;
 	}
@@ -3555,7 +3576,7 @@ static void crypt_io_hints(struct dm_target *ti, struct queue_limits *limits)
 
 static struct target_type crypt_target = {
 	.name   = "crypt",
-	.version = {1, 22, 0},
+	.version = {1, 23, 0},
 	.module = THIS_MODULE,
 	.ctr    = crypt_ctr,
 	.dtr    = crypt_dtr,
-- 
2.30.0

Re: [PATCH 2/2] dm crypt: support using trusted keys

[Jarkko Sakkinen]

On Fri, Jan 22, 2021 at 09:43:21AM +0100, Ahmad Fatoum wrote:
> Commit 27f5411a718c ("dm crypt: support using encrypted keys") extended
> dm-crypt to allow use of "encrypted" keys along with "user" and "logon".
> 
> Along the same lines, teach dm-crypt to support "trusted" keys as well.
> 
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---

Is it possible to test run this with tmpfs? Would be a good test
target for Sumit's ARM-TEE trusted keys patches.

https://lore.kernel.org/linux-integrity/1604419306-26105-1-git-send-email-sumit.garg@linaro.org/

/Jarkko

> Unsure on whether target_type::version is something authors increment or
> maintainers fix up. I can respin if needed.
> 
> Cc: Jan Lübbe <jlu@pengutronix.de>
> Cc: linux-integrity@vger.kernel.org
> Cc: keyrings@vger.kernel.org
> Cc: Dmitry Baryshkov <dbaryshkov@gmail.com>
> ---
>  .../admin-guide/device-mapper/dm-crypt.rst    |  2 +-
>  drivers/md/Kconfig                            |  1 +
>  drivers/md/dm-crypt.c                         | 23 ++++++++++++++++++-
>  3 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/admin-guide/device-mapper/dm-crypt.rst b/Documentation/admin-guide/device-mapper/dm-crypt.rst
> index 1a6753b76dbb..aa2d04d95df6 100644
> --- a/Documentation/admin-guide/device-mapper/dm-crypt.rst
> +++ b/Documentation/admin-guide/device-mapper/dm-crypt.rst
> @@ -67,7 +67,7 @@ Parameters::
>      the value passed in <key_size>.
>  
>  <key_type>
> -    Either 'logon', 'user' or 'encrypted' kernel key type.
> +    Either 'logon', 'user', 'encrypted' or 'trusted' kernel key type.
>  
>  <key_description>
>      The kernel keyring key description crypt target should look for
> diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig
> index 9e44c09f6410..f2014385d48b 100644
> --- a/drivers/md/Kconfig
> +++ b/drivers/md/Kconfig
> @@ -270,6 +270,7 @@ config DM_CRYPT
>  	tristate "Crypt target support"
>  	depends on BLK_DEV_DM
>  	depends on (ENCRYPTED_KEYS || ENCRYPTED_KEYS=n)
> +	depends on (TRUSTED_KEYS || TRUSTED_KEYS=n)
>  	select CRYPTO
>  	select CRYPTO_CBC
>  	select CRYPTO_ESSIV
> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> index 7eeb9248eda5..6c7c687e546c 100644
> --- a/drivers/md/dm-crypt.c
> +++ b/drivers/md/dm-crypt.c
> @@ -37,6 +37,7 @@
>  #include <linux/key-type.h>
>  #include <keys/user-type.h>
>  #include <keys/encrypted-type.h>
> +#include <keys/trusted-type.h>
>  
>  #include <linux/device-mapper.h>
>  
> @@ -2452,6 +2453,22 @@ static int set_key_encrypted(struct crypt_config *cc, struct key *key)
>  	return 0;
>  }
>  
> +static int set_key_trusted(struct crypt_config *cc, struct key *key)
> +{
> +	const struct trusted_key_payload *tkp;
> +
> +	tkp = key->payload.data[0];
> +	if (!tkp)
> +		return -EKEYREVOKED;
> +
> +	if (cc->key_size != tkp->key_len)
> +		return -EINVAL;
> +
> +	memcpy(cc->key, tkp->key, cc->key_size);
> +
> +	return 0;
> +}
> +
>  static int crypt_set_keyring_key(struct crypt_config *cc, const char *key_string)
>  {
>  	char *new_key_string, *key_desc;
> @@ -2484,6 +2501,10 @@ static int crypt_set_keyring_key(struct crypt_config *cc, const char *key_string
>  		   !strncmp(key_string, "encrypted:", key_desc - key_string + 1)) {
>  		type = &key_type_encrypted;
>  		set_key = set_key_encrypted;
> +	} else if (IS_ENABLED(CONFIG_TRUSTED_KEYS) &&
> +	           !strncmp(key_string, "trusted:", key_desc - key_string + 1)) {
> +		type = &key_type_trusted;
> +		set_key = set_key_trusted;
>  	} else {
>  		return -EINVAL;
>  	}
> @@ -3555,7 +3576,7 @@ static void crypt_io_hints(struct dm_target *ti, struct queue_limits *limits)
>  
>  static struct target_type crypt_target = {
>  	.name   = "crypt",
> -	.version = {1, 22, 0},
> +	.version = {1, 23, 0},
>  	.module = THIS_MODULE,
>  	.ctr    = crypt_ctr,
>  	.dtr    = crypt_dtr,
> -- 
> 2.30.0
> 
> 

Re: [PATCH 2/2] dm crypt: support using trusted keys

[Jarkko Sakkinen]

On Fri, Jan 22, 2021 at 08:05:51PM +0200, Jarkko Sakkinen wrote:
> On Fri, Jan 22, 2021 at 09:43:21AM +0100, Ahmad Fatoum wrote:
> > Commit 27f5411a718c ("dm crypt: support using encrypted keys") extended
> > dm-crypt to allow use of "encrypted" keys along with "user" and "logon".
> > 
> > Along the same lines, teach dm-crypt to support "trusted" keys as well.
> > 
> > Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> > ---
> 
> Is it possible to test run this with tmpfs? Would be a good test
> target for Sumit's ARM-TEE trusted keys patches.
> 
> https://lore.kernel.org/linux-integrity/1604419306-26105-1-git-send-email-sumit.garg@linaro.org/

Also, I would hold merging *this* patch up until we are able to
test TEE trusted keys with TEE trusted keys.

/Jarkko

Re: [PATCH 2/2] dm crypt: support using trusted keys

[Ahmad Fatoum]

Hello Jarkko,

On 22.01.21 19:18, Jarkko Sakkinen wrote:
> On Fri, Jan 22, 2021 at 08:05:51PM +0200, Jarkko Sakkinen wrote:
>> On Fri, Jan 22, 2021 at 09:43:21AM +0100, Ahmad Fatoum wrote:
>>> Commit 27f5411a718c ("dm crypt: support using encrypted keys") extended
>>> dm-crypt to allow use of "encrypted" keys along with "user" and "logon".
>>>
>>> Along the same lines, teach dm-crypt to support "trusted" keys as well.
>>>
>>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>> ---
>>
>> Is it possible to test run this with tmpfs? Would be a good test
>> target for Sumit's ARM-TEE trusted keys patches.

I tested these on top of Sumit's patches with TPM and a CAAM blobifier
backend, I am preparing. The system I am developing these patches against
doesn't have a TEE.  Steps to test these changes:

#!/bin/sh

DEV=/dev/loop0
ALGO=aes-cbc-essiv:sha256
KEYNAME=kmk
BLOCKS=20

fallocate -l $((BLOCKS*512)) /tmp/loop0.img
losetup -P $DEV /tmp/loop0.img
mount -o remount,rw /
KEY="$(keyctl add trusted $KEYNAME 'new 32' @s)"
keyctl pipe $KEY >$HOME/kmk.blob

TABLE="0 $BLOCKS crypt $ALGO :32:trusted:$KEYNAME 0 $DEV 0 1 allow_discards"
echo $TABLE | dmsetup create mydev
echo $TABLE | dmsetup load mydev
dd if=/dev/zero of=/dev/mapper/mydev
echo "It works!" 1<> /dev/mapper/mydev
cryptsetup close mydev

reboot

DEV=/dev/loop0
ALGO=aes-cbc-essiv:sha256
KEYNAME=kmk
BLOCKS=20

losetup -P $DEV $HOME/loop0.img
keyctl add trusted $KEYNAME "load $(cat $HOME/kmk.blob)" @s
TABLE="0 $BLOCKS crypt $ALGO :32:trusted:$KEYNAME 0 $DEV 0 1 allow_discards"
echo $TABLE | dmsetup create mydev
echo $TABLE | dmsetup load mydev

# should print that It works!
hexdump -C /dev/mapper/mydev

>> https://lore.kernel.org/linux-integrity/1604419306-26105-1-git-send-email-sumit.garg@linaro.org/
> 
> Also, I would hold merging *this* patch up until we are able to
> test TEE trusted keys with TEE trusted keys.

Which blocks which? I tested this with TPM-Trusted keys, so it's usable
as is. For convenient usage, it would be nice to have cryptsetup
support for trusted and encrypted keys. I intended to look at this next week.

Cheers,
Ahmad

> 
> /Jarkko
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 2/2] dm crypt: support using trusted keys

[Ahmad Fatoum]

On 22.01.21 20:04, Ahmad Fatoum wrote:
> On 22.01.21 19:18, Jarkko Sakkinen wrote:
>> On Fri, Jan 22, 2021 at 08:05:51PM +0200, Jarkko Sakkinen wrote:
>>> On Fri, Jan 22, 2021 at 09:43:21AM +0100, Ahmad Fatoum wrote:
>>>> Commit 27f5411a718c ("dm crypt: support using encrypted keys") extended
>>>> dm-crypt to allow use of "encrypted" keys along with "user" and "logon".
>>>>
>>>> Along the same lines, teach dm-crypt to support "trusted" keys as well.

Gentle ping.
Is there anything further you require from me regarding these two patches?

>>>>
>>>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>>>> ---
>>>
>>> Is it possible to test run this with tmpfs? Would be a good test
>>> target for Sumit's ARM-TEE trusted keys patches.
> 
> I tested these on top of Sumit's patches with TPM and a CAAM blobifier
> backend, I am preparing. The system I am developing these patches against
> doesn't have a TEE.  Steps to test these changes:
> 
> #!/bin/sh
> 
> DEV=/dev/loop0
> ALGO=aes-cbc-essiv:sha256
> KEYNAME=kmk
> BLOCKS=20
> 
> fallocate -l $((BLOCKS*512)) /tmp/loop0.img
> losetup -P $DEV /tmp/loop0.img
> mount -o remount,rw /
> KEY="$(keyctl add trusted $KEYNAME 'new 32' @s)"
> keyctl pipe $KEY >$HOME/kmk.blob
> 
> TABLE="0 $BLOCKS crypt $ALGO :32:trusted:$KEYNAME 0 $DEV 0 1 allow_discards"
> echo $TABLE | dmsetup create mydev
> echo $TABLE | dmsetup load mydev
> dd if=/dev/zero of=/dev/mapper/mydev
> echo "It works!" 1<> /dev/mapper/mydev
> cryptsetup close mydev
> 
> reboot
> 
> DEV=/dev/loop0
> ALGO=aes-cbc-essiv:sha256
> KEYNAME=kmk
> BLOCKS=20
> 
> losetup -P $DEV $HOME/loop0.img
> keyctl add trusted $KEYNAME "load $(cat $HOME/kmk.blob)" @s
> TABLE="0 $BLOCKS crypt $ALGO :32:trusted:$KEYNAME 0 $DEV 0 1 allow_discards"
> echo $TABLE | dmsetup create mydev
> echo $TABLE | dmsetup load mydev
> 
> # should print that It works!
> hexdump -C /dev/mapper/mydev
> 
>>> https://lore.kernel.org/linux-integrity/1604419306-26105-1-git-send-email-sumit.garg@linaro.org/
>>
>> Also, I would hold merging *this* patch up until we are able to
>> test TEE trusted keys with TEE trusted keys.
> 
> Which blocks which? I tested this with TPM-Trusted keys, so it's usable
> as is. For convenient usage, it would be nice to have cryptsetup
> support for trusted and encrypted keys. I intended to look at this next week.
> 
> Cheers,
> Ahmad
> 
>>
>> /Jarkko
>>
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/2] dm crypt: replaced #if defined with IS_ENABLED

[Mike Snitzer]

On Fri, Jan 22 2021 at  3:43am -0500,
Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:

> IS_ENABLED(CONFIG_ENCRYPTED_KEYS) is true whether the option is built-in
> or a module, so use it instead of #if defined checking for each
> separately.
> 
> The other #if was to avoid a static function defined, but unused
> warning. As we now always build the callsite when the function
> is defined, we can remove that first #if guard.
> 
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
> ---
> Cc: Dmitry Baryshkov <dbaryshkov@gmail.com>
> ---
>  drivers/md/dm-crypt.c | 7 ++-----
>  1 file changed, 2 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
> index 8c874710f0bc..7eeb9248eda5 100644
> --- a/drivers/md/dm-crypt.c
> +++ b/drivers/md/dm-crypt.c
> @@ -2436,7 +2436,6 @@ static int set_key_user(struct crypt_config *cc, struct key *key)
>  	return 0;
>  }
>  
> -#if defined(CONFIG_ENCRYPTED_KEYS) || defined(CONFIG_ENCRYPTED_KEYS_MODULE)
>  static int set_key_encrypted(struct crypt_config *cc, struct key *key)
>  {
>  	const struct encrypted_key_payload *ekp;
> @@ -2452,7 +2451,6 @@ static int set_key_encrypted(struct crypt_config *cc, struct key *key)
>  
>  	return 0;
>  }
> -#endif /* CONFIG_ENCRYPTED_KEYS */
>  
>  static int crypt_set_keyring_key(struct crypt_config *cc, const char *key_string)
>  {
> @@ -2482,11 +2480,10 @@ static int crypt_set_keyring_key(struct crypt_config *cc, const char *key_string
>  	} else if (!strncmp(key_string, "user:", key_desc - key_string + 1)) {
>  		type = &key_type_user;
>  		set_key = set_key_user;
> -#if defined(CONFIG_ENCRYPTED_KEYS) || defined(CONFIG_ENCRYPTED_KEYS_MODULE)
> -	} else if (!strncmp(key_string, "encrypted:", key_desc - key_string + 1)) {
> +	} else if (IS_ENABLED(CONFIG_ENCRYPTED_KEYS) &&
> +		   !strncmp(key_string, "encrypted:", key_desc - key_string + 1)) {
>  		type = &key_type_encrypted;
>  		set_key = set_key_encrypted;
> -#endif
>  	} else {
>  		return -EINVAL;
>  	}
> -- 
> 2.30.0
> 

I could be mistaken but the point of the previous way used to enable
this code was to not compile the code at all.  How you have it, the
branch just isn't taken but the compiled code is left to bloat dm-crypt.

Why not leave this as is and follow same pattern in your next patch?

Mike

Re: [PATCH 1/2] dm crypt: replaced #if defined with IS_ENABLED

[Ahmad Fatoum]

Hello Mike,

On 02.02.21 19:10, Mike Snitzer wrote:
> On Fri, Jan 22 2021 at  3:43am -0500,
> Ahmad Fatoum <a.fatoum@pengutronix.de> wrote:
> 
>> IS_ENABLED(CONFIG_ENCRYPTED_KEYS) is true whether the option is built-in
>> or a module, so use it instead of #if defined checking for each
>> separately.
>>
>> The other #if was to avoid a static function defined, but unused
>> warning. As we now always build the callsite when the function
>> is defined, we can remove that first #if guard.
>>
>> Suggested-by: Arnd Bergmann <arnd@arndb.de>
>> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>
>> ---
>> Cc: Dmitry Baryshkov <dbaryshkov@gmail.com>
>> ---
>>  drivers/md/dm-crypt.c | 7 ++-----
>>  1 file changed, 2 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/md/dm-crypt.c b/drivers/md/dm-crypt.c
>> index 8c874710f0bc..7eeb9248eda5 100644
>> --- a/drivers/md/dm-crypt.c
>> +++ b/drivers/md/dm-crypt.c
>> @@ -2436,7 +2436,6 @@ static int set_key_user(struct crypt_config *cc, struct key *key)
>>  	return 0;
>>  }
>>  
>> -#if defined(CONFIG_ENCRYPTED_KEYS) || defined(CONFIG_ENCRYPTED_KEYS_MODULE)
>>  static int set_key_encrypted(struct crypt_config *cc, struct key *key)
>>  {
>>  	const struct encrypted_key_payload *ekp;
>> @@ -2452,7 +2451,6 @@ static int set_key_encrypted(struct crypt_config *cc, struct key *key)
>>  
>>  	return 0;
>>  }
>> -#endif /* CONFIG_ENCRYPTED_KEYS */
>>  
>>  static int crypt_set_keyring_key(struct crypt_config *cc, const char *key_string)
>>  {
>> @@ -2482,11 +2480,10 @@ static int crypt_set_keyring_key(struct crypt_config *cc, const char *key_string
>>  	} else if (!strncmp(key_string, "user:", key_desc - key_string + 1)) {
>>  		type = &key_type_user;
>>  		set_key = set_key_user;
>> -#if defined(CONFIG_ENCRYPTED_KEYS) || defined(CONFIG_ENCRYPTED_KEYS_MODULE)
>> -	} else if (!strncmp(key_string, "encrypted:", key_desc - key_string + 1)) {
>> +	} else if (IS_ENABLED(CONFIG_ENCRYPTED_KEYS) &&
>> +		   !strncmp(key_string, "encrypted:", key_desc - key_string + 1)) {
>>  		type = &key_type_encrypted;
>>  		set_key = set_key_encrypted;
>> -#endif
>>  	} else {
>>  		return -EINVAL;
>>  	}
>> -- 
>> 2.30.0
>>
> 
> I could be mistaken but the point of the previous way used to enable
> this code was to not compile the code at all.  How you have it, the
> branch just isn't taken but the compiled code is left to bloat dm-crypt.
> 
> Why not leave this as is and follow same pattern in your next patch?

It's safe to assume the compiler will constant-fold the resulting if (0) away.
The kernel coding style documentation got a section on that:
https://www.kernel.org/doc/html/v5.11-rc6/process/coding-style.html#conditional-compilation

Cheers,
Ahmad

> 
> Mike
> 
> 
> 

-- 
Pengutronix e.K.                           |                             |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-5555 |

Re: [PATCH 1/2] dm crypt: replaced #if defined with IS_ENABLED

[Dmitry Baryshkov]

пт, 22 янв. 2021 г. в 11:43, Ahmad Fatoum <a.fatoum@pengutronix.de>:
>
> IS_ENABLED(CONFIG_ENCRYPTED_KEYS) is true whether the option is built-in
> or a module, so use it instead of #if defined checking for each
> separately.
>
> The other #if was to avoid a static function defined, but unused
> warning. As we now always build the callsite when the function
> is defined, we can remove that first #if guard.
>
> Suggested-by: Arnd Bergmann <arnd@arndb.de>
> Signed-off-by: Ahmad Fatoum <a.fatoum@pengutronix.de>

Acked-by: Dmitry Baryshkov <dmitry.baryshkov@linaro.org>


-- 
With best wishes
Dmitry