* [RFC PATCH 0/1] dm-integrity: support keys in the kernel keyring @ 2026-09-28 6:27 Lorenz Kofler 2026-09-28 6:27 ` [RFC PATCH 1/1] " Lorenz Kofler 0 siblings, 1 reply; 8+ messages in thread From: Lorenz Kofler @ 2026-09-28 6:27 UTC (permalink / raw) To: Mikulas Patocka, Mike Snitzer, Benjamin Marzinski, Alasdair Kergon Cc: dm-devel, linux-kernel, upstream+dm, Lorenz Kofler dm-integrity currently accepts keys only as hex strings in the table, so keys that never leave the kernel, such as trusted keys, cannot be used. This patch adds support for retrieving keys from the kernel keyring using the same format as dm-crypt: :<key_size>:<key_type>:<key_description> The keyring lookup code is copied from dm-crypt's crypt_set_keyring_key() and its helpers. I would prefer to share this code between dm-crypt and dm-integrity rather than duplicate it, but I am not sure what the best way is. I see only two options: 1. static inline helpers in a drivers/md header, so dm-crypt and dm-integrity each compile their own copy 2. a small library module, similar to dm-bufio, so there is one copy that follows the value (y/m) of dm-crypt and dm-integrity Putting the helpers into dm-mod does not work. The helpers use key_type_encrypted and key_type_trusted, which can be modules. With e.g., BLK_DEV_DM=y, DM_CRYPT=m and ENCRYPTED_KEYS=m, built-in dm-mod would reference a module symbol, and vmlinux fails to link. Please let me know which approach you would prefer, or if there is a better way to share this code. Lorenz Kofler (1): dm-integrity: support keys in the kernel keyring .../device-mapper/dm-integrity.rst | 25 +++ drivers/md/Kconfig | 2 + drivers/md/dm-integrity.c | 164 ++++++++++++++++++ 3 files changed, 191 insertions(+) base-commit: f0100363d8c374bd8e9ea7c9ba02744f0b802ca4 -- 2.55.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* [RFC PATCH 1/1] dm-integrity: support keys in the kernel keyring 2026-09-28 6:27 [RFC PATCH 0/1] dm-integrity: support keys in the kernel keyring Lorenz Kofler @ 2026-09-28 6:27 ` Lorenz Kofler 2026-09-30 15:00 ` Mikulas Patocka 0 siblings, 1 reply; 8+ messages in thread From: Lorenz Kofler @ 2026-09-28 6:27 UTC (permalink / raw) To: Mikulas Patocka, Mike Snitzer, Benjamin Marzinski, Alasdair Kergon Cc: dm-devel, linux-kernel, upstream+dm, Lorenz Kofler The keys of internal_hash, journal_crypt and journal_mac can only be given as hex strings in the table. Keys that never leave the kernel, such as trusted keys, cannot be used. In addition to hex strings, accept keys from the kernel keyring in the same format as dm-crypt: ":<key_size>:<key_type>:<key_description>". The lookup is the same as in dm-crypt. Like DM_CRYPT, DM_INTEGRITY must not be built in when the encrypted or trusted key types are modules. Signed-off-by: Lorenz Kofler <lorenz@sigma-star.at> --- .../device-mapper/dm-integrity.rst | 25 +++ drivers/md/Kconfig | 2 + drivers/md/dm-integrity.c | 164 ++++++++++++++++++ 3 files changed, 191 insertions(+) diff --git a/Documentation/admin-guide/device-mapper/dm-integrity.rst b/Documentation/admin-guide/device-mapper/dm-integrity.rst index 9c21301423c9..2b7f82912962 100644 --- a/Documentation/admin-guide/device-mapper/dm-integrity.rst +++ b/Documentation/admin-guide/device-mapper/dm-integrity.rst @@ -146,6 +146,8 @@ internal_hash:algorithm(:key) (the key is optional) from an upper layer target, such as dm-crypt. The upper layer target should check the validity of the integrity tags. + The format of the key is described below the list of arguments. + recalculate Recalculate the integrity tags automatically. It is only valid when using internal hash. @@ -162,6 +164,8 @@ journal_crypt:algorithm(:key) (the key is optional) the size of files that were written. To protect against this situation, you can encrypt the journal. + The format of the key is described below the list of arguments. + journal_mac:algorithm(:key) (the key is optional) Protect sector numbers in the journal from accidental or malicious modification. To protect against accidental modification, use a @@ -173,6 +177,8 @@ journal_mac:algorithm(:key) (the key is optional) the journal. Thus, modified sector number would be detected at this stage. + The format of the key is described below the list of arguments. + block_size:number (default 512) The size of a data block in bytes. The larger the block size the less overhead there is for per-block integrity metadata. @@ -225,6 +231,25 @@ legacy_recalculate set recalc_sector to zero, and the kernel would not detect the modification. +The key of internal_hash, journal_crypt and journal_mac is encoded either as +a hexadecimal number or it can be passed as <key_string> prefixed with single +colon character (':') for keys residing in kernel keyring service. + +<key_string> + The kernel keyring key is identified by string in following format: + <key_size>:<key_type>:<key_description>. + +<key_size> + The key size in bytes. The kernel key payload size must match + the value passed in <key_size>. + +<key_type> + Either 'logon', 'user', 'encrypted' or 'trusted' kernel key type. + +<key_description> + The kernel keyring key description integrity target should look for + when loading key of <key_type>. + The journal mode (D/J), buffer_sectors, journal_watermark, commit_time and allow_discards can be changed when reloading the target (load an inactive table and swap the tables with suspend and resume). The other arguments diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig index df27c7d066d2..e93bf0c3119d 100644 --- a/drivers/md/Kconfig +++ b/drivers/md/Kconfig @@ -653,6 +653,8 @@ config DM_LOG_WRITES config DM_INTEGRITY tristate "Integrity target support" depends on BLK_DEV_DM + depends on (ENCRYPTED_KEYS || ENCRYPTED_KEYS=n) + depends on (TRUSTED_KEYS || TRUSTED_KEYS=n) select BLK_DEV_INTEGRITY select DM_BUFIO select CRYPTO diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c index 92970e12267a..6995c870cbf5 100644 --- a/drivers/md/dm-integrity.c +++ b/drivers/md/dm-integrity.c @@ -25,6 +25,11 @@ #include <crypto/utils.h> #include <linux/async_tx.h> #include <linux/dm-bufio.h> +#include <linux/ctype.h> +#include <linux/key.h> +#include <keys/user-type.h> +#include <keys/encrypted-type.h> +#include <keys/trusted-type.h> #include "dm-audit.h" @@ -4415,6 +4420,152 @@ static void free_alg(struct alg_spec *a) memset(a, 0, sizeof(*a)); } +#ifdef CONFIG_KEYS + +static bool contains_whitespace(const char *str) +{ + while (*str) + if (isspace(*str++)) + return true; + return false; +} + +static int set_key_user(struct alg_spec *a, struct key *key) +{ + const struct user_key_payload *ukp; + + ukp = user_key_payload_locked(key); + if (!ukp) + return -EKEYREVOKED; + + if (a->key_size != ukp->datalen) + return -EINVAL; + + memcpy(a->key, ukp->data, a->key_size); + + return 0; +} + +static int set_key_encrypted(struct alg_spec *a, struct key *key) +{ + const struct encrypted_key_payload *ekp; + + ekp = key->payload.data[0]; + if (!ekp) + return -EKEYREVOKED; + + if (a->key_size != ekp->decrypted_datalen) + return -EINVAL; + + memcpy(a->key, ekp->decrypted_data, a->key_size); + + return 0; +} + +static int set_key_trusted(struct alg_spec *a, struct key *key) +{ + const struct trusted_key_payload *tkp; + + tkp = key->payload.data[0]; + if (!tkp) + return -EKEYREVOKED; + + if (a->key_size != tkp->key_len) + return -EINVAL; + + memcpy(a->key, tkp->key, a->key_size); + + return 0; +} + +/* + * Load the key from the kernel keyring. a->key_string has the format + * ":<key_size>:<key_type>:<key_description>". + */ +static int get_alg_key_from_keyring(struct alg_spec *a) +{ + int (*set_key)(struct alg_spec *a, struct key *key); + const char *key_type_desc, *key_desc; + struct key_type *type; + unsigned int key_size; + struct key *key; + char dummy; + int r; + + /* + * Reject key_string with whitespace. dm core currently lacks code for + * proper whitespace escaping in arguments on DM_TABLE_STATUS path. + */ + if (contains_whitespace(a->key_string)) + return -EINVAL; + + if (sscanf(a->key_string, ":%u%c", &key_size, &dummy) != 2 || dummy != ':') + return -EINVAL; + if (!key_size || key_size > KMALLOC_MAX_SIZE) + return -EINVAL; + + /* skip ":<key_size>:", the sscanf() above guarantees the second ':' */ + key_type_desc = strchr(a->key_string + 1, ':') + 1; + + /* look for next ':' separating key_type from key_description */ + key_desc = strchr(key_type_desc, ':'); + if (!key_desc || key_desc == key_type_desc || !strlen(key_desc + 1)) + return -EINVAL; + + if (!strncmp(key_type_desc, "logon:", key_desc - key_type_desc + 1)) { + type = &key_type_logon; + set_key = set_key_user; + } else if (!strncmp(key_type_desc, "user:", key_desc - key_type_desc + 1)) { + type = &key_type_user; + set_key = set_key_user; + } else if (IS_ENABLED(CONFIG_ENCRYPTED_KEYS) && + !strncmp(key_type_desc, "encrypted:", key_desc - key_type_desc + 1)) { + type = &key_type_encrypted; + set_key = set_key_encrypted; + } else if (IS_ENABLED(CONFIG_TRUSTED_KEYS) && + !strncmp(key_type_desc, "trusted:", key_desc - key_type_desc + 1)) { + type = &key_type_trusted; + set_key = set_key_trusted; + } else { + return -EINVAL; + } + + a->key = kmalloc(key_size, GFP_KERNEL); + if (!a->key) + return -ENOMEM; + a->key_size = key_size; + + key = request_key(type, key_desc + 1, NULL); + if (IS_ERR(key)) { + r = PTR_ERR(key); + goto free_key; + } + + down_read(&key->sem); + r = set_key(a, key); + up_read(&key->sem); + key_put(key); + if (r < 0) + goto free_key; + + return 0; + +free_key: + kfree_sensitive(a->key); + a->key = NULL; + a->key_size = 0; + return r; +} + +#else + +static int get_alg_key_from_keyring(struct alg_spec *a) +{ + return -EINVAL; +} + +#endif /* CONFIG_KEYS */ + static int get_alg_and_key(const char *arg, struct alg_spec *a, char **error, char *error_inval) { char *k; @@ -4429,6 +4580,19 @@ static int get_alg_and_key(const char *arg, struct alg_spec *a, char **error, ch if (k) { *k = 0; a->key_string = k + 1; + + if (a->key_string[0] == ':') { + int r = get_alg_key_from_keyring(a); + + if (r == -ENOMEM) + goto nomem; + if (r == -EINVAL) + goto inval; + if (r) + *error = "Cannot get key from the kernel keyring"; + return r; + } + if (strlen(a->key_string) & 1) goto inval; -- 2.55.0 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 1/1] dm-integrity: support keys in the kernel keyring 2026-09-28 6:27 ` [RFC PATCH 1/1] " Lorenz Kofler @ 2026-09-30 15:00 ` Mikulas Patocka 2026-10-02 9:12 ` Lorenz Kofler 0 siblings, 1 reply; 8+ messages in thread From: Mikulas Patocka @ 2026-09-30 15:00 UTC (permalink / raw) To: Lorenz Kofler Cc: Mike Snitzer, Benjamin Marzinski, Alasdair Kergon, dm-devel, linux-kernel, upstream+dm Hi Generally, the approach in this patch is OK. Here I have some comments for the patch: Mikulas The patch should increase target version - from "{1, 15, 0}" to "{1, 16, 0}". On Mon, 28 Sep 2026, Lorenz Kofler wrote: > The keys of internal_hash, journal_crypt and journal_mac can only be > given as hex strings in the table. Keys that never leave the kernel, > such as trusted keys, cannot be used. > > In addition to hex strings, accept keys from the kernel keyring in the > same format as dm-crypt: ":<key_size>:<key_type>:<key_description>". > The lookup is the same as in dm-crypt. > > Like DM_CRYPT, DM_INTEGRITY must not be built in when the encrypted or > trusted key types are modules. > > Signed-off-by: Lorenz Kofler <lorenz@sigma-star.at> > --- > .../device-mapper/dm-integrity.rst | 25 +++ > drivers/md/Kconfig | 2 + > drivers/md/dm-integrity.c | 164 ++++++++++++++++++ > 3 files changed, 191 insertions(+) > > diff --git a/Documentation/admin-guide/device-mapper/dm-integrity.rst b/Documentation/admin-guide/device-mapper/dm-integrity.rst > index 9c21301423c9..2b7f82912962 100644 > --- a/Documentation/admin-guide/device-mapper/dm-integrity.rst > +++ b/Documentation/admin-guide/device-mapper/dm-integrity.rst > @@ -146,6 +146,8 @@ internal_hash:algorithm(:key) (the key is optional) > from an upper layer target, such as dm-crypt. The upper layer > target should check the validity of the integrity tags. > > + The format of the key is described below the list of arguments. > + > recalculate > Recalculate the integrity tags automatically. It is only valid > when using internal hash. > @@ -162,6 +164,8 @@ journal_crypt:algorithm(:key) (the key is optional) > the size of files that were written. To protect against this > situation, you can encrypt the journal. > > + The format of the key is described below the list of arguments. > + > journal_mac:algorithm(:key) (the key is optional) > Protect sector numbers in the journal from accidental or malicious > modification. To protect against accidental modification, use a > @@ -173,6 +177,8 @@ journal_mac:algorithm(:key) (the key is optional) > the journal. Thus, modified sector number would be detected at > this stage. > > + The format of the key is described below the list of arguments. > + > block_size:number (default 512) > The size of a data block in bytes. The larger the block size the > less overhead there is for per-block integrity metadata. > @@ -225,6 +231,25 @@ legacy_recalculate > set recalc_sector to zero, and the kernel would not detect the > modification. > > +The key of internal_hash, journal_crypt and journal_mac is encoded either as > +a hexadecimal number or it can be passed as <key_string> prefixed with single > +colon character (':') for keys residing in kernel keyring service. The documentation omits the fact that you need double colon "::" - one colon to end the argument name and one colon as the key prefix. > + > +<key_string> > + The kernel keyring key is identified by string in following format: > + <key_size>:<key_type>:<key_description>. > + > +<key_size> > + The key size in bytes. The kernel key payload size must match > + the value passed in <key_size>. > + > +<key_type> > + Either 'logon', 'user', 'encrypted' or 'trusted' kernel key type. > + > +<key_description> > + The kernel keyring key description integrity target should look for > + when loading key of <key_type>. > + > The journal mode (D/J), buffer_sectors, journal_watermark, commit_time and > allow_discards can be changed when reloading the target (load an inactive > table and swap the tables with suspend and resume). The other arguments > diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig > index df27c7d066d2..e93bf0c3119d 100644 > --- a/drivers/md/Kconfig > +++ b/drivers/md/Kconfig > @@ -653,6 +653,8 @@ config DM_LOG_WRITES > config DM_INTEGRITY > tristate "Integrity target support" > depends on BLK_DEV_DM > + depends on (ENCRYPTED_KEYS || ENCRYPTED_KEYS=n) > + depends on (TRUSTED_KEYS || TRUSTED_KEYS=n) > select BLK_DEV_INTEGRITY > select DM_BUFIO > select CRYPTO > diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c > index 92970e12267a..6995c870cbf5 100644 > --- a/drivers/md/dm-integrity.c > +++ b/drivers/md/dm-integrity.c > @@ -25,6 +25,11 @@ > #include <crypto/utils.h> > #include <linux/async_tx.h> > #include <linux/dm-bufio.h> > +#include <linux/ctype.h> > +#include <linux/key.h> > +#include <keys/user-type.h> > +#include <keys/encrypted-type.h> > +#include <keys/trusted-type.h> > > #include "dm-audit.h" > > @@ -4415,6 +4420,152 @@ static void free_alg(struct alg_spec *a) > memset(a, 0, sizeof(*a)); > } > > +#ifdef CONFIG_KEYS > + > +static bool contains_whitespace(const char *str) > +{ > + while (*str) > + if (isspace(*str++)) > + return true; > + return false; > +} > + > +static int set_key_user(struct alg_spec *a, struct key *key) > +{ > + const struct user_key_payload *ukp; > + > + ukp = user_key_payload_locked(key); > + if (!ukp) > + return -EKEYREVOKED; > + > + if (a->key_size != ukp->datalen) > + return -EINVAL; > + > + memcpy(a->key, ukp->data, a->key_size); > + > + return 0; > +} > + > +static int set_key_encrypted(struct alg_spec *a, struct key *key) > +{ > + const struct encrypted_key_payload *ekp; > + > + ekp = key->payload.data[0]; > + if (!ekp) > + return -EKEYREVOKED; > + > + if (a->key_size != ekp->decrypted_datalen) > + return -EINVAL; > + > + memcpy(a->key, ekp->decrypted_data, a->key_size); > + > + return 0; > +} > + > +static int set_key_trusted(struct alg_spec *a, struct key *key) > +{ > + const struct trusted_key_payload *tkp; > + > + tkp = key->payload.data[0]; > + if (!tkp) > + return -EKEYREVOKED; > + > + if (a->key_size != tkp->key_len) > + return -EINVAL; > + > + memcpy(a->key, tkp->key, a->key_size); > + > + return 0; > +} These four functions are copied from dm-crypt.c and dm-inlinecrypt.c. Copying code is generally malpattern, they should be unified and moved to an include file (that would be included in all three targets) or to the key management code (that would be called from all three targets). > +/* > + * Load the key from the kernel keyring. a->key_string has the format > + * ":<key_size>:<key_type>:<key_description>". > + */ > +static int get_alg_key_from_keyring(struct alg_spec *a) > +{ > + int (*set_key)(struct alg_spec *a, struct key *key); > + const char *key_type_desc, *key_desc; > + struct key_type *type; > + unsigned int key_size; > + struct key *key; > + char dummy; > + int r; > + > + /* > + * Reject key_string with whitespace. dm core currently lacks code for > + * proper whitespace escaping in arguments on DM_TABLE_STATUS path. > + */ > + if (contains_whitespace(a->key_string)) > + return -EINVAL; > + > + if (sscanf(a->key_string, ":%u%c", &key_size, &dummy) != 2 || dummy != ':') > + return -EINVAL; > + if (!key_size || key_size > KMALLOC_MAX_SIZE) > + return -EINVAL; KMALLOC_MAX_SIZE allocations are unreliable, they may fail randomly anytime. You should use "PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER" as the limit (I assume that this is enough for the key) - this is the reliability threshold for kmalloc. If you really need larger allocation, you should use vmalloc or kvmalloc. > + /* skip ":<key_size>:", the sscanf() above guarantees the second ':' */ > + key_type_desc = strchr(a->key_string + 1, ':') + 1; > + > + /* look for next ':' separating key_type from key_description */ > + key_desc = strchr(key_type_desc, ':'); > + if (!key_desc || key_desc == key_type_desc || !strlen(key_desc + 1)) > + return -EINVAL; > + > + if (!strncmp(key_type_desc, "logon:", key_desc - key_type_desc + 1)) { > + type = &key_type_logon; > + set_key = set_key_user; > + } else if (!strncmp(key_type_desc, "user:", key_desc - key_type_desc + 1)) { > + type = &key_type_user; > + set_key = set_key_user; > + } else if (IS_ENABLED(CONFIG_ENCRYPTED_KEYS) && > + !strncmp(key_type_desc, "encrypted:", key_desc - key_type_desc + 1)) { > + type = &key_type_encrypted; > + set_key = set_key_encrypted; > + } else if (IS_ENABLED(CONFIG_TRUSTED_KEYS) && > + !strncmp(key_type_desc, "trusted:", key_desc - key_type_desc + 1)) { > + type = &key_type_trusted; > + set_key = set_key_trusted; > + } else { > + return -EINVAL; > + } > + > + a->key = kmalloc(key_size, GFP_KERNEL); > + if (!a->key) > + return -ENOMEM; > + a->key_size = key_size; > + > + key = request_key(type, key_desc + 1, NULL); > + if (IS_ERR(key)) { > + r = PTR_ERR(key); > + goto free_key; > + } > + > + down_read(&key->sem); > + r = set_key(a, key); > + up_read(&key->sem); > + key_put(key); > + if (r < 0) > + goto free_key; > + > + return 0; > + > +free_key: > + kfree_sensitive(a->key); > + a->key = NULL; > + a->key_size = 0; > + return r; > +} > + > +#else > + > +static int get_alg_key_from_keyring(struct alg_spec *a) > +{ > + return -EINVAL; > +} > + > +#endif /* CONFIG_KEYS */ > + > static int get_alg_and_key(const char *arg, struct alg_spec *a, char **error, char *error_inval) > { > char *k; > @@ -4429,6 +4580,19 @@ static int get_alg_and_key(const char *arg, struct alg_spec *a, char **error, ch > if (k) { > *k = 0; > a->key_string = k + 1; > + > + if (a->key_string[0] == ':') { > + int r = get_alg_key_from_keyring(a); > + > + if (r == -ENOMEM) > + goto nomem; > + if (r == -EINVAL) > + goto inval; > + if (r) > + *error = "Cannot get key from the kernel keyring"; > + return r; > + } > + > if (strlen(a->key_string) & 1) > goto inval; > > -- > 2.55.0 > ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 1/1] dm-integrity: support keys in the kernel keyring 2026-09-30 15:00 ` Mikulas Patocka @ 2026-10-02 9:12 ` Lorenz Kofler 2026-10-02 11:48 ` Mikulas Patocka 0 siblings, 1 reply; 8+ messages in thread From: Lorenz Kofler @ 2026-10-02 9:12 UTC (permalink / raw) To: Mikulas Patocka Cc: Mike Snitzer, Benjamin Marzinski, Alasdair Kergon, dm-devel, linux-kernel, upstream+dm Hi Mikulas, thank you for the review! Some notes below. On 9/30/26 5:00 PM, Mikulas Patocka wrote: > Hi > > Generally, the approach in this patch is OK. > > Here I have some comments for the patch: > > Mikulas > > > > The patch should increase target version - from "{1, 15, 0}" to "{1, 16, > 0}". > > > On Mon, 28 Sep 2026, Lorenz Kofler wrote: > >> The keys of internal_hash, journal_crypt and journal_mac can only be >> given as hex strings in the table. Keys that never leave the kernel, >> such as trusted keys, cannot be used. >> >> In addition to hex strings, accept keys from the kernel keyring in the >> same format as dm-crypt: ":<key_size>:<key_type>:<key_description>". >> The lookup is the same as in dm-crypt. >> >> Like DM_CRYPT, DM_INTEGRITY must not be built in when the encrypted or >> trusted key types are modules. >> >> Signed-off-by: Lorenz Kofler <lorenz@sigma-star.at> >> --- >> .../device-mapper/dm-integrity.rst | 25 +++ >> drivers/md/Kconfig | 2 + >> drivers/md/dm-integrity.c | 164 ++++++++++++++++++ >> 3 files changed, 191 insertions(+) >> >> diff --git a/Documentation/admin-guide/device-mapper/dm-integrity.rst b/Documentation/admin-guide/device-mapper/dm-integrity.rst >> index 9c21301423c9..2b7f82912962 100644 >> --- a/Documentation/admin-guide/device-mapper/dm-integrity.rst >> +++ b/Documentation/admin-guide/device-mapper/dm-integrity.rst >> @@ -146,6 +146,8 @@ internal_hash:algorithm(:key) (the key is optional) >> from an upper layer target, such as dm-crypt. The upper layer >> target should check the validity of the integrity tags. >> >> + The format of the key is described below the list of arguments. >> + >> recalculate >> Recalculate the integrity tags automatically. It is only valid >> when using internal hash. >> @@ -162,6 +164,8 @@ journal_crypt:algorithm(:key) (the key is optional) >> the size of files that were written. To protect against this >> situation, you can encrypt the journal. >> >> + The format of the key is described below the list of arguments. >> + >> journal_mac:algorithm(:key) (the key is optional) >> Protect sector numbers in the journal from accidental or malicious >> modification. To protect against accidental modification, use a >> @@ -173,6 +177,8 @@ journal_mac:algorithm(:key) (the key is optional) >> the journal. Thus, modified sector number would be detected at >> this stage. >> >> + The format of the key is described below the list of arguments. >> + >> block_size:number (default 512) >> The size of a data block in bytes. The larger the block size the >> less overhead there is for per-block integrity metadata. >> @@ -225,6 +231,25 @@ legacy_recalculate >> set recalc_sector to zero, and the kernel would not detect the >> modification. >> >> +The key of internal_hash, journal_crypt and journal_mac is encoded either as >> +a hexadecimal number or it can be passed as <key_string> prefixed with single >> +colon character (':') for keys residing in kernel keyring service. > > The documentation omits the fact that you need double colon "::" - one > colon to end the argument name and one colon as the key prefix. > >> + >> +<key_string> >> + The kernel keyring key is identified by string in following format: >> + <key_size>:<key_type>:<key_description>. >> + >> +<key_size> >> + The key size in bytes. The kernel key payload size must match >> + the value passed in <key_size>. >> + >> +<key_type> >> + Either 'logon', 'user', 'encrypted' or 'trusted' kernel key type. >> + >> +<key_description> >> + The kernel keyring key description integrity target should look for >> + when loading key of <key_type>. >> + >> The journal mode (D/J), buffer_sectors, journal_watermark, commit_time and >> allow_discards can be changed when reloading the target (load an inactive >> table and swap the tables with suspend and resume). The other arguments >> diff --git a/drivers/md/Kconfig b/drivers/md/Kconfig >> index df27c7d066d2..e93bf0c3119d 100644 >> --- a/drivers/md/Kconfig >> +++ b/drivers/md/Kconfig >> @@ -653,6 +653,8 @@ config DM_LOG_WRITES >> config DM_INTEGRITY >> tristate "Integrity target support" >> depends on BLK_DEV_DM >> + depends on (ENCRYPTED_KEYS || ENCRYPTED_KEYS=n) >> + depends on (TRUSTED_KEYS || TRUSTED_KEYS=n) >> select BLK_DEV_INTEGRITY >> select DM_BUFIO >> select CRYPTO >> diff --git a/drivers/md/dm-integrity.c b/drivers/md/dm-integrity.c >> index 92970e12267a..6995c870cbf5 100644 >> --- a/drivers/md/dm-integrity.c >> +++ b/drivers/md/dm-integrity.c >> @@ -25,6 +25,11 @@ >> #include <crypto/utils.h> >> #include <linux/async_tx.h> >> #include <linux/dm-bufio.h> >> +#include <linux/ctype.h> >> +#include <linux/key.h> >> +#include <keys/user-type.h> >> +#include <keys/encrypted-type.h> >> +#include <keys/trusted-type.h> >> >> #include "dm-audit.h" >> >> @@ -4415,6 +4420,152 @@ static void free_alg(struct alg_spec *a) >> memset(a, 0, sizeof(*a)); >> } >> >> +#ifdef CONFIG_KEYS >> + >> +static bool contains_whitespace(const char *str) >> +{ >> + while (*str) >> + if (isspace(*str++)) >> + return true; >> + return false; >> +} >> + >> +static int set_key_user(struct alg_spec *a, struct key *key) >> +{ >> + const struct user_key_payload *ukp; >> + >> + ukp = user_key_payload_locked(key); >> + if (!ukp) >> + return -EKEYREVOKED; >> + >> + if (a->key_size != ukp->datalen) >> + return -EINVAL; >> + >> + memcpy(a->key, ukp->data, a->key_size); >> + >> + return 0; >> +} >> + >> +static int set_key_encrypted(struct alg_spec *a, struct key *key) >> +{ >> + const struct encrypted_key_payload *ekp; >> + >> + ekp = key->payload.data[0]; >> + if (!ekp) >> + return -EKEYREVOKED; >> + >> + if (a->key_size != ekp->decrypted_datalen) >> + return -EINVAL; >> + >> + memcpy(a->key, ekp->decrypted_data, a->key_size); >> + >> + return 0; >> +} >> + >> +static int set_key_trusted(struct alg_spec *a, struct key *key) >> +{ >> + const struct trusted_key_payload *tkp; >> + >> + tkp = key->payload.data[0]; >> + if (!tkp) >> + return -EKEYREVOKED; >> + >> + if (a->key_size != tkp->key_len) >> + return -EINVAL; >> + >> + memcpy(a->key, tkp->key, a->key_size); >> + >> + return 0; >> +} > > These four functions are copied from dm-crypt.c and dm-inlinecrypt.c. > Copying code is generally malpattern, they should be unified and moved to > an include file (that would be included in all three targets) or to the > key management code (that would be called from all three targets). > Yes that is the issue I described in the cover letters. But I don't actually know which way is the preferred one. Afaik there are now three options: 1. static inline helpers in a drivers/md header, so dm-crypt and dm-integrity each compile their own copy 2. a small library module, similar to dm-bufio, so there is one copy that follows the value (y/m) of dm-crypt and dm-integrity 3. integration into key management code Please tell me which option you prefer. >> +/* >> + * Load the key from the kernel keyring. a->key_string has the format >> + * ":<key_size>:<key_type>:<key_description>". >> + */ >> +static int get_alg_key_from_keyring(struct alg_spec *a) >> +{ >> + int (*set_key)(struct alg_spec *a, struct key *key); >> + const char *key_type_desc, *key_desc; >> + struct key_type *type; >> + unsigned int key_size; >> + struct key *key; >> + char dummy; >> + int r; >> + >> + /* >> + * Reject key_string with whitespace. dm core currently lacks code for >> + * proper whitespace escaping in arguments on DM_TABLE_STATUS path. >> + */ >> + if (contains_whitespace(a->key_string)) >> + return -EINVAL; >> + >> + if (sscanf(a->key_string, ":%u%c", &key_size, &dummy) != 2 || dummy != ':') >> + return -EINVAL; >> + if (!key_size || key_size > KMALLOC_MAX_SIZE) >> + return -EINVAL; > > KMALLOC_MAX_SIZE allocations are unreliable, they may fail randomly > anytime. > > You should use "PAGE_SIZE << PAGE_ALLOC_COSTLY_ORDER" as the limit (I > assume that this is enough for the key) - this is the reliability > threshold for kmalloc. > > If you really need larger allocation, you should use vmalloc or kvmalloc. > >> + /* skip ":<key_size>:", the sscanf() above guarantees the second ':' */ >> + key_type_desc = strchr(a->key_string + 1, ':') + 1; >> + >> + /* look for next ':' separating key_type from key_description */ >> + key_desc = strchr(key_type_desc, ':'); >> + if (!key_desc || key_desc == key_type_desc || !strlen(key_desc + 1)) >> + return -EINVAL; >> + >> + if (!strncmp(key_type_desc, "logon:", key_desc - key_type_desc + 1)) { >> + type = &key_type_logon; >> + set_key = set_key_user; >> + } else if (!strncmp(key_type_desc, "user:", key_desc - key_type_desc + 1)) { >> + type = &key_type_user; >> + set_key = set_key_user; >> + } else if (IS_ENABLED(CONFIG_ENCRYPTED_KEYS) && >> + !strncmp(key_type_desc, "encrypted:", key_desc - key_type_desc + 1)) { >> + type = &key_type_encrypted; >> + set_key = set_key_encrypted; >> + } else if (IS_ENABLED(CONFIG_TRUSTED_KEYS) && >> + !strncmp(key_type_desc, "trusted:", key_desc - key_type_desc + 1)) { >> + type = &key_type_trusted; >> + set_key = set_key_trusted; >> + } else { >> + return -EINVAL; >> + } >> + >> + a->key = kmalloc(key_size, GFP_KERNEL); >> + if (!a->key) >> + return -ENOMEM; >> + a->key_size = key_size; >> + >> + key = request_key(type, key_desc + 1, NULL); >> + if (IS_ERR(key)) { >> + r = PTR_ERR(key); >> + goto free_key; >> + } >> + >> + down_read(&key->sem); >> + r = set_key(a, key); >> + up_read(&key->sem); >> + key_put(key); >> + if (r < 0) >> + goto free_key; >> + >> + return 0; >> + >> +free_key: >> + kfree_sensitive(a->key); >> + a->key = NULL; >> + a->key_size = 0; >> + return r; >> +} >> + >> +#else >> + >> +static int get_alg_key_from_keyring(struct alg_spec *a) >> +{ >> + return -EINVAL; >> +} >> + >> +#endif /* CONFIG_KEYS */ >> + >> static int get_alg_and_key(const char *arg, struct alg_spec *a, char **error, char *error_inval) >> { >> char *k; >> @@ -4429,6 +4580,19 @@ static int get_alg_and_key(const char *arg, struct alg_spec *a, char **error, ch >> if (k) { >> *k = 0; >> a->key_string = k + 1; >> + >> + if (a->key_string[0] == ':') { >> + int r = get_alg_key_from_keyring(a); >> + >> + if (r == -ENOMEM) >> + goto nomem; >> + if (r == -EINVAL) >> + goto inval; >> + if (r) >> + *error = "Cannot get key from the kernel keyring"; >> + return r; >> + } >> + >> if (strlen(a->key_string) & 1) >> goto inval; >> >> -- >> 2.55.0 >> > -- sigma star gmbh | Eduard-Bodem-Gasse 6, 6020 Innsbruck, Austria UID/VAT Nr: ATU 66964118 | FN: 374287y ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 1/1] dm-integrity: support keys in the kernel keyring 2026-10-02 9:12 ` Lorenz Kofler @ 2026-10-02 11:48 ` Mikulas Patocka 2026-10-02 20:14 ` Eric Biggers 0 siblings, 1 reply; 8+ messages in thread From: Mikulas Patocka @ 2026-10-02 11:48 UTC (permalink / raw) To: Lorenz Kofler, Eric Biggers Cc: Mike Snitzer, Benjamin Marzinski, Alasdair Kergon, dm-devel, linux-kernel, upstream+dm, David Howells, Jarkko Sakkinen, keyrings On Fri, 2 Oct 2026, Lorenz Kofler wrote: > Hi Mikulas, > > thank you for the review! > > Some notes below. > > >> +#ifdef CONFIG_KEYS > >> + > >> +static bool contains_whitespace(const char *str) > >> +{ > >> + while (*str) > >> + if (isspace(*str++)) > >> + return true; > >> + return false; > >> +} > >> + > >> +static int set_key_user(struct alg_spec *a, struct key *key) > >> +{ > >> + const struct user_key_payload *ukp; > >> + > >> + ukp = user_key_payload_locked(key); > >> + if (!ukp) > >> + return -EKEYREVOKED; > >> + > >> + if (a->key_size != ukp->datalen) > >> + return -EINVAL; > >> + > >> + memcpy(a->key, ukp->data, a->key_size); > >> + > >> + return 0; > >> +} > >> + > >> +static int set_key_encrypted(struct alg_spec *a, struct key *key) > >> +{ > >> + const struct encrypted_key_payload *ekp; > >> + > >> + ekp = key->payload.data[0]; > >> + if (!ekp) > >> + return -EKEYREVOKED; > >> + > >> + if (a->key_size != ekp->decrypted_datalen) > >> + return -EINVAL; > >> + > >> + memcpy(a->key, ekp->decrypted_data, a->key_size); > >> + > >> + return 0; > >> +} > >> + > >> +static int set_key_trusted(struct alg_spec *a, struct key *key) > >> +{ > >> + const struct trusted_key_payload *tkp; > >> + > >> + tkp = key->payload.data[0]; > >> + if (!tkp) > >> + return -EKEYREVOKED; > >> + > >> + if (a->key_size != tkp->key_len) > >> + return -EINVAL; > >> + > >> + memcpy(a->key, tkp->key, a->key_size); > >> + > >> + return 0; > >> +} > > > > These four functions are copied from dm-crypt.c and dm-inlinecrypt.c. > > Copying code is generally malpattern, they should be unified and moved to > > an include file (that would be included in all three targets) or to the > > key management code (that would be called from all three targets). > > > > Yes that is the issue I described in the cover letters. But I don't > actually know which way is the preferred one. Afaik there are now > three options: > > 1. static inline helpers in a drivers/md header, so dm-crypt and > dm-integrity each compile their own copy > 2. a small library module, similar to dm-bufio, so there is one copy > that follows the value (y/m) of dm-crypt and dm-integrity > 3. integration into key management code > > Please tell me which option you prefer. Try 3, if not possible then 1. I think that introducing a module with this would be overkill. The "if (!strncmp(key_string, "logon:", key_desc - key_string + 1)) {" lines are duplicated as well, so I would refactor them and move them to the helper too. I don't know why dm-inlinecrypt only uses the "logon:" key while dm-crypt uses "user:", "encrypted:", "trusted:" as well (Eric - could you explain?). So, perhaps, dm-inlinecrypt could be extended to use all four key types as well. I CC'd keyrings maintainers - so, if they have some suggestions or objections regarding moving the code there, let them say. Mikulas ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 1/1] dm-integrity: support keys in the kernel keyring 2026-10-02 11:48 ` Mikulas Patocka @ 2026-10-02 20:14 ` Eric Biggers 2026-10-02 21:09 ` Mikulas Patocka 2026-10-04 7:44 ` Milan Broz 0 siblings, 2 replies; 8+ messages in thread From: Eric Biggers @ 2026-10-02 20:14 UTC (permalink / raw) To: Mikulas Patocka Cc: Lorenz Kofler, Mike Snitzer, Benjamin Marzinski, Alasdair Kergon, dm-devel, linux-kernel, upstream+dm, David Howells, Jarkko Sakkinen, keyrings On Fri, Oct 02, 2026 at 01:48:23PM +0200, Mikulas Patocka wrote: > > > These four functions are copied from dm-crypt.c and dm-inlinecrypt.c. > > > Copying code is generally malpattern, they should be unified and moved to > > > an include file (that would be included in all three targets) or to the > > > key management code (that would be called from all three targets). > > > > > > > Yes that is the issue I described in the cover letters. But I don't > > actually know which way is the preferred one. Afaik there are now > > three options: > > > > 1. static inline helpers in a drivers/md header, so dm-crypt and > > dm-integrity each compile their own copy > > 2. a small library module, similar to dm-bufio, so there is one copy > > that follows the value (y/m) of dm-crypt and dm-integrity > > 3. integration into key management code > > > > Please tell me which option you prefer. > > Try 3, if not possible then 1. I think that introducing a module with this > would be overkill. > > The "if (!strncmp(key_string, "logon:", key_desc - key_string + 1)) {" > lines are duplicated as well, so I would refactor them and move them to > the helper too. > > I don't know why dm-inlinecrypt only uses the "logon:" key while dm-crypt > uses "user:", "encrypted:", "trusted:" as well (Eric - could you > explain?). So, perhaps, dm-inlinecrypt could be extended to use all four > key types as well. The keyring support didn't exist in my version of the dm-inlinecrypt patch. It seems to have been requested by Milan here: https://lore.kernel.org/dm-devel/682506ea-c9c2-458b-8123-8d78fc53cc7f@gmail.com/ then added by Linlin. From what I understand, the point of the keyring support in dm-{crypt,inlinecrypt,integrity} is: - To support "trusted" keys. But that is not what was actually implemented in dm-inlinecrypt. - To avoid having the key be readable with STATUSTYPE_TABLE. But that is not what was actually implemented in dm-inlinecrypt. Keyrings are also unnecesary to solve that problem. - To cause security bugs such as https://lwn.net/Articles/1090568/ . Since otherwise things aren't exciting enough, I guess. Not sure what I'm missing. But if you really do want to support all four key types in all three of these targets anyway though, then sure, the code might as well be shared since it would otherwise be the same code in each. - Eric ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 1/1] dm-integrity: support keys in the kernel keyring 2026-10-02 20:14 ` Eric Biggers @ 2026-10-02 21:09 ` Mikulas Patocka 2026-10-04 7:44 ` Milan Broz 1 sibling, 0 replies; 8+ messages in thread From: Mikulas Patocka @ 2026-10-02 21:09 UTC (permalink / raw) To: Eric Biggers Cc: Lorenz Kofler, Mike Snitzer, Benjamin Marzinski, Alasdair Kergon, dm-devel, linux-kernel, upstream+dm, David Howells, Jarkko Sakkinen, keyrings On Fri, 2 Oct 2026, Eric Biggers wrote: > >From what I understand, the point of the keyring support in > dm-{crypt,inlinecrypt,integrity} is: > > - To support "trusted" keys. But that is not what was actually > implemented in dm-inlinecrypt. > > - To avoid having the key be readable with STATUSTYPE_TABLE. But that > is not what was actually implemented in dm-inlinecrypt. Keyrings are > also unnecesary to solve that problem. > > - To cause security bugs such as https://lwn.net/Articles/1090568/ . > Since otherwise things aren't exciting enough, I guess. > > Not sure what I'm missing. > > But if you really do want to support all four key types in all three of > these targets anyway though, then sure, the code might as well be > shared since it would otherwise be the same code in each. > > - Eric Yes, I think that the key support in these targets should be unified. Mikulas ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [RFC PATCH 1/1] dm-integrity: support keys in the kernel keyring 2026-10-02 20:14 ` Eric Biggers 2026-10-02 21:09 ` Mikulas Patocka @ 2026-10-04 7:44 ` Milan Broz 1 sibling, 0 replies; 8+ messages in thread From: Milan Broz @ 2026-10-04 7:44 UTC (permalink / raw) To: Eric Biggers, Mikulas Patocka Cc: Lorenz Kofler, Mike Snitzer, Benjamin Marzinski, Alasdair Kergon, dm-devel, linux-kernel, upstream+dm, David Howells, Jarkko Sakkinen, keyrings On 10/2/26 10:14 PM, Eric Biggers wrote: ... > From what I understand, the point of the keyring support in > dm-{crypt,inlinecrypt,integrity} is: > > - To support "trusted" keys. But that is not what was actually > implemented in dm-inlinecrypt. > > - To avoid having the key be readable with STATUSTYPE_TABLE. But that > is not what was actually implemented in dm-inlinecrypt. Keyrings are > also unnecesary to solve that problem. There is more to that - to avoid key cached in dm-crypt (or other target) (dmsetup must be able to retrieve mapping table in the form directly reusable for recreating DM mapping, so raw key must be available) - to avoid inclusion of key in DM ioctl calls (mapping table again) - to somehow simplify keyring handling was used already by other userspace tools (just reference existing keyring instead of creating new one) > > - To cause security bugs such as https://lwn.net/Articles/1090568/ . > Since otherwise things aren't exciting enough, I guess. :-) But TBH, this can happen in any other subsystem working with keys. That said, I see keyring as incredibly complex code with complicated CLI tool... Anyway, DM targets should be unified. Userspace support will be tricky, but that is another issue (we have full support for dm-crypt, dm-integrtity maybe requires some API changes. Will check once kernel get the support.) Milan ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-10-04 7:45 UTC | newest] Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-09-28 6:27 [RFC PATCH 0/1] dm-integrity: support keys in the kernel keyring Lorenz Kofler 2026-09-28 6:27 ` [RFC PATCH 1/1] " Lorenz Kofler 2026-09-30 15:00 ` Mikulas Patocka 2026-10-02 9:12 ` Lorenz Kofler 2026-10-02 11:48 ` Mikulas Patocka 2026-10-02 20:14 ` Eric Biggers 2026-10-02 21:09 ` Mikulas Patocka 2026-10-04 7:44 ` Milan Broz
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®