Re: [PATCH 3/3] mkimge: Reject signing-related flags without FIT_SIGNATURE
On Mon, 7 Dec 2020 at 17:57, Philippe REYNES
wrote:
>
> Hi Joel,
>
> Sorry for this very late answer.
>
> You're right, this is a bad behaviour, but I think that this
> solution remove too many options. For example, If signature
> is disabled, this solution also disable the padding in the fit image.
Ok. I can amend my patch but it wasn't obvious which commands should
be moved from outside the FIT_SIGNATURE guard.
Just -E and -B?
>
> Looking a bit deeper, this patch removes all options that are
> not displayed by the help of mkimage when signature is disabled.
> But I think that this help is not correct. If the signature is disabled,
> the padding is still available.
> So I think that we have an issue in the help too.
>
> Regards,
> Philippe
>
> Le 11/11/2020 à 12:18, Joel Stanley a écrit :
> > When CONFIG_FIT_SIGNATURE=n the signing options are not available. If a
> > user is careful they will notice this when looking at the help output.
> >
> > If they are not careful they will waste several hours wondering what
> > they got wrong, as mkimage will silently ignore the signing related
> > options.
> >
> > Make it obvious that the commands don't work by removing them from the
> > getopt opt_string.
> >
> > The tool will now behave as follows:
> >
> > $ mkimage -f machine.its -k keys -K u-boot-pubkey.dtb -r image.fit
> > mkimage: invalid option -- 'k'
> > Error: Invalid option
> >
> > Signed-off-by: Joel Stanley
> > ---
> > tools/mkimage.c | 9 +++--
> > 1 file changed, 7 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/mkimage.c b/tools/mkimage.c
> > index e78608293e72..10a1b3dc8c18 100644
> > --- a/tools/mkimage.c
> > +++ b/tools/mkimage.c
> > @@ -142,6 +142,12 @@ static int add_content(int type, const char *fname)
> > return 0;
> > }
> >
> > +#ifdef CONFIG_FIT_SIGNATURE
> > +#define OPT_STRING "a:A:b:B:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qstT:vVx"
> > +#else
> > +#define OPT_STRING "a:A:b:C:d:D:e:f:i:ln:O:R:qstT:vVx"
> > +#endif
> > +
> > static void process_args(int argc, char **argv)
> > {
> > char *ptr;
> > @@ -149,8 +155,7 @@ static void process_args(int argc, char **argv)
> > char *datafile = NULL;
> > int opt;
> >
> > - while ((opt = getopt(argc, argv,
> > -"a:A:b:B:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qstT:vVx")) !=
> > -1) {
> > + while ((opt = getopt(argc, argv, OPT_STRING)) != -1) {
> > switch (opt) {
> > case 'a':
> > params.addr = strtoull(optarg, , 16);
Re: [PATCH 3/3] mkimge: Reject signing-related flags without FIT_SIGNATURE
Hi Joel,
Sorry for this very late answer.
You're right, this is a bad behaviour, but I think that this
solution remove too many options. For example, If signature
is disabled, this solution also disable the padding in the fit image.
Looking a bit deeper, this patch removes all options that are
not displayed by the help of mkimage when signature is disabled.
But I think that this help is not correct. If the signature is disabled,
the padding is still available.
So I think that we have an issue in the help too.
Regards,
Philippe
Le 11/11/2020 à 12:18, Joel Stanley a écrit :
When CONFIG_FIT_SIGNATURE=n the signing options are not available. If a
user is careful they will notice this when looking at the help output.
If they are not careful they will waste several hours wondering what
they got wrong, as mkimage will silently ignore the signing related
options.
Make it obvious that the commands don't work by removing them from the
getopt opt_string.
The tool will now behave as follows:
$ mkimage -f machine.its -k keys -K u-boot-pubkey.dtb -r image.fit
mkimage: invalid option -- 'k'
Error: Invalid option
Signed-off-by: Joel Stanley
---
tools/mkimage.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tools/mkimage.c b/tools/mkimage.c
index e78608293e72..10a1b3dc8c18 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -142,6 +142,12 @@ static int add_content(int type, const char *fname)
return 0;
}
+#ifdef CONFIG_FIT_SIGNATURE
+#define OPT_STRING "a:A:b:B:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qstT:vVx"
+#else
+#define OPT_STRING "a:A:b:C:d:D:e:f:i:ln:O:R:qstT:vVx"
+#endif
+
static void process_args(int argc, char **argv)
{
char *ptr;
@@ -149,8 +155,7 @@ static void process_args(int argc, char **argv)
char *datafile = NULL;
int opt;
- while ((opt = getopt(argc, argv,
- "a:A:b:B:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qstT:vVx")) != -1) {
+ while ((opt = getopt(argc, argv, OPT_STRING)) != -1) {
switch (opt) {
case 'a':
params.addr = strtoull(optarg, , 16);
[PATCH 3/3] mkimge: Reject signing-related flags without FIT_SIGNATURE
When CONFIG_FIT_SIGNATURE=n the signing options are not available. If a
user is careful they will notice this when looking at the help output.
If they are not careful they will waste several hours wondering what
they got wrong, as mkimage will silently ignore the signing related
options.
Make it obvious that the commands don't work by removing them from the
getopt opt_string.
The tool will now behave as follows:
$ mkimage -f machine.its -k keys -K u-boot-pubkey.dtb -r image.fit
mkimage: invalid option -- 'k'
Error: Invalid option
Signed-off-by: Joel Stanley
---
tools/mkimage.c | 9 +++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/tools/mkimage.c b/tools/mkimage.c
index e78608293e72..10a1b3dc8c18 100644
--- a/tools/mkimage.c
+++ b/tools/mkimage.c
@@ -142,6 +142,12 @@ static int add_content(int type, const char *fname)
return 0;
}
+#ifdef CONFIG_FIT_SIGNATURE
+#define OPT_STRING "a:A:b:B:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qstT:vVx"
+#else
+#define OPT_STRING "a:A:b:C:d:D:e:f:i:ln:O:R:qstT:vVx"
+#endif
+
static void process_args(int argc, char **argv)
{
char *ptr;
@@ -149,8 +155,7 @@ static void process_args(int argc, char **argv)
char *datafile = NULL;
int opt;
- while ((opt = getopt(argc, argv,
- "a:A:b:B:c:C:d:D:e:Ef:Fk:i:K:ln:N:p:O:rR:qstT:vVx")) != -1) {
+ while ((opt = getopt(argc, argv, OPT_STRING)) != -1) {
switch (opt) {
case 'a':
params.addr = strtoull(optarg, , 16);
--
2.28.0
