Re: [SCM] systemtap: system-wide probe/trace tool branch, master, updated. release-0.9.9-228-gd0822e2

[Dave Brolley <brolley at redhat dot com>]

Mark Wielaard wrote:
> Hi Dave,
> I was looking at this piece:
>
> +lookup_bad_addr(unsigned long addr, size_t size)
>  {
>    struct addr_map_entry* result = 0;
> +
> +#ifndef STP_PRIVILEGED
> +  /* Unprivileged users must not access kernel space memory.  */
> +  if (addr + size > TASK_SIZE)
> +    return 1;
> +#endif
> +
>
> I was wondering if that check cannot "overflow" if size is really big,
> making addr + size > TASK_SIZE succeed because the computation wraps
> around making is smaller than TASK_SIZE.
>   
Right you are. As well as all the other range calculations performed 
later. I think this is best handled by a single range check before 
anything else is done. I've pushed the attached patch.

Thanks for catching this,
Dave

diff --git a/runtime/addr-map.c b/runtime/addr-map.c
index 8c0e84d..a9aa8d8 100644
--- a/runtime/addr-map.c
+++ b/runtime/addr-map.c
@@ -110,6 +110,10 @@ lookup_bad_addr(unsigned long addr, size_t size)
 {
   struct addr_map_entry* result = 0;
 
+  /* Is this a valid memory access?  */
+  if (size == 0 || ULONG_MAX - addr < size - 1)
+    return 1;
+
 #ifndef STP_PRIVILEGED
   /* Unprivileged users must not access kernel space memory.  */
   if (addr + size > TASK_SIZE)