Re: [PATCH] common: The do_repeat flag interferes with commands issued via run_command API

2021-05-28 Thread Farhan Ali
Hi Sean,

Thanks for taking a look. I will add the changes you requested. However a
test case might be difficult to add in the automated tests. This problem
only happens if there is an asynchronous event ( packet received/Button
pressed ) which triggers a run_command API call AND when the user has
entered some console commands which have enabled the do_repeat flag. If the
do_repeat flag is set AND the command issued via the run_command API is a
'non-repeatable' command (e.g mmc write), the command is ignored.

On Thu, May 27, 2021 at 4:24 PM Sean Anderson  wrote:

> > Re: [PATCH] common: The do_repeat flag interferes with commands issued
> via run_command API
>
> The tag here should be "hush: ..." The subject should be an action,
> such as "Clear do_repeat flag after running commands".
>
> On 5/27/21 5:24 PM, Farhan Ali wrote:
> > Must clear the do_repeat flag once it is consumed.
>
> What is "it" here? Please add a few more sentences describing why you
> want to change this. For example, you could note that do_repeat is a
> file-level variable which is used by get_user_input to signal when a
> command should be repeated. Though, I wonder why we don't set the flags
> in the first place...
>
> 'it' refers to the do_repeat flag.

> >
> > Signed-off-by: Farhan Ali 
> > Cc: Simon Glass 
> > Cc: Sean Anderson 
> > Cc: Rasmus Villemoes 
> > Cc: Farhan Ali 
> > Cc: "peng.w...@smartm.com" 
> > Cc: Patrick Delaunay 
> >
> > ---
> >   common/cli_hush.c | 5 +
> >   1 file changed, 5 insertions(+)
> >
> > diff --git a/common/cli_hush.c b/common/cli_hush.c
> > index 1467ff81b3..1c9adf5683 100644
> > --- a/common/cli_hush.c
> > +++ b/common/cli_hush.c
> > @@ -1559,6 +1559,11 @@ static int run_pipe_real(struct pipe *pi)
> >   # endif
> >   #endif  /* __U_BOOT__ */
> >
> > + /* Clear do_repeat after consumption, avoids conflicts
>
> Multi-line comments should start with a blank line.
>
> > +  * with cmds issued  via run_command API
> > +  */
> > + do_repeat = 0;
> > +
>
> Can you add a test case for this?
>
> So a use case could be as follows:
(1) User issues several repeated commands on console. This sets do_repeat
flag internally
(2) Webserver embedded in main polling loop of uboot detects an image
upgrade request
(3) Image downloads, about to be written via run_command("mmc write etc.")
(4) Since 'mmc write' is a non-repeatable command, issuing it with
do_repeat flag set results in command getting ignored
(5) Image upgrade fails

> --Sean
>
> >   nextin = 0;
> >   #ifndef __U_BOOT__
> >   pi->pgrp = -1;
> >
>
>

Regards,

Farhan


smime.p7s
Description: S/MIME Cryptographic Signature


[PATCH] common: The do_repeat flag interferes with commands issued via run_command API

2021-05-27 Thread Farhan Ali
Must clear the do_repeat flag once it is consumed.

Signed-off-by: Farhan Ali 
Cc: Simon Glass 
Cc: Sean Anderson 
Cc: Rasmus Villemoes 
Cc: Farhan Ali 
Cc: "peng.w...@smartm.com" 
Cc: Patrick Delaunay 

---
 common/cli_hush.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/common/cli_hush.c b/common/cli_hush.c
index 1467ff81b3..1c9adf5683 100644
--- a/common/cli_hush.c
+++ b/common/cli_hush.c
@@ -1559,6 +1559,11 @@ static int run_pipe_real(struct pipe *pi)
 # endif
 #endif /* __U_BOOT__ */
 
+   /* Clear do_repeat after consumption, avoids conflicts
+* with cmds issued  via run_command API
+*/
+   do_repeat = 0;
+
nextin = 0;
 #ifndef __U_BOOT__
pi->pgrp = -1;
-- 
2.27.0



