Re: [meta-intel] [PATCH 3/3] rmc: add database extraction functionality

[Todor Minchev]

On Mon, 2017-02-06 at 13:09 -0800, Jianxun Zhang wrote:
> Todor,
> Nice change overall. I haven’t run any test and just share multiple (11) 
> inline comments for this patch.

A patchset incorporating these comments is in progress.

> This should be the last one in the series. Please let me know if I missed any 
> other RMC patches for review.
> 
> I plan to run rmc internal test once we have an updated patch set. We could 
> need to add a new test case for the dumping feature in the future.
> 
> You can refer to the README in rmc project for the pipeline of merging.
> 
> Thanks!
> 
> > On Feb 2, 2017, at 2:37 PM, Todor Minchev  
> > wrote:
> > 
> > The contents of an existing database file can be extracted in the
> > current working directory with the -E option. The top level of the
> > directory tree is rmc_db_dump and all files corresponding to
> > a given record will be saved in a separate sub-directory. The sub-directory
> > name of each record is the signature corresponding to the fingerprint for
> > that record.
> > 
> > Example:
> > ./src/rmc -E -d rmc.db
> > 
> > Successfully extracted rmc.db
> > 
> > Signed-off-by: Todor Minchev 
> > ---
> > inc/rmc_api.h |  9 ++
> > src/lib/api.c | 85 
> > +--
> > src/lib/common/rmcl.c |  3 +-
> > src/rmc.c | 44 +++---
> > 4 files changed, 126 insertions(+), 15 deletions(-)
> > 
> > diff --git a/inc/rmc_api.h b/inc/rmc_api.h
> > index a484389..ce26220 100644
> > --- a/inc/rmc_api.h
> > +++ b/inc/rmc_api.h
> > @@ -74,6 +74,15 @@ extern int rmc_query_file_by_fp(rmc_fingerprint_t *fp, 
> > char *db_pathname, char *
> >  */
> > extern int rmc_gimme_file(char* db_pathname, char *file_name, rmc_file_t 
> > *file);
> > 
> > +
> > +/* extract the contents of a database file and store the files 
> > corresponding to
> > + * each record in a separate directory. The name of each directory is the 
> > signature
> > + * of the fingerpring for that record
> > + * (in) db_pathname: The path and file name of a RMC database file 
> > generated by RMC tool
> > + * return: 0 on success, non-zero on failure.
> > + */
> > +int dump_db(char *db_pathname) ;
> > +
> Please move this into section 1.3, somewhere after next line. It doesn’t 
> belong to section 1.2 “double-action API”

Will do.

> 
> > /* 1.3 - Helper APIs */
> > 
> > /* Free allocated data referred in a fingerprint
> > diff --git a/src/lib/api.c b/src/lib/api.c
> > index 0adb390..aca8d99 100644
> > --- a/src/lib/api.c
> > +++ b/src/lib/api.c
> > @@ -3,6 +3,7 @@
> >  * RMC API implementation for Linux user space
> >  */
> > 
> > +#define _GNU_SOURCE
> > #include 
> > #include 
> > #include 
> > @@ -14,8 +15,11 @@
> > #include 
> > #include 
> > 
> > -#define EFI_SYSTAB_PATH "/sys/firmware/efi/systab"
> > -#define SYSTAB_LEN 4096 /* assume 4kb is enough...*/
> > +#define EFI_SYSTAB_PATH  "/sys/firmware/efi/systab"
> > +#define SYSTAB_LEN   4096 /* assume 4kb is enough...*/
> > +#define DB_DUMP_DIR  "./rmc_db_dump"  /* directory to store db data 
> > dump */
> > +
> > +extern const rmc_uint8_t rmc_db_signature[RMC_DB_SIG_LEN];
> We could have a new small helper API like is_rmcdb(db_blob) in RMCL to hold 
> checker logic at line 357 in this file, so that we can get rid of this line 
> and make the checker reusable. (So far I feel the checker should work in both 
> EFI and Linux contexts.)
> 
> We could even update checker API without bothering its callers in the future. 
> Let me know if it makes sense...

Makes sense

> > 
> > int read_file(const char *pathname, char **data, rmc_size_t* len) {
> > int fd = -1;
> > @@ -325,3 +329,80 @@ int rmc_gimme_file(char* db_pathname, char *file_name, 
> > rmc_file_t *file) {
> > 
> > return ret;
> > }
> > +
> > +/*
> > + * Dump contents of database file
> > + * (in) rmc_db - input database file to extract
> rmc_db VS db_pathname. I think we can remove the comment here, it is already 
> in the header file.
> > + */

OK

> > +int dump_db(char *db_pathname) {
> > +rmc_meta_header_t meta_header;
> > +rmc_db_header_t *db_header = NULL;
> > +rmc_record_header_t record_header;
> > +rmc_uint64_t record_idx = 0;   /* offset of each reacord in db*/
> > +rmc_uint64_t meta_idx = 0; /* offset of each meta in a record */
> > +rmc_uint64_t file_idx = 0; /* offset of file in a meta */
> > +rmc_file_t file;
> > +char *out_dir = NULL, *out_name = NULL;
> > +rmc_size_t db_len = 0;
> > +rmc_uint8_t *rmc_db = NULL;
> > +struct stat s = {0};
> > +
> > +if (read_file(db_pathname, (char **)_db, _len)) {
> > +fprintf(stderr, "Failed to read database file\n\n");
> > +return 1;
> > +}
> > +
> > +db_header = (rmc_db_header_t *)rmc_db;
> > +
> > +/* sanity check of db */
> > +if (strncmp((const char *)db_header->signature,
> > + 

Re: [meta-intel] [PATCH 3/3] rmc: add database extraction functionality

[Jianxun Zhang]

Todor,
Nice change overall. I haven’t run any test and just share multiple (11) inline 
comments for this patch.

This should be the last one in the series. Please let me know if I missed any 
other RMC patches for review.

I plan to run rmc internal test once we have an updated patch set. We could 
need to add a new test case for the dumping feature in the future.

You can refer to the README in rmc project for the pipeline of merging.

Thanks!

> On Feb 2, 2017, at 2:37 PM, Todor Minchev  
> wrote:
> 
> The contents of an existing database file can be extracted in the
> current working directory with the -E option. The top level of the
> directory tree is rmc_db_dump and all files corresponding to
> a given record will be saved in a separate sub-directory. The sub-directory
> name of each record is the signature corresponding to the fingerprint for
> that record.
> 
> Example:
> ./src/rmc -E -d rmc.db
> 
> Successfully extracted rmc.db
> 
> Signed-off-by: Todor Minchev 
> ---
> inc/rmc_api.h |  9 ++
> src/lib/api.c | 85 +--
> src/lib/common/rmcl.c |  3 +-
> src/rmc.c | 44 +++---
> 4 files changed, 126 insertions(+), 15 deletions(-)
> 
> diff --git a/inc/rmc_api.h b/inc/rmc_api.h
> index a484389..ce26220 100644
> --- a/inc/rmc_api.h
> +++ b/inc/rmc_api.h
> @@ -74,6 +74,15 @@ extern int rmc_query_file_by_fp(rmc_fingerprint_t *fp, 
> char *db_pathname, char *
>  */
> extern int rmc_gimme_file(char* db_pathname, char *file_name, rmc_file_t 
> *file);
> 
> +
> +/* extract the contents of a database file and store the files corresponding 
> to
> + * each record in a separate directory. The name of each directory is the 
> signature
> + * of the fingerpring for that record
> + * (in) db_pathname: The path and file name of a RMC database file generated 
> by RMC tool
> + * return: 0 on success, non-zero on failure.
> + */
> +int dump_db(char *db_pathname) ;
> +
Please move this into section 1.3, somewhere after next line. It doesn’t belong 
to section 1.2 “double-action API”

> /* 1.3 - Helper APIs */
> 
> /* Free allocated data referred in a fingerprint
> diff --git a/src/lib/api.c b/src/lib/api.c
> index 0adb390..aca8d99 100644
> --- a/src/lib/api.c
> +++ b/src/lib/api.c
> @@ -3,6 +3,7 @@
>  * RMC API implementation for Linux user space
>  */
> 
> +#define _GNU_SOURCE
> #include 
> #include 
> #include 
> @@ -14,8 +15,11 @@
> #include 
> #include 
> 
> -#define EFI_SYSTAB_PATH "/sys/firmware/efi/systab"
> -#define SYSTAB_LEN 4096 /* assume 4kb is enough...*/
> +#define EFI_SYSTAB_PATH  "/sys/firmware/efi/systab"
> +#define SYSTAB_LEN   4096 /* assume 4kb is enough...*/
> +#define DB_DUMP_DIR  "./rmc_db_dump"  /* directory to store db data dump 
> */
> +
> +extern const rmc_uint8_t rmc_db_signature[RMC_DB_SIG_LEN];
We could have a new small helper API like is_rmcdb(db_blob) in RMCL to hold 
checker logic at line 357 in this file, so that we can get rid of this line and 
make the checker reusable. (So far I feel the checker should work in both EFI 
and Linux contexts.)

We could even update checker API without bothering its callers in the future. 
Let me know if it makes sense...

> 
> int read_file(const char *pathname, char **data, rmc_size_t* len) {
> int fd = -1;
> @@ -325,3 +329,80 @@ int rmc_gimme_file(char* db_pathname, char *file_name, 
> rmc_file_t *file) {
> 
> return ret;
> }
> +
> +/*
> + * Dump contents of database file
> + * (in) rmc_db - input database file to extract
rmc_db VS db_pathname. I think we can remove the comment here, it is already in 
the header file.
> + */
> +int dump_db(char *db_pathname) {
> +rmc_meta_header_t meta_header;
> +rmc_db_header_t *db_header = NULL;
> +rmc_record_header_t record_header;
> +rmc_uint64_t record_idx = 0;   /* offset of each reacord in db*/
> +rmc_uint64_t meta_idx = 0; /* offset of each meta in a record */
> +rmc_uint64_t file_idx = 0; /* offset of file in a meta */
> +rmc_file_t file;
> +char *out_dir = NULL, *out_name = NULL;
> +rmc_size_t db_len = 0;
> +rmc_uint8_t *rmc_db = NULL;
> +struct stat s = {0};
> +
> +if (read_file(db_pathname, (char **)_db, _len)) {
> +fprintf(stderr, "Failed to read database file\n\n");
> +return 1;
> +}
> +
> +db_header = (rmc_db_header_t *)rmc_db;
> +
> +/* sanity check of db */
> +if (strncmp((const char *)db_header->signature,
> +(const char *)rmc_db_signature, RMC_DB_SIG_LEN))
> +return 1;
> +
> +/* create the top level directory */
> +if (stat(DB_DUMP_DIR, ) == -1) {
> +if(mkdir(DB_DUMP_DIR, 0755)) {
> +fprintf(stderr, "Failed to create %s directory\n\n", out_name);
I think we should not go any further when we cannot create the top dir.
> +}
> +}
> +
> +/* query the meta. idx: 

[meta-intel] [PATCH 3/3] rmc: add database extraction functionality

[Todor Minchev]

The contents of an existing database file can be extracted in the
current working directory with the -E option. The top level of the
directory tree is rmc_db_dump and all files corresponding to
a given record will be saved in a separate sub-directory. The sub-directory
name of each record is the signature corresponding to the fingerprint for
that record.

Example:
./src/rmc -E -d rmc.db

Successfully extracted rmc.db

Signed-off-by: Todor Minchev 
---
 inc/rmc_api.h |  9 ++
 src/lib/api.c | 85 +--
 src/lib/common/rmcl.c |  3 +-
 src/rmc.c | 44 +++---
 4 files changed, 126 insertions(+), 15 deletions(-)

diff --git a/inc/rmc_api.h b/inc/rmc_api.h
index a484389..ce26220 100644
--- a/inc/rmc_api.h
+++ b/inc/rmc_api.h
@@ -74,6 +74,15 @@ extern int rmc_query_file_by_fp(rmc_fingerprint_t *fp, char 
*db_pathname, char *
  */
 extern int rmc_gimme_file(char* db_pathname, char *file_name, rmc_file_t 
*file);
 
+
+/* extract the contents of a database file and store the files corresponding to
+ * each record in a separate directory. The name of each directory is the 
signature
+ * of the fingerpring for that record
+ * (in) db_pathname: The path and file name of a RMC database file generated 
by RMC tool
+ * return: 0 on success, non-zero on failure.
+ */
+int dump_db(char *db_pathname) ;
+
 /* 1.3 - Helper APIs */
 
 /* Free allocated data referred in a fingerprint
diff --git a/src/lib/api.c b/src/lib/api.c
index 0adb390..aca8d99 100644
--- a/src/lib/api.c
+++ b/src/lib/api.c
@@ -3,6 +3,7 @@
  * RMC API implementation for Linux user space
  */
 
+#define _GNU_SOURCE
 #include 
 #include 
 #include 
@@ -14,8 +15,11 @@
 #include 
 #include 
 
-#define EFI_SYSTAB_PATH "/sys/firmware/efi/systab"
-#define SYSTAB_LEN 4096 /* assume 4kb is enough...*/
+#define EFI_SYSTAB_PATH  "/sys/firmware/efi/systab"
+#define SYSTAB_LEN   4096 /* assume 4kb is enough...*/
+#define DB_DUMP_DIR  "./rmc_db_dump"  /* directory to store db data dump */
+
+extern const rmc_uint8_t rmc_db_signature[RMC_DB_SIG_LEN];
 
 int read_file(const char *pathname, char **data, rmc_size_t* len) {
 int fd = -1;
@@ -325,3 +329,80 @@ int rmc_gimme_file(char* db_pathname, char *file_name, 
rmc_file_t *file) {
 
 return ret;
 }
+
+/*
+ * Dump contents of database file
+ * (in) rmc_db - input database file to extract
+ */
+int dump_db(char *db_pathname) {
+rmc_meta_header_t meta_header;
+rmc_db_header_t *db_header = NULL;
+rmc_record_header_t record_header;
+rmc_uint64_t record_idx = 0;   /* offset of each reacord in db*/
+rmc_uint64_t meta_idx = 0; /* offset of each meta in a record */
+rmc_uint64_t file_idx = 0; /* offset of file in a meta */
+rmc_file_t file;
+char *out_dir = NULL, *out_name = NULL;
+rmc_size_t db_len = 0;
+rmc_uint8_t *rmc_db = NULL;
+struct stat s = {0};
+
+if (read_file(db_pathname, (char **)_db, _len)) {
+fprintf(stderr, "Failed to read database file\n\n");
+return 1;
+}
+
+db_header = (rmc_db_header_t *)rmc_db;
+
+/* sanity check of db */
+if (strncmp((const char *)db_header->signature,
+(const char *)rmc_db_signature, RMC_DB_SIG_LEN))
+return 1;
+
+/* create the top level directory */
+if (stat(DB_DUMP_DIR, ) == -1) {
+if(mkdir(DB_DUMP_DIR, 0755)) {
+fprintf(stderr, "Failed to create %s directory\n\n", out_name);
+}
+}
+
+/* query the meta. idx: start of record */
+record_idx = sizeof(rmc_db_header_t);
+while (record_idx < db_header->length) {
+memcpy(_header, rmc_db + record_idx,
+sizeof(rmc_record_header_t));
+
+/* directory name is fingerprint signature */
+asprintf(_dir, "%s/%s/", DB_DUMP_DIR, record_header.signature.raw);
+if (stat(out_dir, ) == -1) {
+if(mkdir(out_dir, 0755)) {
+fprintf(stderr, "Failed to create %s directory\n\n", out_name);
+}
+}
+
+/* find meta */
+for (meta_idx = record_idx + sizeof(rmc_record_header_t);
+meta_idx < record_idx + record_header.length;) {
+memcpy(_header, rmc_db + meta_idx, sizeof(rmc_meta_header_t));
+file_idx = meta_idx + sizeof(rmc_meta_header_t);
+rmc_ssize_t name_len = strlen((char *)_db[file_idx]) + 1;
+file.blob = _db[file_idx + name_len];
+file.blob_len = meta_header.length - sizeof(rmc_meta_header_t) -
+name_len;
+file.next = NULL;
+file.type = RMC_GENERIC_FILE;
+asprintf(_name, "%s%s", out_dir, (char *)_db[file_idx]);
+/* write file to dump directory */
+if (write_file((const char *)out_name, file.blob, file.blob_len, 
0))
+return 1;
+
+/* next meta */
+meta_idx +=