If there are multiple verity mode when parsing the verity mode of dm
verity table, it will be set as the last one.
So set to 'allow only once' to prevent it.
Signed-off-by: JeongHyeon Lee <[email protected]>
---
drivers/md/dm-verity-target.c | 38 ++++++++++++++++++++++++++---------
1 file changed, 28 insertions(+), 10 deletions(-)
diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
index 808a98ef624c..b76431dc7721 100644
--- a/drivers/md/dm-verity-target.c
+++ b/drivers/md/dm-verity-target.c
@@ -893,6 +893,28 @@ static int verity_alloc_zero_digest(struct dm_verity *v)
return r;
}
+static inline bool verity_is_verity_mode(const char *arg_name)
+{
+ return (!strcasecmp(arg_name, DM_VERITY_OPT_LOGGING) ||
+ !strcasecmp(arg_name, DM_VERITY_OPT_RESTART) ||
+ !strcasecmp(arg_name, DM_VERITY_OPT_PANIC));
+}
+
+static int verity_parse_verity_mode(struct dm_verity *v, const char *arg_name)
+{
+ if (v->mode)
+ return -EINVAL;
+
+ if (!strcasecmp(arg_name, DM_VERITY_OPT_LOGGING))
+ v->mode = DM_VERITY_MODE_LOGGING;
+ else if (!strcasecmp(arg_name, DM_VERITY_OPT_RESTART))
+ v->mode = DM_VERITY_MODE_RESTART;
+ else if (!strcasecmp(arg_name, DM_VERITY_OPT_PANIC))
+ v->mode = DM_VERITY_MODE_PANIC;
+
+ return 0;
+}
+
static int verity_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v,
struct dm_verity_sig_opts *verify_args)
{
@@ -916,16 +938,12 @@ static int verity_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v,
arg_name = dm_shift_arg(as);
argc--;
- if (!strcasecmp(arg_name, DM_VERITY_OPT_LOGGING)) {
- v->mode = DM_VERITY_MODE_LOGGING;
- continue;
-
- } else if (!strcasecmp(arg_name, DM_VERITY_OPT_RESTART)) {
- v->mode = DM_VERITY_MODE_RESTART;
- continue;
-
- } else if (!strcasecmp(arg_name, DM_VERITY_OPT_PANIC)) {
- v->mode = DM_VERITY_MODE_PANIC;
+ if (verity_is_verity_mode(arg_name)) {
+ r = verity_parse_verity_mode(v, arg_name);
+ if (r) {
+ ti->error = "Already verity mode set";
+ return r;
+ }
continue;
} else if (!strcasecmp(arg_name, DM_VERITY_OPT_IGN_ZEROES)) {
--
2.17.1
On Thu, Mar 11, 2021 at 4:19 AM JeongHyeon Lee <[email protected]> wrote:
>
> If there are multiple verity mode when parsing the verity mode of dm
> verity table, it will be set as the last one.
> So set to 'allow only once' to prevent it.
I agree that we shouldn't allow this, at least not without a warning,
but out of curiosity, do you actually have a situation where this
could happen? One ideally shouldn't be passing untrusted parameters to
dm-verity.
>
> Signed-off-by: JeongHyeon Lee <[email protected]>
> ---
> drivers/md/dm-verity-target.c | 38 ++++++++++++++++++++++++++---------
> 1 file changed, 28 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
> index 808a98ef624c..b76431dc7721 100644
> --- a/drivers/md/dm-verity-target.c
> +++ b/drivers/md/dm-verity-target.c
> @@ -893,6 +893,28 @@ static int verity_alloc_zero_digest(struct dm_verity *v)
> return r;
> }
>
> +static inline bool verity_is_verity_mode(const char *arg_name)
> +{
> + return (!strcasecmp(arg_name, DM_VERITY_OPT_LOGGING) ||
> + !strcasecmp(arg_name, DM_VERITY_OPT_RESTART) ||
> + !strcasecmp(arg_name, DM_VERITY_OPT_PANIC));
> +}
> +
> +static int verity_parse_verity_mode(struct dm_verity *v, const char *arg_name)
> +{
> + if (v->mode)
> + return -EINVAL;
> +
> + if (!strcasecmp(arg_name, DM_VERITY_OPT_LOGGING))
> + v->mode = DM_VERITY_MODE_LOGGING;
> + else if (!strcasecmp(arg_name, DM_VERITY_OPT_RESTART))
> + v->mode = DM_VERITY_MODE_RESTART;
> + else if (!strcasecmp(arg_name, DM_VERITY_OPT_PANIC))
> + v->mode = DM_VERITY_MODE_PANIC;
> +
> + return 0;
> +}
> +
> static int verity_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v,
> struct dm_verity_sig_opts *verify_args)
> {
> @@ -916,16 +938,12 @@ static int verity_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v,
> arg_name = dm_shift_arg(as);
> argc--;
>
> - if (!strcasecmp(arg_name, DM_VERITY_OPT_LOGGING)) {
> - v->mode = DM_VERITY_MODE_LOGGING;
> - continue;
> -
> - } else if (!strcasecmp(arg_name, DM_VERITY_OPT_RESTART)) {
> - v->mode = DM_VERITY_MODE_RESTART;
> - continue;
> -
> - } else if (!strcasecmp(arg_name, DM_VERITY_OPT_PANIC)) {
> - v->mode = DM_VERITY_MODE_PANIC;
> + if (verity_is_verity_mode(arg_name)) {
> + r = verity_parse_verity_mode(v, arg_name);
> + if (r) {
> + ti->error = "Already verity mode set";
I don't have a strong opinion about this, but the documentation
doesn't talk about verity modes, so perhaps this could be reworded to
something like "Conflicting error handling parameters"?
> + return r;
> + }
> continue;
>
> } else if (!strcasecmp(arg_name, DM_VERITY_OPT_IGN_ZEROES)) {
> --
> 2.17.1
>
Sami
Hello, Dear Sami Tolvanen.
Thank you for reply.
> I agree that we shouldn't allow this, at least not without a warning, but
> out of curiosity, do you actually have a situation where this could happen?
> One ideally shouldn't be passing untrusted parameters to dm-verity.
Of course, I don't think this will happen because they are dm-verity experts.
But since we are humans, I think this case could happen accidentally.
So it would be a good at preventing these cases.
> I don't have a strong opinion about this, but the documentation doesn't
> talk about verity modes, so perhaps this could be reworded to something
> like "Conflicting error handling parameters"?
Yes of course. That looks better.
I also had some ambiguous about how to express it.
This is because I couldn't find it in document.
The code says verity mode, so I wrote it down. never mind it :)
like this)
case DM_VERITY_MODE_LOGGING:
case DM_VERITY_MODE_RESTART:
case DM_VERITY_MODE_PANIC:
> On Thu, Mar 11, 2021 at 4:19 AM JeongHyeon Lee <[email protected]>
> wrote:
> >
> > If there are multiple verity mode when parsing the verity mode of dm
> > verity table, it will be set as the last one.
> > So set to 'allow only once' to prevent it.
>
> I agree that we shouldn't allow this, at least not without a warning, but
> out of curiosity, do you actually have a situation where this could happen?
> One ideally shouldn't be passing untrusted parameters to dm-verity.
>
> >
> > Signed-off-by: JeongHyeon Lee <[email protected]>
> > ---
> > drivers/md/dm-verity-target.c | 38
> > ++++++++++++++++++++++++++---------
> > 1 file changed, 28 insertions(+), 10 deletions(-)
> >
> > diff --git a/drivers/md/dm-verity-target.c
> > b/drivers/md/dm-verity-target.c index 808a98ef624c..b76431dc7721
> > 100644
> > --- a/drivers/md/dm-verity-target.c
> > +++ b/drivers/md/dm-verity-target.c
> > @@ -893,6 +893,28 @@ static int verity_alloc_zero_digest(struct
> dm_verity *v)
> > return r;
> > }
> >
> > +static inline bool verity_is_verity_mode(const char *arg_name) {
> > + return (!strcasecmp(arg_name, DM_VERITY_OPT_LOGGING) ||
> > + !strcasecmp(arg_name, DM_VERITY_OPT_RESTART) ||
> > + !strcasecmp(arg_name, DM_VERITY_OPT_PANIC)); }
> > +
> > +static int verity_parse_verity_mode(struct dm_verity *v, const char
> > +*arg_name) {
> > + if (v->mode)
> > + return -EINVAL;
> > +
> > + if (!strcasecmp(arg_name, DM_VERITY_OPT_LOGGING))
> > + v->mode = DM_VERITY_MODE_LOGGING;
> > + else if (!strcasecmp(arg_name, DM_VERITY_OPT_RESTART))
> > + v->mode = DM_VERITY_MODE_RESTART;
> > + else if (!strcasecmp(arg_name, DM_VERITY_OPT_PANIC))
> > + v->mode = DM_VERITY_MODE_PANIC;
> > +
> > + return 0;
> > +}
> > +
> > static int verity_parse_opt_args(struct dm_arg_set *as, struct
> dm_verity *v,
> > struct dm_verity_sig_opts
> > *verify_args) { @@ -916,16 +938,12 @@ static int
> > verity_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v,
> > arg_name = dm_shift_arg(as);
> > argc--;
> >
> > - if (!strcasecmp(arg_name, DM_VERITY_OPT_LOGGING)) {
> > - v->mode = DM_VERITY_MODE_LOGGING;
> > - continue;
> > -
> > - } else if (!strcasecmp(arg_name, DM_VERITY_OPT_RESTART)) {
> > - v->mode = DM_VERITY_MODE_RESTART;
> > - continue;
> > -
> > - } else if (!strcasecmp(arg_name, DM_VERITY_OPT_PANIC)) {
> > - v->mode = DM_VERITY_MODE_PANIC;
> > + if (verity_is_verity_mode(arg_name)) {
> > + r = verity_parse_verity_mode(v, arg_name);
> > + if (r) {
> > + ti->error = "Already verity mode set";
>
> I don't have a strong opinion about this, but the documentation doesn't
> talk about verity modes, so perhaps this could be reworded to something
> like "Conflicting error handling parameters"?
>
> > + return r;
> > + }
> > continue;
> >
> > } else if (!strcasecmp(arg_name,
> > DM_VERITY_OPT_IGN_ZEROES)) {
> > --
> > 2.17.1
> >
>
> Sami