Re: [PATCH v2] spl: Add callback for preprocessing loaded FIT header before parsing

2021-03-29 Thread Farhan Ali
Phillipe,

In our implementation we store our binaries outside the FIT header, and
introduce a gap between the header and the start of binary data (-p and -E
option in mkimage). After the FIT has been generated, we sign the FIT
header and insert the signature into this gap. The weak function then
checks the signature after 'only' the header has been loaded, but before
any of the FIT fields have been parsed.

Whatever common implementation we decide on, it is imperative that the
signature can be inserted 'AFTER' the complete FIT has been generated. The
reason this is so critical is to allow for off-line signing via customer
HSMs.

Regards,
Farhan

On Wed, Mar 24, 2021 at 12:09 AM Simon Glass  wrote:

> Hi Philippe,
>
> On Wed, 24 Mar 2021 at 06:16, Philippe REYNES
>  wrote:
> >
> > Hi Simon and Alex,
> >
> > Le 23/03/2021 à 01:56, Simon Glass a écrit :
> > > Hi Alex,
> > >
> > > On Tue, 23 Mar 2021 at 04:12, Alex G.  wrote:
> > >> On 3/22/21 9:27 AM, Philippe REYNES wrote:
> > >>> Hi all,
> > >>>
> > >>>
> > >>> Le 11/03/2021 à 00:10, Alex G a écrit :
> > >> [snip]
> > >>> I reach the same issue, my customers are also worried with the actual
> > >>> signature check scheme on u-boot.
> > >>> The fit data/node are parsed before being checked : data should be
> used
> > >>> only after being checked, not before.
> > >>> The code become quite complex for a signature, and the more complex
> the
> > >>> code is more risk to have/introduce a bug or security issue.
> > >> [snip]
> > >>
> > > The reason I used a weak function was to mirror the already
> > > upstreamed board_spl_fit_post_load(),
> >  I see why you'd think it was a good idea. board_spl_fit_pre_load()
> >  sneaks in a dependency on arch-specific code (CONFIG_IMX_HAB). I
> don't
> >  really like the way it's implemented, and I don't know if it would
> >  work with SPL_LOAD_FIT_FULL or bootm.
> > 
> > >>> As I reach the same issue, I was also thinking strongly about adding
> a
> > >>> "hook" before the fit image is launched/analyzed. In my mind this
> "pre
> > >>> load" function should be able to do some check/update to the fit
> image,
> > >>> but also modify the beginning of the fit image (to remove a header
> for
> > >>> example). Such function/feature may allow to:
> > >>> - check a signature for the full fit (without parsing the node)
> > >>> - cipher the full fit (even the node)
> > >>> - compress the full fit
> > >>> - probably that users will find a lot of others ideas .
> > >>>
> > >>> I think that this feature pre load should be implemented in spl and
> > >>> bootm command.
> > >>>
> > >>> I have understood the feedback about a useful implementation/usage of
> > >>> pre_load.
> > >>> I propose to sent an example soon (probably based on signature
> check).
> > >> So "what" you want to do is verify untrusted metadata before using it.
> > >> That's a very logical and reasonable thing to do.
> > >>
> > >> "How" you are trying to do this is by
> > >>(1) adding a weak function
> > >>(2) allowing each board to have a completely different
> implementation
> > >>
> > >> Those are two terrible ideas.
> > >>
> > >> I agree that there is a deficiency in the way FIT images are signed.
> Can
> > >> we stick the signature between the fdt_header and before dt_struct?
> > > That seems like a reasonable idea to me. Even better might be to have
> > > it completely separate, e.g. before the FIT starts, so no parsing at
> > > all is needed?
> >
> >
> > That's my idea, a header with only the minimum information (like fit
> > size and signature).
> > The information about the signature (hash, algo, padding, public key,
> > ...) may be stored
> > in the u-boot device tree. So u-boot won't parse the fit image, only
> > compute the hash
> > to check the signature.
> >
> > >
> > > Also, which signature? FIT supports multiple signatures which can be
> > > added at different times. Perhaps this could be for a base signature,
> > > enough to get through to verifying the 'real' signature.
> >
> >
> > I was thinking that the signature information could be stored in the
> > u-boot device tree
> > (or hardcoded in the u-boot configuration). The idea is to have a very
> > simple header.
> > I also think that this signature may be used with the signature in the
> > fit.  It is two
> > options, so users may eanble both options.
> >
> > As we agree on the principle, I will sent a RFC asap.
>
> You can store the public key (or whatever is used) in the U-Boot
> devicetree, but the signature presumably has to be attached to the
> FIT, right?
>
> Regards,
> Simon
>


