[PATCH] smatch_kernel_host_data: enable additional debug

[PATCH] smatch_kernel_host_data: enable additional debug

[Elena Reshetova]

This allows calling __smatch_host_rl(*val) from the
analysed code in the same way as __smatch_user_rl, but
for the data influenced by the host in confidential
computing threat model.

Code author: Dan Carpenter

Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
---
 check_debug.c | 35 +++++++++++++++++++++++++++++++++++
 check_debug.h |  1 +
 2 files changed, 36 insertions(+)

diff --git a/check_debug.c b/check_debug.c
index 8b7b845f..058585ac 100644
--- a/check_debug.c
+++ b/check_debug.c
@@ -215,6 +215,40 @@ static void match_user_rl(const char *fn, struct expression *expr, void *info)
 	free_string(name);
 }
 
+static void match_host_rl(const char *fn, struct expression *expr, void *info)
+{
+       struct expression *arg;
+       struct range_list *rl = NULL;
+       struct sm_state *sm;
+       bool capped = false;
+       bool new = false;
+       int host_id;
+       char *name;
+
+       host_id = id_from_name("register_kernel_host_data");
+       if (!host_id) {
+               sm_msg("no host id");
+               return;
+       }
+
+       arg = get_argument_from_call_expr(expr->args, 0);
+       name = expr_to_str(arg);
+
+       get_host_rl(arg, &rl);
+       if (rl)
+               capped = user_rl_capped(arg);
+       sm = get_sm_state_expr(host_id, arg);
+       if (sm && estate_new(sm->state))
+               new = true;
+
+       sm_msg("host rl: '%s' = '%s'%s %s sm='%s'", name, show_rl(rl),
+              capped ? " (capped)" : "",
+              new ? "(new)" : "(old)",
+              show_sm(sm));
+
+       free_string(name);
+}
+
 static void match_capped(const char *fn, struct expression *expr, void *info)
 {
 	struct expression *arg;
@@ -856,6 +890,7 @@ void check_debug(int id)
 	add_function_hook("__smatch_implied_min", &match_print_implied_min, NULL);
 	add_function_hook("__smatch_implied_max", &match_print_implied_max, NULL);
 	add_function_hook("__smatch_user_rl", &match_user_rl, NULL);
+	add_function_hook("__smatch_host_rl", &match_host_rl, NULL);
 	add_function_hook("__smatch_capped", &match_capped, NULL);
 	add_function_hook("__smatch_hard_max", &match_print_hard_max, NULL);
 	add_function_hook("__smatch_fuzzy_max", &match_print_fuzzy_max, NULL);
diff --git a/check_debug.h b/check_debug.h
index 683a565f..f2e1a879 100644
--- a/check_debug.h
+++ b/check_debug.h
@@ -13,6 +13,7 @@ static inline void __smatch_implied(long long val){}
 static inline void __smatch_implied_min(long long val){}
 static inline void __smatch_implied_max(long long val){}
 static inline void __smatch_user_rl(long long val){}
+static inline void __smatch_host_rl(long long val){}
 static inline void __smatch_capped(long long val){}
 
 static inline void __smatch_hard_max(long long val){}
-- 
2.25.1

Re: [PATCH] smatch_kernel_host_data: enable additional debug

[Dan Carpenter]

On Wed, May 18, 2022 at 11:07:41AM +0300, Elena Reshetova wrote:
> This allows calling __smatch_host_rl(*val) from the
> analysed code in the same way as __smatch_user_rl, but
> for the data influenced by the host in confidential
> computing threat model.
> 
> Code author: Dan Carpenter
> 
> Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> ---

Thanks.  Applied.

It might take a while to push this because I'm trying to make some other
changes.  Smatch has become gradually unusably slow for me so it's time
for the annual DB cleanup.

regards,
dan carpenter

RE: [PATCH] smatch_kernel_host_data: enable additional debug

[Reshetova, Elena]

 On Wed, May 18, 2022 at 11:07:41AM +0300, Elena Reshetova wrote:
> > This allows calling __smatch_host_rl(*val) from the
> > analysed code in the same way as __smatch_user_rl, but
> > for the data influenced by the host in confidential
> > computing threat model.
> >
> > Code author: Dan Carpenter
> >
> > Signed-off-by: Elena Reshetova <elena.reshetova@intel.com>
> > ---
> 
> Thanks.  Applied.
> 
> It might take a while to push this because I'm trying to make some other
> changes.  Smatch has become gradually unusably slow for me so it's time
> for the annual DB cleanup.


Thank you Dan! No rush, I am just cleaning up my environment, rebasing,
etc. so wanted to send this last piece that related to host input now. 

Best Regards,
Elena.

Re: [PATCH] smatch_kernel_host_data: enable additional debug

[Dan Carpenter]

Hey Elena,

I've pushed some changes to smatch_kernel_host_data.c and
smatch_points_to_host_data.c.  It hopefully just brings that code more
in line with the user_data code.

The main thing is that smatch_points_to_host_data.c didn't differentiate
between when the function gets host data from the user vs when it is
passed in.  That means if you have function:

int *frob(int *x)
{
	return x;
}

And one caller passes in host data then all of them get host data back.
So then now we're passing host data pointers to even more functions and
the problem gets worse and worse over time.

Which is maybe not a huge deal in terms of the warnings generated, but
it was taking 8GB of data in my database.

regards,
dan carpenter

RE: [PATCH] smatch_kernel_host_data: enable additional debug

[Reshetova, Elena]

> Hey Elena,
> 
> I've pushed some changes to smatch_kernel_host_data.c and
> smatch_points_to_host_data.c.  It hopefully just brings that code more
> in line with the user_data code.

Hi Dan, 

Thank you very much for the fixes!

I am actually still planning to work on this further, especially I need to finish
re-writing the new pattern that produces the warning list for all host
input processing done in the code (we use this as a basis for our fuzzing coverage
analysis). 

> 
> The main thing is that smatch_points_to_host_data.c didn't differentiate
> between when the function gets host data from the user vs when it is
> passed in.  That means if you have function:
> 
> int *frob(int *x)
> {
> 	return x;
> }
> 
> And one caller passes in host data then all of them get host data back.
> So then now we're passing host data pointers to even more functions and
> the problem gets worse and worse over time.

Yes, makes sense, I didn’t realize that this was happening with the previous code. 

> 
> Which is maybe not a huge deal in terms of the warnings generated, but
> it was taking 8GB of data in my database.

Oh, this is really bad for the database. I am sorry about this!

Best Regards,
Elena.

> 
> regards,
> dan carpenter