| Message ID | 20230608120444.382527-1-nayna@linux.ibm.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] security/integrity: fix pointer to ESL data and its size on pseries | expand |
On Thu Jun 8, 2023 at 3:04 PM EEST, Nayna Jain wrote: > On PowerVM guest, variable data is prefixed with 8 bytes of timestamp. > Extract ESL by stripping off the timestamp before passing to ESL parser. > > Fixes: 4b3e71e9a34c ("integrity/powerpc: Support loading keys from PLPKS") > Cc: stable@vger.kenrnel.org # v6.3 > Signed-off-by: Nayna Jain <nayna@linux.ibm.com> > --- > Changelog: > v2: Fixed feedback from Jarkko > * added CC to stable > * moved *data declaration to same line as *db,*dbx > Renamed extract_data() macro to extract_esl() for clarity > .../integrity/platform_certs/load_powerpc.c | 40 ++++++++++++------- > 1 file changed, 26 insertions(+), 14 deletions(-) > > diff --git a/security/integrity/platform_certs/load_powerpc.c b/security/integrity/platform_certs/load_powerpc.c > index b9de70b90826..170789dc63d2 100644 > --- a/security/integrity/platform_certs/load_powerpc.c > +++ b/security/integrity/platform_certs/load_powerpc.c > @@ -15,6 +15,9 @@ > #include "keyring_handler.h" > #include "../integrity.h" > > +#define extract_esl(db, data, size, offset) \ > + do { db = data + offset; size = size - offset; } while (0) > + > /* > * Get a certificate list blob from the named secure variable. > * > @@ -55,8 +58,9 @@ static __init void *get_cert_list(u8 *key, unsigned long keylen, u64 *size) > */ > static int __init load_powerpc_certs(void) > { > - void *db = NULL, *dbx = NULL; > - u64 dbsize = 0, dbxsize = 0; > + void *db = NULL, *dbx = NULL, *data = NULL; > + u64 dsize = 0; > + u64 offset = 0; > int rc = 0; > ssize_t len; > char buf[32]; > @@ -74,38 +78,46 @@ static int __init load_powerpc_certs(void) > return -ENODEV; > } > > + if (strcmp("ibm,plpks-sb-v1", buf) == 0) > + /* PLPKS authenticated variables ESL data is prefixed with 8 bytes of timestamp */ > + offset = 8; > + > /* > * Get db, and dbx. They might not exist, so it isn't an error if we > * can't get them. > */ > - db = get_cert_list("db", 3, &dbsize); > - if (!db) { > + data = get_cert_list("db", 3, &dsize); > + if (!data) { > pr_info("Couldn't get db list from firmware\n"); > - } else if (IS_ERR(db)) { > - rc = PTR_ERR(db); > + } else if (IS_ERR(data)) { > + rc = PTR_ERR(data); > pr_err("Error reading db from firmware: %d\n", rc); > return rc; > } else { > - rc = parse_efi_signature_list("powerpc:db", db, dbsize, > + extract_esl(db, data, dsize, offset); > + > + rc = parse_efi_signature_list("powerpc:db", db, dsize, > get_handler_for_db); > if (rc) > pr_err("Couldn't parse db signatures: %d\n", rc); > - kfree(db); > + kfree(data); > } > > - dbx = get_cert_list("dbx", 4, &dbxsize); > - if (!dbx) { > + data = get_cert_list("dbx", 4, &dsize); > + if (!data) { > pr_info("Couldn't get dbx list from firmware\n"); > - } else if (IS_ERR(dbx)) { > - rc = PTR_ERR(dbx); > + } else if (IS_ERR(data)) { > + rc = PTR_ERR(data); > pr_err("Error reading dbx from firmware: %d\n", rc); > return rc; > } else { > - rc = parse_efi_signature_list("powerpc:dbx", dbx, dbxsize, > + extract_esl(dbx, data, dsize, offset); > + > + rc = parse_efi_signature_list("powerpc:dbx", dbx, dsize, > get_handler_for_dbx); > if (rc) > pr_err("Couldn't parse dbx signatures: %d\n", rc); > - kfree(dbx); > + kfree(data); > } > > return rc; > -- > 2.31.1 Acked-by: Jarkko Sakkinen <jarkko@kernel.org> BR, Jarkko
On 08/06/23 5:34 pm, Nayna Jain wrote: > On PowerVM guest, variable data is prefixed with 8 bytes of timestamp. > Extract ESL by stripping off the timestamp before passing to ESL parser. > > Fixes: 4b3e71e9a34c ("integrity/powerpc: Support loading keys from PLPKS") > Cc: stable@vger.kenrnel.org # v6.3 > Signed-off-by: Nayna Jain <nayna@linux.ibm.com> > --- Tested-by: Nageswara R Sastry <rnsastry@linux.ibm.com> Seeing the keyring difference with and with out patch: With out patch: [root@ltcrain80-lp2 ~]# grep keyring /proc/keys | grep -v _ses 0131ebb4 I--Q--- 1 perm 0c030000 0 65534 keyring .user_reg: 2 039dfd93 I------ 1 perm 1f0f0000 0 0 keyring .ima: 1 03a160b5 I------ 1 perm 1f0b0000 0 0 keyring .builtin_trusted_keys: 2 05377b73 I------ 1 perm 1f0b0000 0 0 keyring .platform: empty 0d7ea730 I------ 1 perm 0f0b0000 0 0 keyring .blacklist: empty 16235f2d I--Q--- 6 perm 1f3f0000 0 65534 keyring _uid.0: empty 1721f130 I------ 1 perm 1f0f0000 0 0 keyring .evm: empty With patch: [root@ltcrain80-lp2 ~]# grep keyring /proc/keys | grep -v _ses 04820159 I------ 1 perm 0f0b0000 0 0 keyring .blacklist: 1 16d05827 I--Q--- 1 perm 0c030000 0 65534 keyring .user_reg: 2 17648d6a I------ 1 perm 1f0b0000 0 0 keyring .builtin_trusted_keys: 2 2158b34f I--Q--- 6 perm 1f3f0000 0 65534 keyring _uid.0: empty 2237eff6 I------ 1 perm 1f0f0000 0 0 keyring .evm: empty 26d0330c I------ 1 perm 1f0b0000 0 0 keyring .platform: 1 2daa48ab I------ 1 perm 1f0f0000 0 0 keyring .ima: 1 Thank you. > Changelog: > v2: Fixed feedback from Jarkko > * added CC to stable > * moved *data declaration to same line as *db,*dbx > Renamed extract_data() macro to extract_esl() for clarity > .../integrity/platform_certs/load_powerpc.c | 40 ++++++++++++------- > 1 file changed, 26 insertions(+), 14 deletions(-) > > diff --git a/security/integrity/platform_certs/load_powerpc.c b/security/integrity/platform_certs/load_powerpc.c > index b9de70b90826..170789dc63d2 100644 > --- a/security/integrity/platform_certs/load_powerpc.c > +++ b/security/integrity/platform_certs/load_powerpc.c > @@ -15,6 +15,9 @@ > #include "keyring_handler.h" > #include "../integrity.h" > > +#define extract_esl(db, data, size, offset) \ > + do { db = data + offset; size = size - offset; } while (0) > + > /* > * Get a certificate list blob from the named secure variable. > * > @@ -55,8 +58,9 @@ static __init void *get_cert_list(u8 *key, unsigned long keylen, u64 *size) > */ > static int __init load_powerpc_certs(void) > { > - void *db = NULL, *dbx = NULL; > - u64 dbsize = 0, dbxsize = 0; > + void *db = NULL, *dbx = NULL, *data = NULL; > + u64 dsize = 0; > + u64 offset = 0; > int rc = 0; > ssize_t len; > char buf[32]; > @@ -74,38 +78,46 @@ static int __init load_powerpc_certs(void) > return -ENODEV; > } > > + if (strcmp("ibm,plpks-sb-v1", buf) == 0) > + /* PLPKS authenticated variables ESL data is prefixed with 8 bytes of timestamp */ > + offset = 8; > + > /* > * Get db, and dbx. They might not exist, so it isn't an error if we > * can't get them. > */ > - db = get_cert_list("db", 3, &dbsize); > - if (!db) { > + data = get_cert_list("db", 3, &dsize); > + if (!data) { > pr_info("Couldn't get db list from firmware\n"); > - } else if (IS_ERR(db)) { > - rc = PTR_ERR(db); > + } else if (IS_ERR(data)) { > + rc = PTR_ERR(data); > pr_err("Error reading db from firmware: %d\n", rc); > return rc; > } else { > - rc = parse_efi_signature_list("powerpc:db", db, dbsize, > + extract_esl(db, data, dsize, offset); > + > + rc = parse_efi_signature_list("powerpc:db", db, dsize, > get_handler_for_db); > if (rc) > pr_err("Couldn't parse db signatures: %d\n", rc); > - kfree(db); > + kfree(data); > } > > - dbx = get_cert_list("dbx", 4, &dbxsize); > - if (!dbx) { > + data = get_cert_list("dbx", 4, &dsize); > + if (!data) { > pr_info("Couldn't get dbx list from firmware\n"); > - } else if (IS_ERR(dbx)) { > - rc = PTR_ERR(dbx); > + } else if (IS_ERR(data)) { > + rc = PTR_ERR(data); > pr_err("Error reading dbx from firmware: %d\n", rc); > return rc; > } else { > - rc = parse_efi_signature_list("powerpc:dbx", dbx, dbxsize, > + extract_esl(dbx, data, dsize, offset); > + > + rc = parse_efi_signature_list("powerpc:dbx", dbx, dsize, > get_handler_for_dbx); > if (rc) > pr_err("Couldn't parse dbx signatures: %d\n", rc); > - kfree(dbx); > + kfree(data); > } > > return rc;
On Thu, 08 Jun 2023 08:04:44 -0400, Nayna Jain wrote: > On PowerVM guest, variable data is prefixed with 8 bytes of timestamp. > Extract ESL by stripping off the timestamp before passing to ESL parser. > > Applied to powerpc/next. [1/1] security/integrity: fix pointer to ESL data and its size on pseries https://git.kernel.org/powerpc/c/e66effaf61ffb1dc6088492ca3a0e98dcbf1c10d cheers
diff --git a/security/integrity/platform_certs/load_powerpc.c b/security/integrity/platform_certs/load_powerpc.c index b9de70b90826..170789dc63d2 100644 --- a/security/integrity/platform_certs/load_powerpc.c +++ b/security/integrity/platform_certs/load_powerpc.c @@ -15,6 +15,9 @@ #include "keyring_handler.h" #include "../integrity.h" +#define extract_esl(db, data, size, offset) \ + do { db = data + offset; size = size - offset; } while (0) + /* * Get a certificate list blob from the named secure variable. * @@ -55,8 +58,9 @@ static __init void *get_cert_list(u8 *key, unsigned long keylen, u64 *size) */ static int __init load_powerpc_certs(void) { - void *db = NULL, *dbx = NULL; - u64 dbsize = 0, dbxsize = 0; + void *db = NULL, *dbx = NULL, *data = NULL; + u64 dsize = 0; + u64 offset = 0; int rc = 0; ssize_t len; char buf[32]; @@ -74,38 +78,46 @@ static int __init load_powerpc_certs(void) return -ENODEV; } + if (strcmp("ibm,plpks-sb-v1", buf) == 0) + /* PLPKS authenticated variables ESL data is prefixed with 8 bytes of timestamp */ + offset = 8; + /* * Get db, and dbx. They might not exist, so it isn't an error if we * can't get them. */ - db = get_cert_list("db", 3, &dbsize); - if (!db) { + data = get_cert_list("db", 3, &dsize); + if (!data) { pr_info("Couldn't get db list from firmware\n"); - } else if (IS_ERR(db)) { - rc = PTR_ERR(db); + } else if (IS_ERR(data)) { + rc = PTR_ERR(data); pr_err("Error reading db from firmware: %d\n", rc); return rc; } else { - rc = parse_efi_signature_list("powerpc:db", db, dbsize, + extract_esl(db, data, dsize, offset); + + rc = parse_efi_signature_list("powerpc:db", db, dsize, get_handler_for_db); if (rc) pr_err("Couldn't parse db signatures: %d\n", rc); - kfree(db); + kfree(data); } - dbx = get_cert_list("dbx", 4, &dbxsize); - if (!dbx) { + data = get_cert_list("dbx", 4, &dsize); + if (!data) { pr_info("Couldn't get dbx list from firmware\n"); - } else if (IS_ERR(dbx)) { - rc = PTR_ERR(dbx); + } else if (IS_ERR(data)) { + rc = PTR_ERR(data); pr_err("Error reading dbx from firmware: %d\n", rc); return rc; } else { - rc = parse_efi_signature_list("powerpc:dbx", dbx, dbxsize, + extract_esl(dbx, data, dsize, offset); + + rc = parse_efi_signature_list("powerpc:dbx", dbx, dsize, get_handler_for_dbx); if (rc) pr_err("Couldn't parse dbx signatures: %d\n", rc); - kfree(dbx); + kfree(data); } return rc;
On PowerVM guest, variable data is prefixed with 8 bytes of timestamp. Extract ESL by stripping off the timestamp before passing to ESL parser. Fixes: 4b3e71e9a34c ("integrity/powerpc: Support loading keys from PLPKS") Cc: stable@vger.kenrnel.org # v6.3 Signed-off-by: Nayna Jain <nayna@linux.ibm.com> --- Changelog: v2: Fixed feedback from Jarkko * added CC to stable * moved *data declaration to same line as *db,*dbx Renamed extract_data() macro to extract_esl() for clarity .../integrity/platform_certs/load_powerpc.c | 40 ++++++++++++------- 1 file changed, 26 insertions(+), 14 deletions(-)