smime.p7s
Description: S/MIME Cryptographic Signature


Re: [PATCH v2] spl: Add callback for preprocessing loaded FIT header before parsing

2021-03-10 Thread Farhan Ali
On Wed, Mar 10, 2021 at 11:38 AM Alex G.  wrote:

> On 3/9/21 5:55 PM, Farhan Ali wrote:
> > This change adds a callback for preprocessing the FIT header before
> > it is parsed. There are 3 main reasons for this callback:
> >
> > (1) If a vulnerability is discovered in the FIT parsing/loading code,
> > or libfdt, this callback allows users to scan the FIT header for
> > specific exploit signatures and prevent flashing/booting of the image
> >
> > (2) If users want to implement a single signature which covers the
> > entire FIT header, which is then appended to the end of the header,
> > then this callback can be used to authenticate that signature.
> >
> > (3) If users want to check the FIT header contents against specific
> > metadata stored outside the FIT header, then this callback allows
> > them to do that.
>
> Hi Fahran,
>
> This patch describes "how" you're trying to achieve it, but "what" you
> want to achieve. I'll get later into why I think the "how" is
> fundamentally flawed.
>
> Hi Alex,
The 'what' is basically this: I want to be able to parse the fit header for
correctness before
any image loading takes place. This 'correctness' will be user defined
The 'how' is this: A weak function which is invoked right after the fit
HEADER ONLY is read.

There should be at least a use case implemented in this series. When
> someone notices an empty function that isn't used, the first instinct
> would be to submit a patch to remove it. But more importantly, seeing an
> actual example of "what" you are trying to achieve will help others
> suggest a better way on "how" to achieve it.
>
>  The main use case for us is two folds:
(1) Customers are worried about our reliance on libfdt for FIT parsing and
want to prescan the FIT header to
check for any future exploits
(2) We implement a signature on the entire FIT header ( instead of
individual nodes ). This speeds
up the signing process for production/development builds. We want to be
able validate this signature
before the FIT parsing starts. Signature is stored elsewhere, outside the
FIT header.

Second issue is that spl_simple_fit_read() is intended to bring a FIT
> image to memory. If you need to make decisions on the content of that
> image, then spl_simple_fit_read() is the wrong place to do it. A better
> place might be spl_simple_fit_parse().
>
spl_simple_fit_parse()  parses the 'contents' of the fit using standard
APIs. We need to check
the FIT header for correctness BEFORE its contents are parsed, using a user
defined 'safe'
parsing function. The standard FIT loading flow checks for only a few
things ( hashes/configuration etc),
there can be a lot of other USER defined checks which may need to be
checked. This callback will achieve this

The third issue is that whatever the end goal is, you're trying to
> achieve it with weak functions. Weak functions aren't always bad. There
> are a limited number of cases where they work very well for the purpose
> -- I've even used them before. But to introduce a weak function, a
> really strong argument is needed. Maybe you have it, but that argument
> needs to be made clear.
>
> The reason I used a weak function was to mirror the already
upstreamed board_spl_fit_post_load(), I
thought that the justifications for that function would also apply to this
new board_spl_fit_pre_load().
If board_spl_fit_pre_load() is fundamentally different in some aspect, then
I am happy to look for an
alternative implementation.

Regards,

Farhan

> Let's assume board 'c' implements this. Then later someone with board
> 'd' implements this at the SOC level. Does board 'c' get the old
> implementation, or the new? Things become ambiguous in everything but
> the simplest of cases.
>
> A more elegant way would be to have a proper interface to hook into the
> FIT processing. That could be done by a function pointer, ops structure,
> or perhaps new UCLASS (Simon?).
>
> Alex
>
>
>
>
> > Signed-off-by: Farhan Ali 
> > Cc: Simon Glass 
> > Cc: Alexandru Gagniuc 
> > Cc: Marek Vasut 
> > Cc: Michal Simek 
> > Cc: Philippe Reynes 
> > Cc: Samuel Holland 
> >
> > ---
> > Changes for v2:
> > - Callback now returns a value
> > - Added a log message on failure
> > ---
> >   common/spl/spl_fit.c | 22 +-
> >   1 file changed, 21 insertions(+), 1 deletion(-)
> >
> > diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
> > index 75c8ff0..01aee1c 100644
> > --- a/common/spl/spl_fit.c
> > +++ b/common/spl/spl_fit.c
> > @@ -43,6 +43,14 @@ __weak ulong board_spl_fit_size_align(ulong size)
> >   return size;
> >   }
> >
> > +

[PATCH v2] spl: Add callback for preprocessing loaded FIT header before parsing

2021-03-09 Thread Farhan Ali
This change adds a callback for preprocessing the FIT header before
it is parsed. There are 3 main reasons for this callback:

(1) If a vulnerability is discovered in the FIT parsing/loading code,
or libfdt, this callback allows users to scan the FIT header for
specific exploit signatures and prevent flashing/booting of the image

(2) If users want to implement a single signature which covers the
entire FIT header, which is then appended to the end of the header,
then this callback can be used to authenticate that signature.

(3) If users want to check the FIT header contents against specific
metadata stored outside the FIT header, then this callback allows
them to do that.

Signed-off-by: Farhan Ali 
Cc: Simon Glass 
Cc: Alexandru Gagniuc 
Cc: Marek Vasut 
Cc: Michal Simek 
Cc: Philippe Reynes 
Cc: Samuel Holland 

---
Changes for v2:
   - Callback now returns a value
   - Added a log message on failure
---
 common/spl/spl_fit.c | 22 +-
 1 file changed, 21 insertions(+), 1 deletion(-)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 75c8ff0..01aee1c 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -43,6 +43,14 @@ __weak ulong board_spl_fit_size_align(ulong size)
return size;
 }
 
+__weak int board_spl_fit_pre_load(struct spl_load_info *load_info,
+ const void *fit,
+ ulong start_sector,
+ ulong loaded_sector_count)
+{
+   return 0;
+}
+
 static int find_node_from_desc(const void *fit, int node, const char *str)
 {
int child;
@@ -527,6 +535,7 @@ static int spl_simple_fit_read(struct spl_fit_info *ctx,
unsigned long count, size;
int sectors;
void *buf;
+   int rc = 0;
 
/*
 * For FIT with external data, figure out where the external images
@@ -552,7 +561,18 @@ static int spl_simple_fit_read(struct spl_fit_info *ctx,
debug("fit read sector %lx, sectors=%d, dst=%p, count=%lu, 
size=0x%lx\n",
  sector, sectors, buf, count, size);
 
-   return (count == 0) ? -EIO : 0;
+   if (count) {
+   /* preprocess loaded fit header before parsing and loading 
binaries */
+   rc = board_spl_fit_pre_load(info, fit_header, sector, sectors);
+   if (rc) {
+   debug("%s: fit header pre processing failed. rc=%d\n",
+ __func__, rc);
+   }
+   } else {
+   rc = -EIO;
+   }
+
+   return rc;
 }
 
 static int spl_simple_fit_parse(struct spl_fit_info *ctx)
-- 
1.8.3.1



[PATCH v2] cmd: gpt: Add option to write GPT partitions to environment variable

2021-02-26 Thread Farhan Ali
This change would enhance the existing 'gpt read' command to allow
(optionally) writing of the read GPT partitions to an environment
variable in the UBOOT partitions layout format. This would allow users
to easily change the overall partition settings by editing said variable
and then using the variable in the 'gpt write' and 'gpt verify' commands.

Signed-off-by: Farhan Ali 
Cc: Simon Glass 
Cc: Heinrich Schuchardt 
Cc: Corneliu Doban 
Cc: Rayagonda Kokatanur 
Cc: Rasmus Villemoes 

---
Changes for v2:
   - Checked for argv[4] existence before calling do_get_gpt_info
   - Added missing update to doc/README.gpt
---
 cmd/gpt.c  | 46 ++
 doc/README.gpt | 17 +
 2 files changed, 55 insertions(+), 8 deletions(-)

diff --git a/cmd/gpt.c b/cmd/gpt.c
index 76a95ad..17f2b83 100644
--- a/cmd/gpt.c
+++ b/cmd/gpt.c
@@ -350,17 +350,46 @@ static int get_gpt_info(struct blk_desc *dev_desc)
 }
 
 /* a wrapper to test get_gpt_info */
-static int do_get_gpt_info(struct blk_desc *dev_desc)
+static int do_get_gpt_info(struct blk_desc *dev_desc, char * const namestr)
 {
-   int ret;
+   int numparts;
+
+   numparts = get_gpt_info(dev_desc);
+
+   if (numparts > 0) {
+   if (namestr) {
+   char disk_guid[UUID_STR_LEN + 1];
+   char *partitions_list;
+   int partlistlen;
+   int ret = -1;
+
+   ret = get_disk_guid(dev_desc, disk_guid);
+   if (ret < 0)
+   return ret;
+
+   partlistlen = calc_parts_list_len(numparts);
+   partitions_list = malloc(partlistlen);
+   if (!partitions_list) {
+   del_gpt_info();
+   return -ENOMEM;
+   }
+   memset(partitions_list, '\0', partlistlen);
+
+   ret = create_gpt_partitions_list(numparts, disk_guid,
+partitions_list);
+   if (ret < 0)
+   printf("Error: Could not create partition list 
string!\n");
+   else
+   env_set(namestr, partitions_list);
 
-   ret = get_gpt_info(dev_desc);
-   if (ret > 0) {
-   print_gpt_info();
+   free(partitions_list);
+   } else {
+   print_gpt_info();
+   }
del_gpt_info();
return 0;
}
-   return ret;
+   return numparts;
 }
 #endif
 
@@ -982,7 +1011,7 @@ static int do_gpt(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
ret = do_disk_guid(blk_dev_desc, argv[4]);
 #ifdef CONFIG_CMD_GPT_RENAME
} else if (strcmp(argv[1], "read") == 0) {
-   ret = do_get_gpt_info(blk_dev_desc);
+   ret = do_get_gpt_info(blk_dev_desc, (argc == 5) ? argv[4] : 
NULL);
} else if ((strcmp(argv[1], "swap") == 0) ||
   (strcmp(argv[1], "rename") == 0)) {
ret = do_rename_gpt_parts(blk_dev_desc, argv[1], argv[4], 
argv[5]);
@@ -1028,8 +1057,9 @@ U_BOOT_CMD(gpt, CONFIG_SYS_MAXARGS, 1, do_gpt,
" gpt guid mmc 0 varname\n"
 #ifdef CONFIG_CMD_GPT_RENAME
"gpt partition renaming commands:\n"
-   " gpt read  \n"
+   " gpt read   []\n"
"- read GPT into a data structure for manipulation\n"
+   "- read GPT partitions into environment variable\n"
" gpt swap\n"
"- change all partitions named name1 to name2\n"
"  and vice-versa\n"
diff --git a/doc/README.gpt b/doc/README.gpt
index ac975f6..91e397d 100644
--- a/doc/README.gpt
+++ b/doc/README.gpt
@@ -237,6 +237,23 @@ doc/arch/index.rst:
 => gpt swap host 0 name othername
 [ . . . ]
 
+Modifying GPT partition layout from U-Boot:
+===
+
+The entire GPT partition layout can be exported to an environment
+variable and then modified enmasse. Users can change the partition
+numbers, offsets, names and sizes. The resulting variable can used to
+reformat the device. Here is an example of reading the GPT partitions
+into a variable and then modifying them:
+
+U-BOOT> gpt read mmc 0 current_partitions
+U-BOOT> env edit current_partitions
+edit: uuid_disk=[...];name=part1,start=0x4000,size=0x4000,uuid=[...];
+name=part2,start=0xc000,size=0xc000,uuid=[...];[ . . . ]
+
+U-BOOT> gpt write mmc 0 $current_partitions
+U-BOOT> gpt verify mmc 0 $current_partitions
+
 Partition type GUID:
 
 
-- 
1.8.3.1



Re: [PATCH] cmd: gpt: Add option to write GPT partitions to environment variable

2021-02-25 Thread Farhan Ali
Hi Heinrich,

Thank you for your comments. I will add your recommendations and resubmit,
I just want to clarify the use case here:

-Right now we have commands to read the GPT table into an internal data
structure, and from then on we can only rename or swap partitions
-If a user wants to modify all aspects of the partition layout, they
currently dont have any way to do so from the uboot command line
-With this proposed change, users would be able to export the entire
partition table to a uboot environment variable and then edit all of it
enmasse

Regards, Farhan




On Thu, Feb 25, 2021 at 12:03 PM Heinrich Schuchardt 
wrote:

> On 2/25/21 7:56 PM, Farhan Ali wrote:
> > This change would enhance the existing 'gpt read' command to allow
> > (optionally) writing of the read GPT partitions to an environment
> > variable. This would allow users to easily change the partition
> > settings and then simply reuse the variable in the 'gpt write' and
> > 'gpt verify' commands.
> >
> > Signed-off-by: Farhan Ali 
>
> Hello Farhan,
>
> It is unclear what your use case is.
>
> 'gpt read' already reads the data into a data structure for
> manipulation. See doc/README.gpt.
>
> Please, provide an example showing how you will use the variable.
>
> > ---
> > Cc: Simon Glass 
> > Cc: Heinrich Schuchardt 
> > Cc: Corneliu Doban 
> > Cc: Rayagonda Kokatanur 
> >
> >   cmd/gpt.c | 46 ++
> >   1 file changed, 38 insertions(+), 8 deletions(-)
> >
> > diff --git a/cmd/gpt.c b/cmd/gpt.c
> > index 76a95ad..12d0551 100644
> > --- a/cmd/gpt.c
> > +++ b/cmd/gpt.c
> > @@ -350,17 +350,46 @@ static int get_gpt_info(struct blk_desc *dev_desc)
> >   }
> >
> >   /* a wrapper to test get_gpt_info */
> > -static int do_get_gpt_info(struct blk_desc *dev_desc)
> > +static int do_get_gpt_info(struct blk_desc *dev_desc, char * const
> namestr)
> >   {
> > - int ret;
> > + int numparts;
> > +
> > + numparts = get_gpt_info(dev_desc);
> > +
> > + if (numparts > 0) {
> > + if (namestr) {
>
> If the parameter is missing, the caller passes random bytes and not
> NULL. So this check does not work.
>
> > + char disk_guid[UUID_STR_LEN + 1];
> > + char *partitions_list;
> > + int partlistlen;
> > + int ret = -1;
> > +
> > + ret = get_disk_guid(dev_desc, disk_guid);
> > + if (ret < 0)
> > + return ret;
> > +
> > + partlistlen = calc_parts_list_len(numparts);
> > + partitions_list = malloc(partlistlen);
> > + if (!partitions_list) {
> > + del_gpt_info();
> > + return -ENOMEM;
> > + }
> > + memset(partitions_list, '\0', partlistlen);
> > +
> > + ret = create_gpt_partitions_list(numparts,
> disk_guid,
> > +  partitions_list);
> > + if (ret < 0)
> > + printf("Error: Could not create partition
> list string!\n");
> > + else
> > + env_set(namestr, partitions_list);
> >
> > - ret = get_gpt_info(dev_desc);
> > - if (ret > 0) {
> > - print_gpt_info();
> > + free(partitions_list);
> > + } else {
> > + print_gpt_info();
> > + }
> >   del_gpt_info();
> >   return 0;
> >   }
> > - return ret;
> > + return numparts;
> >   }
> >   #endif
> >
> > @@ -982,7 +1011,7 @@ static int do_gpt(struct cmd_tbl *cmdtp, int flag,
> int argc, char *const argv[])
> >   ret = do_disk_guid(blk_dev_desc, argv[4]);
> >   #ifdef CONFIG_CMD_GPT_RENAME
> >   } else if (strcmp(argv[1], "read") == 0) {
> > - ret = do_get_gpt_info(blk_dev_desc);
> > + ret = do_get_gpt_info(blk_dev_desc, argv[4]);
>
> You have to check argc to know if argv[4] exists and pass argv[4] or
> NULL accordingly.
>
> >   } else if ((strcmp(argv[1], "swap") == 0) ||
> >  (strcmp(argv[1], "rename") == 0)) {
> >   ret = do_rename_gpt_parts(blk_dev_desc, argv[1], argv[4],
> argv[5]);
> >

[PATCH] cmd: gpt: Add option to write GPT partitions to environment variable

2021-02-25 Thread Farhan Ali
This change would enhance the existing 'gpt read' command to allow
(optionally) writing of the read GPT partitions to an environment
variable. This would allow users to easily change the partition
settings and then simply reuse the variable in the 'gpt write' and
'gpt verify' commands.

Signed-off-by: Farhan Ali 
---
Cc: Simon Glass 
Cc: Heinrich Schuchardt 
Cc: Corneliu Doban 
Cc: Rayagonda Kokatanur 

 cmd/gpt.c | 46 ++
 1 file changed, 38 insertions(+), 8 deletions(-)

diff --git a/cmd/gpt.c b/cmd/gpt.c
index 76a95ad..12d0551 100644
--- a/cmd/gpt.c
+++ b/cmd/gpt.c
@@ -350,17 +350,46 @@ static int get_gpt_info(struct blk_desc *dev_desc)
 }
 
 /* a wrapper to test get_gpt_info */
-static int do_get_gpt_info(struct blk_desc *dev_desc)
+static int do_get_gpt_info(struct blk_desc *dev_desc, char * const namestr)
 {
-   int ret;
+   int numparts;
+
+   numparts = get_gpt_info(dev_desc);
+
+   if (numparts > 0) {
+   if (namestr) {
+   char disk_guid[UUID_STR_LEN + 1];
+   char *partitions_list;
+   int partlistlen;
+   int ret = -1;
+
+   ret = get_disk_guid(dev_desc, disk_guid);
+   if (ret < 0)
+   return ret;
+
+   partlistlen = calc_parts_list_len(numparts);
+   partitions_list = malloc(partlistlen);
+   if (!partitions_list) {
+   del_gpt_info();
+   return -ENOMEM;
+   }
+   memset(partitions_list, '\0', partlistlen);
+
+   ret = create_gpt_partitions_list(numparts, disk_guid,
+partitions_list);
+   if (ret < 0)
+   printf("Error: Could not create partition list 
string!\n");
+   else
+   env_set(namestr, partitions_list);
 
-   ret = get_gpt_info(dev_desc);
-   if (ret > 0) {
-   print_gpt_info();
+   free(partitions_list);
+   } else {
+   print_gpt_info();
+   }
del_gpt_info();
return 0;
}
-   return ret;
+   return numparts;
 }
 #endif
 
@@ -982,7 +1011,7 @@ static int do_gpt(struct cmd_tbl *cmdtp, int flag, int 
argc, char *const argv[])
ret = do_disk_guid(blk_dev_desc, argv[4]);
 #ifdef CONFIG_CMD_GPT_RENAME
} else if (strcmp(argv[1], "read") == 0) {
-   ret = do_get_gpt_info(blk_dev_desc);
+   ret = do_get_gpt_info(blk_dev_desc, argv[4]);
} else if ((strcmp(argv[1], "swap") == 0) ||
   (strcmp(argv[1], "rename") == 0)) {
ret = do_rename_gpt_parts(blk_dev_desc, argv[1], argv[4], 
argv[5]);
@@ -1028,8 +1057,9 @@ U_BOOT_CMD(gpt, CONFIG_SYS_MAXARGS, 1, do_gpt,
" gpt guid mmc 0 varname\n"
 #ifdef CONFIG_CMD_GPT_RENAME
"gpt partition renaming commands:\n"
-   " gpt read  \n"
+   " gpt read   []\n"
"- read GPT into a data structure for manipulation\n"
+   "- read GPT partitions into environment variable\n"
" gpt swap\n"
"- change all partitions named name1 to name2\n"
"  and vice-versa\n"
-- 
1.8.3.1



[PATCH] mtd: Update fail_addr when erase fails due to bad blocks

2021-02-24 Thread Farhan Ali
For all other erase failures, the fail_addr is updated with the
failing address. Only in the case of erase failure due to bad block
detection, the fail_addr is not updated. This change simply updates
the fail_addr for this specific scenario so that it is consistent with
the rest of the code.

Signed-off-by: Farhan Ali 
---
Cc: Simon Glass 

 drivers/mtd/nand/raw/nand_base.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/mtd/nand/raw/nand_base.c b/drivers/mtd/nand/raw/nand_base.c
index 6557fad..3679ee7 100644
--- a/drivers/mtd/nand/raw/nand_base.c
+++ b/drivers/mtd/nand/raw/nand_base.c
@@ -3559,6 +3559,8 @@ int nand_erase_nand(struct mtd_info *mtd, struct 
erase_info *instr,
pr_warn("%s: attempt to erase a bad block at page 
0x%08x\n",
__func__, page);
instr->state = MTD_ERASE_FAILED;
+   instr->fail_addr =
+   ((loff_t)page << chip->page_shift);
goto erase_exit;
}
 
-- 
1.8.3.1



smime.p7s
Description: S/MIME Cryptographic Signature


[PATCH] spl: Add callback for preprocessing loaded FIT header before parsing

2021-02-24 Thread Farhan Ali
This change adds a callback for preprocessing the FIT header before
it is parsed. There are 3 main reasons for this callback:

(1) If a vulnerability is discovered in the FIT parsing/loading code,
or libfdt, this callback allows users to scan the FIT header for
specific exploit signatures and prevent flashing/booting of the image

(2) If users want to implement a single signature which covers the
entire FIT header, which is then appended to the end of the header,
then this callback can be used to authenticate that signature.

(3) If users want to check the FIT header contents against specific
metadata stored outside the FIT header, then this callback allows
them to do that.

Signed-off-by: Farhan Ali 
---
Cc: Simon Glass 
Cc: Alexandru Gagniuc 
Cc: Marek Vasut 
Cc: Michal Simek 
Cc: Philippe Reynes 
Cc: Samuel Holland 

 common/spl/spl_fit.c | 9 +
 1 file changed, 9 insertions(+)

diff --git a/common/spl/spl_fit.c b/common/spl/spl_fit.c
index 75c8ff0..e03c67b 100644
--- a/common/spl/spl_fit.c
+++ b/common/spl/spl_fit.c
@@ -43,6 +43,12 @@ __weak ulong board_spl_fit_size_align(ulong size)
return size;
 }
 
+__weak void board_spl_fit_pre_load(struct spl_load_info *load_info, void *fit,
+  ulong start_sector,
+  ulong loaded_sector_count)
+{
+}
+
 static int find_node_from_desc(const void *fit, int node, const char *str)
 {
int child;
@@ -552,6 +558,9 @@ static int spl_simple_fit_read(struct spl_fit_info *ctx,
debug("fit read sector %lx, sectors=%d, dst=%p, count=%lu, 
size=0x%lx\n",
  sector, sectors, buf, count, size);
 
+   /* preprocess loaded fit header before parsing and loading binaries */
+   board_spl_fit_pre_load(info, fit_header, sector, sectors);
+
return (count == 0) ? -EIO : 0;
 }
 
-- 
1.8.3.1



smime.p7s
Description: S/MIME Cryptographic Signature